Log Message
Range.comparePoint shouldn't throw an exception if the range and the node are in the same detached tree https://bugs.webkit.org/show_bug.cgi?id=148733
Reviewed by Chris Dumez. Source/WebCore: Don't throw WRONG_DOCUMENT_ERR when refNode is not in the document. The new behavior matches DOM4 as well as the behavior of Firefox. See https://dom.spec.whatwg.org/#dom-range-comparepoint WRONG_DOCUMENT_ERR is still thrown by compareBoundaryPoints later in the function when the root nodes of refNode and boundary points are different. There is one subtlety here that we need to throw WRONG_DOCUMENT_ERR instead of INDEX_SIZE_ERR when refNode and the boundary points don't share the same root node even if (refNode, offset) pair is invalid since DOM4 spec checks the former condition first. We implement this behavior by first validating the offset and then explicitly checking for the root node difference if the former check failed to avoid the latter O(n) check in common cases. Test: fast/dom/Range/range-comparePoint-detached-nodes.html * dom/Range.cpp: (WebCore::Range::comparePoint): LayoutTests: Added a regression test and rebaselined a W3C test with more test cases passing. * fast/dom/Range/range-comparePoint-detached-nodes-expected.txt: Added. * fast/dom/Range/range-comparePoint-detached-nodes.html: Added. * http/tests/w3c/dom/ranges/Range-set-expected.txt:
Modified Paths
- trunk/LayoutTests/ChangeLog
- trunk/LayoutTests/http/tests/w3c/dom/ranges/Range-set-expected.txt
- trunk/Source/WebCore/ChangeLog
- trunk/Source/WebCore/dom/Range.cpp
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (189326 => 189327)
--- trunk/LayoutTests/ChangeLog 2015-09-04 00:44:37 UTC (rev 189326)
+++ trunk/LayoutTests/ChangeLog 2015-09-04 00:48:26 UTC (rev 189327)
@@ -1,3 +1,16 @@
+2015-09-03 Ryosuke Niwa <[email protected]>
+
+ Range.comparePoint shouldn't throw an exception if the range and the node are in the same detached tree
+ https://bugs.webkit.org/show_bug.cgi?id=148733
+
+ Reviewed by Chris Dumez.
+
+ Added a regression test and rebaselined a W3C test with more test cases passing.
+
+ * fast/dom/Range/range-comparePoint-detached-nodes-expected.txt: Added.
+ * fast/dom/Range/range-comparePoint-detached-nodes.html: Added.
+ * http/tests/w3c/dom/ranges/Range-set-expected.txt:
+
2015-09-03 Tim Horton <[email protected]>
Add a test for swipe-start hysteresis
Added: trunk/LayoutTests/fast/dom/Range/range-comparePoint-detached-nodes-expected.txt (0 => 189327)
--- trunk/LayoutTests/fast/dom/Range/range-comparePoint-detached-nodes-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/Range/range-comparePoint-detached-nodes-expected.txt 2015-09-04 00:48:26 UTC (rev 189327)
@@ -0,0 +1,17 @@
+This test calls comparePoint on detached nodes. They should compare nodes when they're in the same detached tree and throw otherwise.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS var range = new Range; range.setStart(detachedDiv, 0); range.comparePoint(detachedDiv, 0); is 0
+PASS var range = new Range; range.setStart(detachedDiv, 0); range.comparePoint(detachedDiv, 1); is 1
+PASS var range = new Range; range.setStart(detachedDiv, 2); range.comparePoint(detachedDiv, 1); is -1
+PASS var range = new Range; range.setStart(spanInDetachedDiv1, 0); range.comparePoint(spanInDetachedDiv2, 0); is 1
+PASS var range = new Range; range.setStart(spanInDetachedDiv2, 0); range.comparePoint(spanInDetachedDiv1, 0); is -1
+PASS var range = new Range; range.setStart(spanInDetachedDiv1, 0); range.comparePoint(spanInDetachedDiv1, 0); is 0
+PASS var range = new Range; range.setStart(detachedDiv, 0); range.comparePoint(detachedP, 0); threw exception Error: WrongDocumentError: DOM Exception 4.
+PASS var range = new Range; range.setStart(spanInDetachedDiv1, 0); range.comparePoint(spanInDetachedP, 0); threw exception Error: WrongDocumentError: DOM Exception 4.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/Range/range-comparePoint-detached-nodes.html (0 => 189327)
--- trunk/LayoutTests/fast/dom/Range/range-comparePoint-detached-nodes.html (rev 0)
+++ trunk/LayoutTests/fast/dom/Range/range-comparePoint-detached-nodes.html 2015-09-04 00:48:26 UTC (rev 189327)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+
+description("This test calls comparePoint on detached nodes. They should compare nodes when they're in the same detached tree and throw otherwise.")
+
+var detachedDiv = document.createElement('div');
+var spanInDetachedDiv1 = document.createElement('span');
+var spanInDetachedDiv2 = document.createElement('span');
+detachedDiv.appendChild(spanInDetachedDiv1);
+detachedDiv.appendChild(spanInDetachedDiv2);
+
+var detachedP = document.createElement('p');
+var spanInDetachedP = document.createElement('span');
+detachedP.appendChild(spanInDetachedP);
+
+shouldBe("var range = new Range; range.setStart(detachedDiv, 0); range.comparePoint(detachedDiv, 0);", "0");
+shouldBe("var range = new Range; range.setStart(detachedDiv, 0); range.comparePoint(detachedDiv, 1);", "1");
+shouldBe("var range = new Range; range.setStart(detachedDiv, 2); range.comparePoint(detachedDiv, 1);", "-1");
+shouldBe("var range = new Range; range.setStart(spanInDetachedDiv1, 0); range.comparePoint(spanInDetachedDiv2, 0);", "1");
+shouldBe("var range = new Range; range.setStart(spanInDetachedDiv2, 0); range.comparePoint(spanInDetachedDiv1, 0);", "-1");
+shouldBe("var range = new Range; range.setStart(spanInDetachedDiv1, 0); range.comparePoint(spanInDetachedDiv1, 0);", "0");
+shouldThrow("var range = new Range; range.setStart(detachedDiv, 0); range.comparePoint(detachedP, 0);", "'Error: WrongDocumentError: DOM Exception 4'");
+shouldThrow("var range = new Range; range.setStart(spanInDetachedDiv1, 0); range.comparePoint(spanInDetachedP, 0);", "'Error: WrongDocumentError: DOM Exception 4'");
+
+var successfullyParsed = true;
+
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/LayoutTests/http/tests/w3c/dom/ranges/Range-set-expected.txt (189326 => 189327)
--- trunk/LayoutTests/http/tests/w3c/dom/ranges/Range-set-expected.txt 2015-09-04 00:44:37 UTC (rev 189326)
+++ trunk/LayoutTests/http/tests/w3c/dom/ranges/Range-set-expected.txt 2015-09-04 00:48:26 UTC (rev 189327)
@@ -581,9 +581,9 @@
PASS setStart() with range 6 [detachedPara1.firstChild, 0, detachedPara1.firstChild, 0], point 13 [paras[1].firstChild, 9]
PASS setStart() with range 6 [detachedPara1.firstChild, 0, detachedPara1.firstChild, 0], point 14 [paras[1].firstChild, 10]
PASS setStart() with range 6 [detachedPara1.firstChild, 0, detachedPara1.firstChild, 0], point 15 [paras[1].firstChild, 65535]
-FAIL setStart() with range 6 [detachedPara1.firstChild, 0, detachedPara1.firstChild, 0], point 16 [detachedPara1.firstChild, 0] WrongDocumentError: DOM Exception 4
-FAIL setStart() with range 6 [detachedPara1.firstChild, 0, detachedPara1.firstChild, 0], point 17 [detachedPara1.firstChild, 1] WrongDocumentError: DOM Exception 4
-FAIL setStart() with range 6 [detachedPara1.firstChild, 0, detachedPara1.firstChild, 0], point 18 [detachedPara1.firstChild, 8] WrongDocumentError: DOM Exception 4
+PASS setStart() with range 6 [detachedPara1.firstChild, 0, detachedPara1.firstChild, 0], point 16 [detachedPara1.firstChild, 0]
+PASS setStart() with range 6 [detachedPara1.firstChild, 0, detachedPara1.firstChild, 0], point 17 [detachedPara1.firstChild, 1]
+PASS setStart() with range 6 [detachedPara1.firstChild, 0, detachedPara1.firstChild, 0], point 18 [detachedPara1.firstChild, 8]
PASS setStart() with range 6 [detachedPara1.firstChild, 0, detachedPara1.firstChild, 0], point 19 [detachedPara1.firstChild, 9]
PASS setStart() with range 6 [detachedPara1.firstChild, 0, detachedPara1.firstChild, 0], point 20 [foreignPara1.firstChild, 0]
PASS setStart() with range 6 [detachedPara1.firstChild, 0, detachedPara1.firstChild, 0], point 21 [foreignPara1.firstChild, 1]
@@ -669,9 +669,9 @@
PASS setStart() with range 7 [detachedPara1.firstChild, 2, detachedPara1.firstChild, 8], point 13 [paras[1].firstChild, 9]
PASS setStart() with range 7 [detachedPara1.firstChild, 2, detachedPara1.firstChild, 8], point 14 [paras[1].firstChild, 10]
PASS setStart() with range 7 [detachedPara1.firstChild, 2, detachedPara1.firstChild, 8], point 15 [paras[1].firstChild, 65535]
-FAIL setStart() with range 7 [detachedPara1.firstChild, 2, detachedPara1.firstChild, 8], point 16 [detachedPara1.firstChild, 0] WrongDocumentError: DOM Exception 4
-FAIL setStart() with range 7 [detachedPara1.firstChild, 2, detachedPara1.firstChild, 8], point 17 [detachedPara1.firstChild, 1] WrongDocumentError: DOM Exception 4
-FAIL setStart() with range 7 [detachedPara1.firstChild, 2, detachedPara1.firstChild, 8], point 18 [detachedPara1.firstChild, 8] WrongDocumentError: DOM Exception 4
+PASS setStart() with range 7 [detachedPara1.firstChild, 2, detachedPara1.firstChild, 8], point 16 [detachedPara1.firstChild, 0]
+PASS setStart() with range 7 [detachedPara1.firstChild, 2, detachedPara1.firstChild, 8], point 17 [detachedPara1.firstChild, 1]
+PASS setStart() with range 7 [detachedPara1.firstChild, 2, detachedPara1.firstChild, 8], point 18 [detachedPara1.firstChild, 8]
PASS setStart() with range 7 [detachedPara1.firstChild, 2, detachedPara1.firstChild, 8], point 19 [detachedPara1.firstChild, 9]
PASS setStart() with range 7 [detachedPara1.firstChild, 2, detachedPara1.firstChild, 8], point 20 [foreignPara1.firstChild, 0]
PASS setStart() with range 7 [detachedPara1.firstChild, 2, detachedPara1.firstChild, 8], point 21 [foreignPara1.firstChild, 1]
@@ -1549,9 +1549,9 @@
PASS setStart() with range 17 [detachedPara1, 0, detachedPara1, 1], point 13 [paras[1].firstChild, 9]
PASS setStart() with range 17 [detachedPara1, 0, detachedPara1, 1], point 14 [paras[1].firstChild, 10]
PASS setStart() with range 17 [detachedPara1, 0, detachedPara1, 1], point 15 [paras[1].firstChild, 65535]
-FAIL setStart() with range 17 [detachedPara1, 0, detachedPara1, 1], point 16 [detachedPara1.firstChild, 0] WrongDocumentError: DOM Exception 4
-FAIL setStart() with range 17 [detachedPara1, 0, detachedPara1, 1], point 17 [detachedPara1.firstChild, 1] WrongDocumentError: DOM Exception 4
-FAIL setStart() with range 17 [detachedPara1, 0, detachedPara1, 1], point 18 [detachedPara1.firstChild, 8] WrongDocumentError: DOM Exception 4
+PASS setStart() with range 17 [detachedPara1, 0, detachedPara1, 1], point 16 [detachedPara1.firstChild, 0]
+PASS setStart() with range 17 [detachedPara1, 0, detachedPara1, 1], point 17 [detachedPara1.firstChild, 1]
+PASS setStart() with range 17 [detachedPara1, 0, detachedPara1, 1], point 18 [detachedPara1.firstChild, 8]
PASS setStart() with range 17 [detachedPara1, 0, detachedPara1, 1], point 19 [detachedPara1.firstChild, 9]
PASS setStart() with range 17 [detachedPara1, 0, detachedPara1, 1], point 20 [foreignPara1.firstChild, 0]
PASS setStart() with range 17 [detachedPara1, 0, detachedPara1, 1], point 21 [foreignPara1.firstChild, 1]
Modified: trunk/Source/WebCore/ChangeLog (189326 => 189327)
--- trunk/Source/WebCore/ChangeLog 2015-09-04 00:44:37 UTC (rev 189326)
+++ trunk/Source/WebCore/ChangeLog 2015-09-04 00:48:26 UTC (rev 189327)
@@ -1,3 +1,27 @@
+2015-09-03 Ryosuke Niwa <[email protected]>
+
+ Range.comparePoint shouldn't throw an exception if the range and the node are in the same detached tree
+ https://bugs.webkit.org/show_bug.cgi?id=148733
+
+ Reviewed by Chris Dumez.
+
+ Don't throw WRONG_DOCUMENT_ERR when refNode is not in the document. The new behavior matches DOM4 as well
+ as the behavior of Firefox. See https://dom.spec.whatwg.org/#dom-range-comparepoint
+
+ WRONG_DOCUMENT_ERR is still thrown by compareBoundaryPoints later in the function when the root nodes of
+ refNode and boundary points are different.
+
+ There is one subtlety here that we need to throw WRONG_DOCUMENT_ERR instead of INDEX_SIZE_ERR when refNode
+ and the boundary points don't share the same root node even if (refNode, offset) pair is invalid since
+ DOM4 spec checks the former condition first. We implement this behavior by first validating the offset
+ and then explicitly checking for the root node difference if the former check failed to avoid the latter
+ O(n) check in common cases.
+
+ Test: fast/dom/Range/range-comparePoint-detached-nodes.html
+
+ * dom/Range.cpp:
+ (WebCore::Range::comparePoint):
+
2015-09-03 Jer Noble <[email protected]>
[iOS] Playback does not pause when deselecting route and locking screen.
Modified: trunk/Source/WebCore/dom/Range.cpp (189326 => 189327)
--- trunk/Source/WebCore/dom/Range.cpp 2015-09-04 00:44:37 UTC (rev 189326)
+++ trunk/Source/WebCore/dom/Range.cpp 2015-09-04 00:48:26 UTC (rev 189327)
@@ -253,15 +253,20 @@
return 0;
}
- if (!refNode->inDocument() || &refNode->document() != &ownerDocument()) {
+ if (&refNode->document() != &ownerDocument()) {
ec = WRONG_DOCUMENT_ERR;
return 0;
}
ec = 0;
checkNodeWOffset(refNode, offset, ec);
- if (ec)
+ if (ec) {
+ // DOM4 spec requires us to check whether refNode and start container have the same root first
+ // but we do it in the reverse order to avoid O(n) operation here in common case.
+ if (!refNode->inDocument() && !commonAncestorContainer(refNode, &startContainer()))
+ ec = WRONG_DOCUMENT_ERR;
return 0;
+ }
// compare to start, and point comes before
if (compareBoundaryPoints(refNode, offset, &startContainer(), m_start.offset(), ec) < 0)
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
