[geary/mjog/493-undo-send: 11/25] Minimise DOM changes made by ComposerPageState::cleanContent JS

[Michael Gratton <mjog src gnome org>]

commit d7c8a2993ca6c57ab7f5c4bcabcef96f2ab704ae
Author: Michael Gratton <mike vee net>
Date:   Fri Nov 8 10:39:46 2019 +1100

    Minimise DOM changes made by ComposerPageState::cleanContent JS
    
    By overriding ComposerPageState::getHtml to accept a parameter that
    supports getting HTML with empty composer parts removed, cleanContent
    can simply be made to linkify the page, thus allowing a ComposerWidget
    to be re-used multiple times when sending an email.

 src/client/composer/composer-web-view.vala       |  9 +++++
 src/client/composer/composer-widget.vala         | 14 +++++---
 test/client/composer/composer-web-view-test.vala |  9 +++++
 test/js/composer-page-state-test.vala            | 24 +++++++------
 ui/composer-web-view.js                          | 46 ++++++++++--------------
 5 files changed, 59 insertions(+), 43 deletions(-)
---
diff --git a/src/client/composer/composer-web-view.vala b/src/client/composer/composer-web-view.vala
index 5a7fd526..98770197 100644
--- a/src/client/composer/composer-web-view.vala
+++ b/src/client/composer/composer-web-view.vala
@@ -178,6 +178,15 @@ public class ComposerWebView : ClientWebView {
         base.load_html((string) html.data);
     }
 
+    /**
+     * Returns the view's content as HTML without being cleaned.
+     */
+    public async string? get_html_for_draft() throws Error {
+        return Util.JS.to_string(
+            yield call(Util.JS.callable("geary.getHtml").bool(false), null)
+        );
+    }
+
     /**
      * Makes the view uneditable and stops signals from being sent.
      */
diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala
index e16891af..ea39c3a8 100644
--- a/src/client/composer/composer-widget.vala
+++ b/src/client/composer/composer-widget.vala
@@ -1019,8 +1019,8 @@ public class ComposerWidget : Gtk.EventBox, Geary.BaseInterface {
         return true;
     }
 
-    public async Geary.ComposedEmail get_composed_email(DateTime? date_override = null,
-        bool only_html = false) {
+    public async Geary.ComposedEmail get_composed_email(GLib.DateTime? date_override = null,
+                                                        bool for_draft = false) {
         Geary.ComposedEmail email = new Geary.ComposedEmail(
             date_override ?? new DateTime.now_local(),
             from
@@ -1055,10 +1055,14 @@ public class ComposerWidget : Gtk.EventBox, Geary.BaseInterface {
         email.img_src_prefix = ClientWebView.INTERNAL_URL_PREFIX;
 
         try {
-            if (this.editor.is_rich_text || only_html)
-                email.body_html = yield this.editor.get_html();
-            if (!only_html)
+            if (!for_draft) {
+                if (this.editor.is_rich_text) {
+                    email.body_html = yield this.editor.get_html();
+                }
                 email.body_text = yield this.editor.get_text();
+            } else {
+                email.body_html = yield this.editor.get_html_for_draft();
+            }
         } catch (Error error) {
             debug("Error getting composer message body: %s", error.message);
         }
diff --git a/test/client/composer/composer-web-view-test.vala 
b/test/client/composer/composer-web-view-test.vala
index ef5b97e0..49b222ae 100644
--- a/test/client/composer/composer-web-view-test.vala
+++ b/test/client/composer/composer-web-view-test.vala
@@ -13,6 +13,7 @@ public class ComposerWebViewTest : ClientWebViewTestCase<ComposerWebView> {
         add_test("load_resources", load_resources);
         add_test("edit_context", edit_context);
         add_test("get_html", get_html);
+        add_test("get_html_for_draft", get_html_for_draft);
         add_test("get_text", get_text);
         add_test("get_text_with_quote", get_text_with_quote);
         add_test("get_text_with_nested_quote", get_text_with_nested_quote);
@@ -50,6 +51,14 @@ public class ComposerWebViewTest : ClientWebViewTestCase<ComposerWebView> {
         load_body_fixture(BODY);
         this.test_view.get_html.begin((obj, ret) => { async_complete(ret); });
         string html = this.test_view.get_html.end(async_result());
+        assert_string(ComposerPageStateTest.CLEAN_BODY_TEMPLATE.printf(BODY), html);
+    }
+
+    public void get_html_for_draft() throws GLib.Error {
+        string BODY = "<p>para</p>";
+        load_body_fixture(BODY);
+        this.test_view.get_html_for_draft.begin((obj, ret) => { async_complete(ret); });
+        string html = this.test_view.get_html.end(async_result());
         assert_string(ComposerPageStateTest.COMPLETE_BODY_TEMPLATE.printf(BODY), html);
     }
 
diff --git a/test/js/composer-page-state-test.vala b/test/js/composer-page-state-test.vala
index 465bcc12..197efeff 100644
--- a/test/js/composer-page-state-test.vala
+++ b/test/js/composer-page-state-test.vala
@@ -9,7 +9,12 @@ class ComposerPageStateTest : ClientWebViewTestCase<ComposerWebView> {
 
     public const string COMPLETE_BODY_TEMPLATE =
         """<div id="geary-body" dir="auto">%s<div><br></div><div><br></div></div><div id="geary-signature" 
dir="auto"></div>""";
-    public const string CLEAN_BODY_TEMPLATE = "%s<div><br></div><div><br></div>";
+    public const string DIRTY_BODY_TEMPLATE =
+        """
+<div id="geary-body" dir="auto" class="geary-focus" 
contenteditable="true">%s<div><br></div><div><br></div></div>
+<div id="geary-signature" class="geary-no-display" dir="auto" contenteditable="true"></div>
+""";
+    public const string CLEAN_BODY_TEMPLATE = """<div id="geary-body" 
dir="auto">%s<div><br></div><div><br></div></div>""";
 
     public ComposerPageStateTest() {
         base("ComposerPageStateTest");
@@ -254,12 +259,11 @@ I can send email through smtp.gmail.com:587 or through <a href="https://www.gmai
 
         try {
             run_javascript("geary.cleanContent();");
-            assert(
-                Util.JS.to_string(
-                    run_javascript("geary.bodyPart.innerHTML;")
-                    .get_js_value()
-                ) == CLEAN_BODY_TEMPLATE.printf(expected)
+            string result = Util.JS.to_string(
+                run_javascript("window.document.body.innerHTML;")
+                .get_js_value()
             );
+            assert(result == DIRTY_BODY_TEMPLATE.printf(expected));
         } catch (Util.JS.Error err) {
             print("Util.JS.Error: %s\n", err.message);
             assert_not_reached();
@@ -273,12 +277,10 @@ I can send email through smtp.gmail.com:587 or through <a href="https://www.gmai
         string html = "<p>para</p>";
         load_body_fixture(html);
         try {
-            assert(
-                Util.JS.to_string(
-                   run_javascript(@"window.geary.getHtml();")
-                   .get_js_value()
-                ) == COMPLETE_BODY_TEMPLATE.printf(html)
+            string result = Util.JS.to_string(
+                run_javascript(@"window.geary.getHtml();").get_js_value()
             );
+            assert(result == CLEAN_BODY_TEMPLATE.printf(html));
         } catch (Util.JS.Error err) {
             print("Util.JS.Error: %s\n", err.message);
             assert_not_reached();
diff --git a/ui/composer-web-view.js b/ui/composer-web-view.js
index 848290ed..7b548be3 100644
--- a/ui/composer-web-view.js
+++ b/ui/composer-web-view.js
@@ -60,6 +60,9 @@ ComposerPageState.prototype = {
         }
 
         this.signaturePart = document.getElementById("geary-signature");
+        if (this.signaturePart != null) {
+            ComposerPageState.linkify(this.signaturePart);
+        }
         this.quotePart = document.getElementById("geary-quote");
 
         // Should be using 'e.key' in listeners below instead of
@@ -283,38 +286,32 @@ ComposerPageState.prototype = {
     },
     cleanContent: function() {
         // Prevent any modification signals being sent when mutating
-        // the document below.
+        // the document
         this.stopBodyObserver();
-
-        ComposerPageState.cleanPart(this.bodyPart, false);
         ComposerPageState.linkify(this.bodyPart);
-
-        this.signaturePart = ComposerPageState.cleanPart(this.signaturePart, true);
-        this.quotePart = ComposerPageState.cleanPart(this.quotePart, true);
+        this.startBodyObserver();
     },
-    getHtml: function() {
-        // 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
+    getHtml: function(removeEmpty) {
+        if (removeEmpty === undefined) {
+            removeEmpty = true;
+        }
+
         let parent = document.createElement("DIV");
         parent.appendChild(
-            ComposerPageState.cleanPart(this.bodyPart.cloneNode(true), false)
+            ComposerPageState.cleanPart(this.bodyPart.cloneNode(true))
         );
 
-        if (this.signaturePart != null) {
+        if (this.signaturePart != null &&
+            (!removeEmpty || this.signaturePart.innerText.trim() != "")) {
             parent.appendChild(
-                ComposerPageState.cleanPart(
-                    this.signaturePart.cloneNode(true), false
-                )
+                ComposerPageState.cleanPart(this.signaturePart.cloneNode(true))
             );
         }
 
-        if (this.quotePart != null) {
+        if (this.quotePart != null &&
+            (!removeEmpty || this.quotePart.innerText.trim() != "")) {
             parent.appendChild(
-                ComposerPageState.cleanPart(
-                    this.quotePart.cloneNode(true), false
-                )
+                ComposerPageState.cleanPart(this.quotePart.cloneNode(true))
             );
         }
 
@@ -433,17 +430,12 @@ ComposerPageState.containsKeywords = function(line, wordKeys, suffixKeys) {
 };
 
 /**
- * Removes internal attributes from a composer part..
+ * Removes internal attributes from a composer part.
  */
-ComposerPageState.cleanPart = function(part, removeIfEmpty) {
+ComposerPageState.cleanPart = function(part) {
     if (part != null) {
         part.removeAttribute("class");
         part.removeAttribute("contenteditable");
-
-        if (removeIfEmpty && part.innerText.trim() == "") {
-            part.parentNode.removeChild(part);
-            part = null;
-        }
     }
     return part;
 };