diff --git a/src/client/composer/composer-web-view.vala b/src/client/composer/composer-web-view.vala index f86b0e49..67bdf640 100644 --- a/src/client/composer/composer-web-view.vala +++ b/src/client/composer/composer-web-view.vala @@ -16,53 +16,6 @@ public class ComposerWebView : ClientWebView { private const string CURSOR_CONTEXT_CHANGED = "cursorContextChanged"; private const string DOCUMENT_MODIFIED = "documentModified"; - private const string HTML_BODY = """ - - - -
%s
- """; - - /** * Encapsulates editing-related state for a specific DOM node. * @@ -125,10 +78,14 @@ public class ComposerWebView : ClientWebView { } + private static WebKit.UserStyleSheet? app_style = null; private static WebKit.UserScript? app_script = null; public static void load_resources() throws Error { + ComposerWebView.app_style = ClientWebView.load_app_stylesheet( + "composer-web-view.css" + ); ComposerWebView.app_script = ClientWebView.load_app_script( "composer-web-view.js" ); @@ -171,8 +128,8 @@ public class ComposerWebView : ClientWebView { add_events(Gdk.EventMask.KEY_PRESS_MASK | Gdk.EventMask.KEY_RELEASE_MASK); + this.user_content_manager.add_style_sheet(ComposerWebView.app_style); this.user_content_manager.add_script(ComposerWebView.app_script); - // this.should_insert_text.connect(on_should_insert_text); register_message_handler(COMMAND_STACK_CHANGED, on_command_stack_changed); register_message_handler(CURSOR_CONTEXT_CHANGED, on_cursor_context_changed); @@ -187,39 +144,54 @@ public class ComposerWebView : ClientWebView { string quote, bool top_posting, bool is_draft) { - const string CURSOR = ""; - const string SPACER = "
"; + const string HTML_PRE = """"""; + const string HTML_POST = """"""; + const string BODY_PRE = """ +
"""; + const string BODY_POST = """
+"""; + const string SIGNATURE = """ +
%s
+"""; + const string QUOTE = """ +

%s
+"""; + const string CURSOR = "

"; + const string SPACER = "

"; StringBuilder html = new StringBuilder(); + html.append(HTML_PRE); if (!is_draft) { - if (!Geary.String.is_empty(body)) { + html.append(BODY_PRE); + bool have_body = !Geary.String.is_empty(body); + if (have_body) { html.append(body); - html.append(SPACER); - html.append(SPACER); } if (!top_posting && !Geary.String.is_empty(quote)) { + if (have_body) { + html.append(SPACER); + } html.append(quote); - html.append(SPACER); } + html.append(SPACER); html.append(CURSOR); + html.append(SPACER); + html.append(BODY_POST); if (!Geary.String.is_empty(signature)) { - html.append(SPACER); - html.append(signature); + html.append_printf(SIGNATURE, signature); } if (top_posting && !Geary.String.is_empty(quote)) { - html.append(SPACER); - html.append(SPACER); - html.append(quote); + html.append_printf(QUOTE, quote); } } else { html.append(quote); } - - base.load_html(HTML_BODY.printf(html.data)); + html.append(HTML_POST); + base.load_html((string) html.data); } /** @@ -440,10 +412,13 @@ public class ComposerWebView : ClientWebView { } /** - * Converts plain text URLs in the editor content into links. + * Cleans the editor content ready for sending. + * + * This modifies the DOM, so there's no going back after calling + * this. */ - public async void linkify_content() throws Error { - this.call.begin(Geary.JS.callable("geary.linkifyContent"), null); + public async void clean_content() throws Error { + this.call.begin(Geary.JS.callable("geary.cleanContent"), null); } /** diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala index 94c4c502..0746c280 100644 --- a/src/client/composer/composer-widget.vala +++ b/src/client/composer/composer-widget.vala @@ -1268,7 +1268,7 @@ public class ComposerWidget : Gtk.EventBox { // Perform send. try { - yield this.editor.linkify_content(); + yield this.editor.clean_content(); yield this.account.send_email_async(yield get_composed_email()); } catch (Error e) { GLib.message("Error sending email: %s", e.message); diff --git a/src/engine/rfc822/rfc822-utils.vala b/src/engine/rfc822/rfc822-utils.vala index 2b57333f..8a50b28b 100644 --- a/src/engine/rfc822/rfc822-utils.vala +++ b/src/engine/rfc822/rfc822-utils.vala @@ -257,7 +257,7 @@ public string quote_email_for_reply(Geary.Email email, string? quote, TextFormat } quoted += "
"; - quoted += "\n" + quote_body(email, quote, true, format); + quoted += quote_body(email, quote, true, format); return quoted; } @@ -276,7 +276,7 @@ public string quote_email_for_forward(Geary.Email email, string? quote, TextForm return ""; string quoted = _("---------- Forwarded message ----------"); - quoted += "\n\n"; + quoted += "\n"; string from_line = email_addresses_for_reply(email.from, format); if (!String.is_empty_or_whitespace(from_line)) quoted += _("From: %s\n").printf(from_line); @@ -289,11 +289,8 @@ public string quote_email_for_forward(Geary.Email email, string? quote, TextForm if (!String.is_empty_or_whitespace(cc_line)) quoted += _("Cc: %s\n").printf(cc_line); quoted += "\n"; // A blank line between headers and body - quoted = quoted.replace("\n", "
"); - quoted += quote_body(email, quote, false, format); - return quoted; } diff --git a/test/client/composer/composer-web-view-test.vala b/test/client/composer/composer-web-view-test.vala index 6fe7e79a..f72a8449 100644 --- a/test/client/composer/composer-web-view-test.vala +++ b/test/client/composer/composer-web-view-test.vala @@ -7,6 +7,8 @@ public class ComposerWebViewTest : ClientWebViewTestCase { + private const string BODY_TEMPLATE = """
%s



"""; + public ComposerWebViewTest() { base("ComposerWebViewTest"); add_test("load_resources", load_resources); @@ -45,7 +47,8 @@ public class ComposerWebViewTest : ClientWebViewTestCase { load_body_fixture(html); this.test_view.get_html.begin((obj, ret) => { async_complete(ret); }); try { - assert(this.test_view.get_html.end(async_result()) == html + "

"); + assert(this.test_view.get_html.end(async_result()) == + BODY_TEMPLATE.printf(html)); } catch (Error err) { print("Error: %s\n", err.message); assert_not_reached(); @@ -56,7 +59,7 @@ public class ComposerWebViewTest : ClientWebViewTestCase { load_body_fixture("

para

"); this.test_view.get_text.begin((obj, ret) => { async_complete(ret); }); try { - assert(this.test_view.get_text.end(async_result()) == "para\n\n\n\n\n"); + assert(this.test_view.get_text.end(async_result()) == "para\n\n\n\n\n\n"); } catch (Error err) { print("Error: %s\n", err.message); assert_not_reached(); @@ -68,7 +71,7 @@ public class ComposerWebViewTest : ClientWebViewTestCase { this.test_view.get_text.begin((obj, ret) => { async_complete(ret); }); try { assert(this.test_view.get_text.end(async_result()) == - "pre\n\n> quote\n> \npost\n\n\n\n\n"); + "pre\n\n> quote\n> \npost\n\n\n\n\n\n"); } catch (Error err) { print("Error: %s\n", err.message); assert_not_reached(); @@ -80,7 +83,7 @@ public class ComposerWebViewTest : ClientWebViewTestCase { this.test_view.get_text.begin((obj, ret) => { async_complete(ret); }); try { assert(this.test_view.get_text.end(async_result()) == - "pre\n\n> quote1\n> \n>> quote2\n>> \npost\n\n\n\n\n"); + "pre\n\n> quote1\n> \n>> quote2\n>> \npost\n\n\n\n\n\n"); } catch (Error err) { print("Error: %s\n", err.message); assert_not_reached(); @@ -102,6 +105,7 @@ long long, long long, long. + """); } catch (Error err) { print("Error: %s\n", err.message); @@ -128,6 +132,7 @@ long long, long long, long. + """); } catch (Error err) { print("Error: %s\n", err.message); @@ -161,6 +166,7 @@ long, long, long, long, long, long, long, long, long, long, + """); } catch (Error err) { print("Error: %s\n", err.message); diff --git a/test/js/composer-page-state-test.vala b/test/js/composer-page-state-test.vala index 9e345912..f931b6c3 100644 --- a/test/js/composer-page-state-test.vala +++ b/test/js/composer-page-state-test.vala @@ -7,13 +7,16 @@ class ComposerPageStateTest : ClientWebViewTestCase { + private const string COMPLETE_BODY_TEMPLATE = """
%s



"""; + private const string CLEAN_BODY_TEMPLATE = "%s



"; + public ComposerPageStateTest() { base("ComposerPageStateTest"); add_test("edit_context_font", edit_context_font); add_test("edit_context_link", edit_context_link); add_test("indent_line", indent_line); add_test("contains_attachment_keywords", contains_attachment_keywords); - add_test("linkify_content", linkify_content); + add_test("clean_content", clean_content); add_test("get_html", get_html); add_test("get_text", get_text); add_test("get_text_with_quote", get_text_with_quote); @@ -81,17 +84,17 @@ class ComposerPageStateTest : ClientWebViewTestCase { } public void contains_attachment_keywords() { - load_body_fixture(""" -
inner quote
+ load_body_fixture_full(""" +
innerquote

some text

some text - ---
sig - -

outerquote text

-"""); +""", + "--
sig", + "

outerquote text

", + true + ); try { assert(WebKitUtil.to_bool(run_javascript( @"geary.containsAttachmentKeyword(\"some\", \"subject text\");" @@ -117,7 +120,7 @@ some text } } - public void linkify_content() { + public void clean_content() { // XXX split these up into multiple tests load_body_fixture(""" http://example1.com @@ -141,12 +144,12 @@ unknown://example6.com http://example5.com unknown://example6.com -

"""; +"""; try { - run_javascript("geary.linkifyContent();"); - assert(WebKitUtil.to_string(run_javascript("geary.messageBody.innerHTML;")) == - expected); + run_javascript("geary.cleanContent();"); + assert(WebKitUtil.to_string(run_javascript("geary.bodyPart.innerHTML;")) == + CLEAN_BODY_TEMPLATE.printf(expected)); } catch (Geary.JS.Error err) { print("Geary.JS.Error: %s\n", err.message); assert_not_reached(); @@ -161,7 +164,7 @@ unknown://example6.com load_body_fixture(html); try { assert(WebKitUtil.to_string(run_javascript(@"window.geary.getHtml();")) == - html + "

"); + COMPLETE_BODY_TEMPLATE.printf(html)); } catch (Geary.JS.Error err) { print("Geary.JS.Error: %s\n", err.message); assert_not_reached(); @@ -175,7 +178,7 @@ unknown://example6.com load_body_fixture("

para

"); try { assert(WebKitUtil.to_string(run_javascript(@"window.geary.getText();")) == - "para\n\n\n\n"); + "para\n\n\n\n\n"); } catch (Geary.JS.Error err) { print("Geary.JS.Error: %s\n", err.message); assert_not_reached(); @@ -190,7 +193,7 @@ unknown://example6.com load_body_fixture("

pre

quote

post

"); try { assert(WebKitUtil.to_string(run_javascript(@"window.geary.getText();")) == - @"pre\n\n$(q_marker)quote\n$(q_marker)\npost\n\n\n\n"); + @"pre\n\n$(q_marker)quote\n$(q_marker)\npost\n\n\n\n\n"); } catch (Geary.JS.Error err) { print("Geary.JS.Error: %s", err.message); assert_not_reached(); @@ -205,7 +208,7 @@ unknown://example6.com load_body_fixture("

pre

quote1

quote2

post

"); try { assert(WebKitUtil.to_string(run_javascript(@"window.geary.getText();")) == - @"pre\n\n$(q_marker)quote1\n$(q_marker)\n$(q_marker)$(q_marker)quote2\n$(q_marker)$(q_marker)\npost\n\n\n\n"); + @"pre\n\n$(q_marker)quote1\n$(q_marker)\n$(q_marker)$(q_marker)quote2\n$(q_marker)$(q_marker)\npost\n\n\n\n\n"); } catch (Geary.JS.Error err) { print("Geary.JS.Error: %s\n", err.message); assert_not_reached(); @@ -343,8 +346,15 @@ unknown://example6.com return new ComposerWebView(this.config); } - protected override void load_body_fixture(string html = "") { - this.test_view.load_html(html, "", "", false, false); + protected override void load_body_fixture(string body = "") { + load_body_fixture_full(body, "", "", true); + } + + protected void load_body_fixture_full(string body, + string sig, + string quote, + bool top_posting) { + this.test_view.load_html(body, sig, quote, top_posting, false); while (this.test_view.is_loading) { Gtk.main_iteration(); } diff --git a/ui/CMakeLists.txt b/ui/CMakeLists.txt index bc5c57d1..dc410911 100644 --- a/ui/CMakeLists.txt +++ b/ui/CMakeLists.txt @@ -11,6 +11,7 @@ set(RESOURCE_LIST STRIPBLANKS "composer-link-popover.ui" STRIPBLANKS "composer-menus.ui" STRIPBLANKS "composer-widget.ui" + "composer-web-view.css" "composer-web-view.js" STRIPBLANKS "conversation-email.ui" STRIPBLANKS "conversation-email-attachment-view.ui" diff --git a/ui/composer-web-view.css b/ui/composer-web-view.css new file mode 100644 index 00000000..83c36e90 --- /dev/null +++ b/ui/composer-web-view.css @@ -0,0 +1,60 @@ +/* + * Copyright 2016 Software Freedom Conservancy Inc. + * Copyright 2017 Michael Gratton + */ + +body { + margin: 0 !important; + border: 0 !important; + padding: 0 !important; + background-color: #f9f9f9 !important; + font-size: medium !important; +} + +body.plain, body.plain * { + font-family: monospace !important; + font-weight: normal; + font-style: normal; + font-size: medium !important; + color: black; + text-decoration: none; +} + +body.plain a { + cursor: text; +} + +body > div#geary-body { + margin: 0 !important; + border: 0 !important; + padding: 12px !important; + outline: 0px !important; +} + +body > div#geary-signature, +body > div#geary-quote { + margin: 0 6px !important; + border: 0 !important; + padding: 6px !important; +} + +body > div.geary-focus { + background-color: white !important; +} + +body > div#geary-signature.geary-focus, +body > div#geary-quote.geary-focus { + outline: 1px dashed #ccc !important; +} + +blockquote { + margin: 0 10px; + border: 0; + border-left: 3px #aaa solid; + padding: 0 5px; +} + +pre { + white-space: pre-wrap; + margin: 0; +} diff --git a/ui/composer-web-view.js b/ui/composer-web-view.js index e52ec383..a0ab0970 100644 --- a/ui/composer-web-view.js +++ b/ui/composer-web-view.js @@ -12,7 +12,6 @@ let ComposerPageState = function() { this.init.apply(this, arguments); }; -ComposerPageState.BODY_ID = "message-body"; ComposerPageState.KEYWORD_SPLIT_REGEX = /[\s]+/g; ComposerPageState.QUOTE_START = "\x91"; // private use one ComposerPageState.QUOTE_END = "\x92"; // private use two @@ -26,7 +25,11 @@ ComposerPageState.prototype = { __proto__: PageState.prototype, init: function() { PageState.prototype.init.apply(this, []); - this.messageBody = null; + this.bodyPart = null; + this.signaturePart = null; + this.quotePart = null; + this.focusedPart = null; + this.undoEnabled = false; this.redoEnabled = false; this.selections = new Map(); @@ -55,12 +58,19 @@ ComposerPageState.prototype = { loaded: function() { let state = this; - this.messageBody = document.getElementById(ComposerPageState.BODY_ID); + this.bodyPart = document.getElementById("geary-body"); + if (this.bodyPart == null) { + this.bodyPart = document.body; + } + + this.signaturePart = document.getElementById("geary-signature"); + this.quotePart = document.getElementById("geary-quote"); + // Should be using 'e.key' in listeners below instead of // keyIdentifier, but that was only fixed in WK in Oct 2016 // (WK Bug 36267). Migrate to that when we can rely on it // being in WebKitGTK. - this.messageBody.addEventListener("keydown", function(e) { + document.body.addEventListener("keydown", function(e) { if (e.keyIdentifier == "U+0009" &&// Tab !e.ctrlKey && !e.altKey && !e.metaKey) { if (!e.shiftKey) { @@ -74,7 +84,7 @@ ComposerPageState.prototype = { // We can't use keydown for this, captured or bubbled, since // that will also cause the line that the cursor is currenty // positioned on when Enter is pressed to also be outdented. - this.messageBody.addEventListener("keyup", function(e) { + document.body.addEventListener("keyup", function(e) { if (e.keyIdentifier == "Enter" && !e.shiftKey) { // XXX WebKit seems to support both InsertNewline and // InsertNewlineInQuotedContent arguments for @@ -101,6 +111,7 @@ ComposerPageState.prototype = { // Focus within the HTML document document.body.focus(); + this.updateFocusClass(this.bodyPart); // Set text cursor at appropriate position let cursor = document.getElementById("cursormarker"); @@ -117,14 +128,20 @@ ComposerPageState.prototype = { // Enable editing and observation machinery only after // modifying the body above. - this.messageBody.contentEditable = true; + this.bodyPart.contentEditable = true; + if (this.signaturePart != null) { + this.signaturePart.contentEditable = true; + } + if (this.quotePart != null) { + this.quotePart.contentEditable = true; + } let config = { attributes: true, childList: true, characterData: true, subtree: true }; - this.bodyObserver.observe(this.messageBody, config); + this.bodyObserver.observe(document.body, config); // Chain up PageState.prototype.loaded.apply(this, []); @@ -184,20 +201,21 @@ ComposerPageState.prototype = { } }, updateSignature: function(signature) { - // XXX need mark the sig somehow so we can find it, select - // it and replace it using execCommand + if (this.signaturePart != null) { + console.log(signature); + this.signaturePart.innerHTML = signature; + } }, deleteQuotedMessage: function() { - // XXX need mark the quote somehow so we can find it, select - // it and delete it using execCommand + if (this.quotePart != null) { + this.quotePart.parentNode.removeChild(this.quotePart); + this.quotePart = null; + } }, /** * Determines if subject or body content refers to attachments. */ containsAttachmentKeyword: function(keywordSpec, subject) { - // XXX this could also use a structured representation of the - // message body so we don't need to text to check - let ATTACHMENT_KEYWORDS_SUFFIX = "doc|pdf|xls|ppt|rtf|pps"; let completeKeys = new Set(keywordSpec.toLocaleLowerCase().split("|")); @@ -209,57 +227,30 @@ ComposerPageState.prototype = { } // Check interesting body text + let node = this.bodyPart.firstChild; + let content = []; let breakingElements = new Set([ "BR", "P", "DIV", "BLOCKQUOTE", "TABLE", "OL", "UL", "HR" ]); - let content = this.messageBody.firstChild; - let found = false; - let done = false; - let textContent = []; - while (content != null && !done) { - if (content.nodeType == Node.TEXT_NODE) { - textContent.push(content.textContent); + while (node != null) { + if (node.nodeType == Node.TEXT_NODE) { + content.push(node.textContent); } else if (content.nodeType == Node.ELEMENT_NODE) { - let isBreaking = breakingElements.has(content.nodeName); + let isBreaking = breakingElements.has(node.nodeName); if (isBreaking) { - textContent.push("\n"); + content.push("\n"); } - // Always exclude quoted text + // Only include non-quoted text if (content.nodeName != "BLOCKQUOTE") { - textContent.push(content.innerText); - } - - if (isBreaking || content.nextSibling == null) { - for (let line of textContent.join("").split("\n")) { - // Ignore everything after a sig or a - // forwarded message. - // XXX This breaks if the user types this at - // the start of a line, also, WK's innerText - // impl strips trailing whitespace, so can't - // test for 'line == "-- "' :( - if (line.startsWith("--")) { - done = true; - break; - } - - line = line.trim(); - if (line != "") { - if (ComposerPageState.containsKeywords(line, completeKeys, suffixKeys)) { - found = true; - done = true; - break; - } - } - } - textContent = []; + content.push(content.textContent); } } - - content = content.nextSibling; + node = node.nextSibling; } - - return found; + return ComposerPageState.containsKeywords( + content.join(""), completeKeys, suffixKeys + ); }, tabOut: function() { document.execCommand( @@ -284,7 +275,7 @@ ComposerPageState.prototype = { // execCommand affecting the DOM srtcuture let count = 0; let node = SelectionUtil.getCursorElement(); - while (node != this.messageBody) { + while (node != document.body) { if (node.nodeName == "BLOCKQUOTE") { count++; } @@ -295,14 +286,39 @@ ComposerPageState.prototype = { count--; } }, - linkifyContent: function() { - ComposerPageState.linkify(this.messageBody); + cleanContent: function() { + ComposerPageState.cleanPart(this.bodyPart, false); + ComposerPageState.linkify(this.bodyPart); + + this.signaturePart = ComposerPageState.cleanPart(this.signaturePart, true); + this.quotePart = ComposerPageState.cleanPart(this.quotePart, true); }, getHtml: function() { - return this.messageBody.innerHTML; + // Clone the message parts so we can clean them without + // modifiying the DOM, needed when saving drafts. In contrast + // with cleanContent above, we don't remove empty elements so + // they still exist when restoring from draft + let parent = document.createElement("DIV"); + parent.appendChild( + ComposerPageState.cleanPart(this.bodyPart.cloneNode(true), false) + ); + + if (this.signaturePart != null) { + parent.appendChild( + ComposerPageState.cleanPart(this.signaturePart.cloneNode(true), false) + ); + } + + if (this.quotePart != null) { + parent.appendChild( + ComposerPageState.cleanPart(this.quotePart.cloneNode(true), false) + ); + } + + return parent.innerHTML; }, getText: function() { - return ComposerPageState.htmlToQuotedText(this.messageBody); + return ComposerPageState.htmlToQuotedText(document.body); }, setRichText: function(enabled) { if (enabled) { @@ -330,6 +346,8 @@ ComposerPageState.prototype = { PageState.prototype.selectionChanged.apply(this, []); let cursor = SelectionUtil.getCursorElement(); + + // Update cursor context if (cursor != null) { let newContext = new EditContext(cursor); if (!newContext.equals(this.cursorContext)) { @@ -339,6 +357,30 @@ ComposerPageState.prototype = { ); } } + + while (cursor != null) { + let parent = cursor.parentNode; + if (parent == document.body) { + this.updateFocusClass(cursor); + break; + } + cursor = parent; + } + }, + /** + * Work around WebKit note yet supporting :focus-inside pseudoclass. + */ + updateFocusClass: function(newFocus) { + if (this.focusedPart != null) { + this.focusedPart.classList.remove("geary-focus"); + this.focusedPart = null; + } + if (newFocus == this.bodyPart || + newFocus == this.signaturePart || + newFocus == this.quotePart) { + this.focusedPart = newFocus; + this.focusedPart.classList.add("geary-focus"); + } } }; @@ -375,6 +417,22 @@ ComposerPageState.containsKeywords = function(line, completeKeys, suffixKeys) { return false; }; +/** + * Removes internal attributes from a composer part.. + */ +ComposerPageState.cleanPart = function(part, removeIfEmpty) { + if (part != null) { + part.removeAttribute("class"); + part.removeAttribute("contenteditable"); + + if (removeIfEmpty && part.innerText.trim() == "") { + part.parentNode.removeChild(part); + part = null; + } + } + return part; +}; + /** * Convert a HTML DOM tree to plain text with delineated quotes. *