[geary/wip/geary-is-descendant-of] Fix select-quoting not preserving newlines in some cases



commit f3b51ce349101e216e86de1c83235010f1dbfa3c
Author: Michael Gratton <mike vee net>
Date:   Sat Feb 2 14:48:22 2019 +1100

    Fix select-quoting not preserving newlines in some cases
    
    If the common ancestor of the quoted text is the plain-text-message DIV
    itself, the isDescendant test fails and the style to preserve new lines
    is not maintained. This adds a non-strict check to isDescendant and
    enables that when checking the common ancestor node and a test case for
    it.

 test/js/conversation-page-state-test.vala | 14 ++++++++++++++
 ui/conversation-web-view.js               | 29 ++++++++++++++++++-----------
 2 files changed, 32 insertions(+), 11 deletions(-)
---
diff --git a/test/js/conversation-page-state-test.vala b/test/js/conversation-page-state-test.vala
index 7d49eedb..fc680fff 100644
--- a/test/js/conversation-page-state-test.vala
+++ b/test/js/conversation-page-state-test.vala
@@ -20,6 +20,7 @@ class ConversationPageStateTest : ClientWebViewTestCase<ConversationWebView> {
         add_test("is_descendant_of", is_descendant_of);
         add_test("is_descendant_of_with_class", is_descendant_of_with_class);
         add_test("is_descendant_of_no_match", is_descendant_of_no_match);
+        add_test("is_descendant_of_lax", is_descendant_of_lax);
 
         try {
             ConversationWebView.load_resources(File.new_for_path(""));
@@ -115,6 +116,19 @@ class ConversationPageStateTest : ClientWebViewTestCase<ConversationWebView> {
         );
     }
 
+    public void is_descendant_of_lax() throws GLib.Error {
+        load_body_fixture("<blockquote class='test-class'><div id='test'>ohhai</div></blockquote>");
+        assert(
+            WebKitUtil.to_bool(
+                run_javascript("""
+                    ConversationPageState.isDescendantOf(
+                        document.getElementById('test'), "DIV", null, false
+                    );
+                """)
+           )
+        );
+    }
+
 
     protected override ConversationWebView set_up_test_view() {
         return new ConversationWebView(this.config);
diff --git a/ui/conversation-web-view.js b/ui/conversation-web-view.js
index fb6d9aca..ef209223 100644
--- a/ui/conversation-web-view.js
+++ b/ui/conversation-web-view.js
@@ -201,12 +201,15 @@ ConversationPageState.prototype = {
                 ancestor = ancestor.parentNode;
             }
 
-            // If the selection is part of a plain text message,
-            // we have to stick it in an appropriately styled div,
-            // so that new lines are preserved.
+            // If the selection is part of a plain text message, we
+            // have to stick it in an appropriately styled div, so
+            // that new lines are preserved. Do a non-strict ancestor
+            // check since the common ancestor may well be the plain
+            // text DIV itself
             let dummy = document.createElement("DIV");
             let includeDummy = false;
-            if (ConversationPageState.isDescendantOf(ancestor, "DIV", "plaintext")) {
+            if (ConversationPageState.isDescendantOf(
+                ancestor, "DIV", "plaintext", false)) {
                 dummy.classList.add("plaintext");
                 dummy.setAttribute("style", "white-space: pre-wrap;");
                 includeDummy = true;
@@ -324,17 +327,21 @@ ConversationPageState.isDeceptiveText = function(text, href) {
 /**
  * See if this element has an ancestor with the given tag and class.
  *
- * ancestorTag must be all uppercase.
+ * The value of ancestorTag must be all uppercase.
  *
  * If ancestorClass is null, no class checking is done.
+ * If strict is is true, the given node will not be checked.
  */
-ConversationPageState.isDescendantOf = function(node, ancestorTag, ancestorClass = null) {
-    let ancestor = node.parentNode;
+ConversationPageState.isDescendantOf = function(node,
+                                                ancestorTag,
+                                                ancestorClass = null,
+                                                strict = true) {
+    let ancestor = strict ? node.parentNode : node;
     while (ancestor != null) {
-        if (ancestor.nodeName.toUpperCase() == ancestorTag) {
-            if (!ancestorClass || ancestor.classList.contains(ancestorClass)) {
-                return true;
-            }
+        if (ancestor.nodeName.toUpperCase() == ancestorTag &&
+            (ancestorClass == null ||
+             ancestor.classList.contains(ancestorClass))) {
+            return true;
         }
         ancestor = ancestor.parentNode;
     }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]