From 0f554d23712277893f5173ab3033b0cb0169effd Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sat, 2 Feb 2019 14:48:22 +1100 Subject: [PATCH] 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 { 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 { ); } + public void is_descendant_of_lax() throws GLib.Error { + load_body_fixture("
ohhai
"); + 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; }