- Revision
- 122537
- Author
- e...@webkit.org
- Date
- 2012-07-12 19:12:21 -0700 (Thu, 12 Jul 2012)
Log Message
Incorrect behaviour calling Range setStart or setEnd with boundary in different document
https://bugs.webkit.org/show_bug.cgi?id=42517
Reviewed by Ojan Vafai.
Source/WebCore:
Added a new static inline "checkForDifferentRootContainer" to share some code
and made setStart/setEnd do the right thing in the x-document case. I removed
the bogus checks in set*After/set*Before functions, and since they just call
through to setStart/setEnd, they also now do the right thing.
Test: fast/dom/Range/set-wrong-document-err.html
* dom/Range.cpp:
(WebCore::checkForDifferentRootContainer):
(WebCore):
(WebCore::Range::setStart):
(WebCore::Range::setEnd):
(WebCore::Range::setStartAfter):
(WebCore::Range::setEndBefore):
(WebCore::Range::setEndAfter):
(WebCore::Range::setStartBefore):
LayoutTests:
Add a new test to cover this changed behavior, and correct a FIXME in an old test
which was documenting our incorrect behavior.
* fast/dom/Range/set-wrong-document-err-expected.txt: Added.
* fast/dom/Range/set-wrong-document-err.html: Added.
* fast/dom/move-nodes-across-documents.html:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (122536 => 122537)
--- trunk/LayoutTests/ChangeLog 2012-07-13 02:01:09 UTC (rev 122536)
+++ trunk/LayoutTests/ChangeLog 2012-07-13 02:12:21 UTC (rev 122537)
@@ -1,3 +1,17 @@
+2012-07-12 Eric Seidel <e...@webkit.org>
+
+ Incorrect behaviour calling Range setStart or setEnd with boundary in different document
+ https://bugs.webkit.org/show_bug.cgi?id=42517
+
+ Reviewed by Ojan Vafai.
+
+ Add a new test to cover this changed behavior, and correct a FIXME in an old test
+ which was documenting our incorrect behavior.
+
+ * fast/dom/Range/set-wrong-document-err-expected.txt: Added.
+ * fast/dom/Range/set-wrong-document-err.html: Added.
+ * fast/dom/move-nodes-across-documents.html:
+
2012-07-12 Konrad Piascik <kpias...@rim.com>
Web Inspector: Geolocation override
Added: trunk/LayoutTests/fast/dom/Range/set-wrong-document-err-expected.txt (0 => 122537)
--- trunk/LayoutTests/fast/dom/Range/set-wrong-document-err-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/Range/set-wrong-document-err-expected.txt 2012-07-13 02:12:21 UTC (rev 122537)
@@ -0,0 +1,27 @@
+Range set* functions should not throw WRONG_DOCUMENT_ERR.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS range.setStart(iframe.contentDocument.body, 0) did not throw exception.
+PASS range.startContainer is iframe.contentDocument.body
+PASS range.collapsed is true
+PASS range.setEnd(iframe.contentDocument.body, 0) did not throw exception.
+PASS range.startContainer is iframe.contentDocument.body
+PASS range.collapsed is true
+PASS range.setStartAfter(iframe.contentDocument.body.firstChild) did not throw exception.
+PASS range.startContainer is iframe.contentDocument.body
+PASS range.collapsed is true
+PASS range.setStartBefore(iframe.contentDocument.body.firstChild) did not throw exception.
+PASS range.startContainer is iframe.contentDocument.body
+PASS range.collapsed is true
+PASS range.setEndAfter(iframe.contentDocument.body.firstChild) did not throw exception.
+PASS range.startContainer is iframe.contentDocument.body
+PASS range.collapsed is true
+PASS range.setEndBefore(iframe.contentDocument.body.firstChild) did not throw exception.
+PASS range.startContainer is iframe.contentDocument.body
+PASS range.collapsed is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/Range/set-wrong-document-err.html (0 => 122537)
--- trunk/LayoutTests/fast/dom/Range/set-wrong-document-err.html (rev 0)
+++ trunk/LayoutTests/fast/dom/Range/set-wrong-document-err.html 2012-07-13 02:12:21 UTC (rev 122537)
@@ -0,0 +1,54 @@
+<!DOCTYPE html>
+<body>
+<script src=""
+<script>
+
+function newRange() {
+ var range = document.createRange();
+ range.selectNodeContents(iframe);
+ return range;
+}
+
+function checkRange() {
+ shouldBe("range.startContainer", "iframe.contentDocument.body");
+ shouldBeTrue("range.collapsed");
+}
+
+description("Range set* functions should not throw WRONG_DOCUMENT_ERR.");
+window.iframe = document.createElement("iframe");
+document.body.appendChild(iframe);
+iframe.contentDocument.write("<html><head><body>content</body></html>");
+
+// Move range start to the iframe document. According to the DOM
+// Range spec, this should collapse the Range to the new boundary.
+// http://www.w3.org/TR/DOM-Level-2-Traversal-Range/ranges.html#Level-2-Range-Changing
+// http://www.w3.org/TR/dom/#interface-range
+
+window.range = newRange();
+shouldNotThrow("range.setStart(iframe.contentDocument.body, 0)");
+checkRange();
+
+window.range = newRange();
+shouldNotThrow("range.setEnd(iframe.contentDocument.body, 0)");
+checkRange();
+
+window.range = newRange();
+shouldNotThrow("range.setStartAfter(iframe.contentDocument.body.firstChild)");
+checkRange();
+
+window.range = newRange();
+shouldNotThrow("range.setStartBefore(iframe.contentDocument.body.firstChild)");
+checkRange();
+
+window.range = newRange();
+shouldNotThrow("range.setEndAfter(iframe.contentDocument.body.firstChild)");
+checkRange();
+
+window.range = newRange();
+shouldNotThrow("range.setEndBefore(iframe.contentDocument.body.firstChild)");
+checkRange();
+
+iframe.parentNode.removeChild(iframe);
+</script>
+<script src=""
+</body>
Modified: trunk/LayoutTests/fast/dom/move-nodes-across-documents.html (122536 => 122537)
--- trunk/LayoutTests/fast/dom/move-nodes-across-documents.html 2012-07-13 02:01:09 UTC (rev 122536)
+++ trunk/LayoutTests/fast/dom/move-nodes-across-documents.html 2012-07-13 02:12:21 UTC (rev 122537)
@@ -181,27 +181,26 @@
console.log(document.doctype);
}, 'HIERARCHY_REQUEST_ERR');
- // FIXME: This doesn't match http://www.w3.org/TR/DOM-Level-2-Traversal-Range/ranges.html#Level-2-Range-Changing.
- // Gecko implements it correctly. When setting a boundary of the range in a different
+ // When setting a boundary of the range in a different
// document, the call should succeed and the range should be collapsed.
runTest(function() {
rangeInIframe().setStart(elementInCurrentDocument('setStart'), 0);
- }, 'WRONG_DOCUMENT_ERR');
+ });
runTest(function() {
rangeInIframe().setEnd(elementInCurrentDocument('setEnd'), 0);
- }, 'WRONG_DOCUMENT_ERR');
+ });
runTest(function() {
rangeInIframe().setStartBefore(elementInCurrentDocument('setStartBefore'), 0);
- }, 'WRONG_DOCUMENT_ERR');
+ });
runTest(function() {
rangeInIframe().setStartAfter(elementInCurrentDocument('setStartAfter'), 0);
- }, 'WRONG_DOCUMENT_ERR');
+ });
runTest(function() {
rangeInIframe().setEndBefore(elementInCurrentDocument('setEndBefore'), 0);
- }, 'WRONG_DOCUMENT_ERR');
+ });
runTest(function() {
rangeInIframe().setEndAfter(elementInCurrentDocument('setEndAfter'), 0);
- }, 'WRONG_DOCUMENT_ERR');
+ });
// FIXME: isPointInRange isn't specced anywhere, but Gecko doesn't throw an exception here.
// It doesn't seem like we should either. Gecko returns false.
Modified: trunk/Source/WebCore/ChangeLog (122536 => 122537)
--- trunk/Source/WebCore/ChangeLog 2012-07-13 02:01:09 UTC (rev 122536)
+++ trunk/Source/WebCore/ChangeLog 2012-07-13 02:12:21 UTC (rev 122537)
@@ -1,3 +1,27 @@
+2012-07-12 Eric Seidel <e...@webkit.org>
+
+ Incorrect behaviour calling Range setStart or setEnd with boundary in different document
+ https://bugs.webkit.org/show_bug.cgi?id=42517
+
+ Reviewed by Ojan Vafai.
+
+ Added a new static inline "checkForDifferentRootContainer" to share some code
+ and made setStart/setEnd do the right thing in the x-document case. I removed
+ the bogus checks in set*After/set*Before functions, and since they just call
+ through to setStart/setEnd, they also now do the right thing.
+
+ Test: fast/dom/Range/set-wrong-document-err.html
+
+ * dom/Range.cpp:
+ (WebCore::checkForDifferentRootContainer):
+ (WebCore):
+ (WebCore::Range::setStart):
+ (WebCore::Range::setEnd):
+ (WebCore::Range::setStartAfter):
+ (WebCore::Range::setEndBefore):
+ (WebCore::Range::setEndAfter):
+ (WebCore::Range::setStartBefore):
+
2012-07-12 Erik Arvidsson <a...@chromium.org>
[V8] Simplify CodeGeneratorV8 since V8OnProto is only true for DOMWindow
Modified: trunk/Source/WebCore/dom/Range.cpp (122536 => 122537)
--- trunk/Source/WebCore/dom/Range.cpp 2012-07-13 02:01:09 UTC (rev 122536)
+++ trunk/Source/WebCore/dom/Range.cpp 2012-07-13 02:12:21 UTC (rev 122537)
@@ -196,6 +196,18 @@
return m_start == m_end;
}
+static inline bool checkForDifferentRootContainer(const RangeBoundaryPoint& start, const RangeBoundaryPoint& end)
+{
+ Node* endRootContainer = end.container();
+ while (endRootContainer->parentNode())
+ endRootContainer = endRootContainer->parentNode();
+ Node* startRootContainer = start.container();
+ while (startRootContainer->parentNode())
+ startRootContainer = startRootContainer->parentNode();
+
+ return startRootContainer != endRootContainer || (Range::compareBoundaryPoints(start, end, ASSERT_NO_EXCEPTION) > 0);
+}
+
void Range::setStart(PassRefPtr<Node> refNode, int offset, ExceptionCode& ec)
{
if (!m_start.container()) {
@@ -208,9 +220,10 @@
return;
}
+ bool didMoveDocument = false;
if (refNode->document() != m_ownerDocument) {
- ec = WRONG_DOCUMENT_ERR;
- return;
+ setDocument(refNode->document());
+ didMoveDocument = true;
}
ec = 0;
@@ -220,20 +233,8 @@
m_start.set(refNode, offset, childNode);
- // check if different root container
- Node* endRootContainer = m_end.container();
- while (endRootContainer->parentNode())
- endRootContainer = endRootContainer->parentNode();
- Node* startRootContainer = m_start.container();
- while (startRootContainer->parentNode())
- startRootContainer = startRootContainer->parentNode();
- if (startRootContainer != endRootContainer)
+ if (didMoveDocument || checkForDifferentRootContainer(m_start, m_end))
collapse(true, ec);
- // check if new start after end
- else if (compareBoundaryPoints(m_start, m_end, ec) > 0) {
- ASSERT(!ec);
- collapse(true, ec);
- }
}
void Range::setEnd(PassRefPtr<Node> refNode, int offset, ExceptionCode& ec)
@@ -248,9 +249,10 @@
return;
}
+ bool didMoveDocument = false;
if (refNode->document() != m_ownerDocument) {
- ec = WRONG_DOCUMENT_ERR;
- return;
+ setDocument(refNode->document());
+ didMoveDocument = true;
}
ec = 0;
@@ -260,20 +262,8 @@
m_end.set(refNode, offset, childNode);
- // check if different root container
- Node* endRootContainer = m_end.container();
- while (endRootContainer->parentNode())
- endRootContainer = endRootContainer->parentNode();
- Node* startRootContainer = m_start.container();
- while (startRootContainer->parentNode())
- startRootContainer = startRootContainer->parentNode();
- if (startRootContainer != endRootContainer)
+ if (didMoveDocument || checkForDifferentRootContainer(m_start, m_end))
collapse(false, ec);
- // check if new end before start
- if (compareBoundaryPoints(m_start, m_end, ec) > 0) {
- ASSERT(!ec);
- collapse(false, ec);
- }
}
void Range::setStart(const Position& start, ExceptionCode& ec)
@@ -1253,11 +1243,6 @@
return;
}
- if (refNode->document() != m_ownerDocument) {
- ec = WRONG_DOCUMENT_ERR;
- return;
- }
-
ec = 0;
checkNodeBA(refNode, ec);
if (ec)
@@ -1278,11 +1263,6 @@
return;
}
- if (refNode->document() != m_ownerDocument) {
- ec = WRONG_DOCUMENT_ERR;
- return;
- }
-
ec = 0;
checkNodeBA(refNode, ec);
if (ec)
@@ -1303,18 +1283,12 @@
return;
}
- if (refNode->document() != m_ownerDocument) {
- ec = WRONG_DOCUMENT_ERR;
- return;
- }
-
ec = 0;
checkNodeBA(refNode, ec);
if (ec)
return;
setEnd(refNode->parentNode(), refNode->nodeIndex() + 1, ec);
-
}
void Range::selectNode(Node* refNode, ExceptionCode& ec)
@@ -1529,11 +1503,6 @@
return;
}
- if (refNode->document() != m_ownerDocument) {
- ec = WRONG_DOCUMENT_ERR;
- return;
- }
-
ec = 0;
checkNodeBA(refNode, ec);
if (ec)
