Debug assert in DOMSelection::containsNode when node belongs to a different tree
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Mar 2019 21:23:49 +0000 (21:23 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Mar 2019 21:23:49 +0000 (21:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196342

Reviewed by Antti Koivisto.

Source/WebCore:

The assertion was wrong. It's possible for Range::compareBoundaryPoints to return WRONG_DOCUMENT_ERR
when the node and the start container belong to two different trees.

Return false in such a case for now since it's unclear (unspecified) what these methods on Selection
should do with respect to shadow trees, preserving the current behavior of release builds.

Test: editing/selection/containsNode-with-no-common-ancestor.html

* page/DOMSelection.cpp:
(WebCore::DOMSelection::containsNode const):

LayoutTests:

Added a regression test to catch the debug assertion failure. The test always passed in release builds.

* editing/selection/containsNode-with-no-common-ancestor-expected.txt: Added.
* editing/selection/containsNode-with-no-common-ancestor.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@243621 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/editing/selection/containsNode-with-no-common-ancestor-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/containsNode-with-no-common-ancestor.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/DOMSelection.cpp

index 668504c..3d3e178 100644 (file)
@@ -1,3 +1,15 @@
+2019-03-28  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Debug assert in DOMSelection::containsNode when node belongs to a different tree
+        https://bugs.webkit.org/show_bug.cgi?id=196342
+
+        Reviewed by Antti Koivisto.
+
+        Added a regression test to catch the debug assertion failure. The test always passed in release builds.
+
+        * editing/selection/containsNode-with-no-common-ancestor-expected.txt: Added.
+        * editing/selection/containsNode-with-no-common-ancestor.html: Added.
+
 2019-03-28  Shawn Roberts  <sroberts@apple.com>
 
         http/wpt/cache-storage/quota-third-party.https.html is a flaky failure
diff --git a/LayoutTests/editing/selection/containsNode-with-no-common-ancestor-expected.txt b/LayoutTests/editing/selection/containsNode-with-no-common-ancestor-expected.txt
new file mode 100644 (file)
index 0000000..d66e26c
--- /dev/null
@@ -0,0 +1,11 @@
+This tests calling containsNode when the selection is set inside a shadow tree.
+WebKit should not hit a debug assertion
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS document.querySelector('input').select(); getSelection().containsNode(document.body) is false
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/editing/selection/containsNode-with-no-common-ancestor.html b/LayoutTests/editing/selection/containsNode-with-no-common-ancestor.html
new file mode 100644 (file)
index 0000000..d51e104
--- /dev/null
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<body>
+<input type="text" value="hello">
+<script src="../../resources/js-test.js"></script>
+<script>
+
+description(`This tests calling containsNode when the selection is set inside a shadow tree.<br>
+WebKit should not hit a debug assertion`);
+
+shouldBeFalse(`document.querySelector('input').select(); getSelection().containsNode(document.body)`);
+
+</script>
+</body>
+</html>
index 9860880..93ab661 100644 (file)
@@ -1,3 +1,21 @@
+2019-03-28  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Debug assert in DOMSelection::containsNode when node belongs to a different tree
+        https://bugs.webkit.org/show_bug.cgi?id=196342
+
+        Reviewed by Antti Koivisto.
+
+        The assertion was wrong. It's possible for Range::compareBoundaryPoints to return WRONG_DOCUMENT_ERR
+        when the node and the start container belong to two different trees.
+
+        Return false in such a case for now since it's unclear (unspecified) what these methods on Selection
+        should do with respect to shadow trees, preserving the current behavior of release builds.
+
+        Test: editing/selection/containsNode-with-no-common-ancestor.html
+
+        * page/DOMSelection.cpp:
+        (WebCore::DOMSelection::containsNode const):
+
 2019-03-28  Tim Horton  <timothy_horton@apple.com>
 
         Fix the !ENABLE(APPLE_PAY) build
index 5e067cf..475e64b 100644 (file)
@@ -415,7 +415,9 @@ bool DOMSelection::containsNode(Node& node, bool allowPartial) const
     unsigned nodeIndex = node.computeNodeIndex();
 
     auto startsResult = Range::compareBoundaryPoints(parentNode, nodeIndex, &selectedRange->startContainer(), selectedRange->startOffset());
-    ASSERT(!startsResult.hasException());
+    if (startsResult.hasException())
+        return false;
+
     auto endsResult = Range::compareBoundaryPoints(parentNode, nodeIndex + 1, &selectedRange->endContainer(), selectedRange->endOffset());
     ASSERT(!endsResult.hasException());
     bool isNodeFullySelected = !startsResult.hasException() && startsResult.releaseReturnValue() >= 0