Title: [104866] trunk
- Revision
- 104866
- Author
- aba...@webkit.org
- Date
- 2012-01-12 15:18:36 -0800 (Thu, 12 Jan 2012)
Log Message
NodeIterator loses track of the reference node when the reference node is removed from the document (IETC ni_removeReferenceNode)
https://bugs.webkit.org/show_bug.cgi?id=76146
Reviewed by Eric Seidel.
Source/WebCore:
In the case where we're removing the reference node we can end up with
the wrong reference node. This patch makes sure we traverse outside of
the removed node's subtree.
This bug was caught by the following IE Test Center test:
http://samples.msdn.microsoft.com/ietestcenter/domtraversal/showdomtraversaltest.htm?ni_removeReferenceNode
Our new behavior also match Firefox.
I experimented a bit with adding ASSERT_NOT_REACHED to various branches
in NodeIterator::updateForNodeRemoval, and it seems our test coverage
for this function is relatively poor. In the future, we should
consider adding more tests for this complicated function.
Test: fast/dom/node-iterator-reference-node-removed.html
* dom/NodeIterator.cpp:
(WebCore::NodeIterator::updateForNodeRemoval):
LayoutTests:
Test based on http://samples.msdn.microsoft.com/ietestcenter/domtraversal/showdomtraversaltest.htm?ni_removeReferenceNode
* fast/dom/node-iterator-reference-node-removed-expected.txt: Added.
* fast/dom/node-iterator-reference-node-removed.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (104865 => 104866)
--- trunk/LayoutTests/ChangeLog 2012-01-12 23:12:29 UTC (rev 104865)
+++ trunk/LayoutTests/ChangeLog 2012-01-12 23:18:36 UTC (rev 104866)
@@ -1,3 +1,15 @@
+2012-01-12 Adam Barth <aba...@webkit.org>
+
+ NodeIterator loses track of the reference node when the reference node is removed from the document (IETC ni_removeReferenceNode)
+ https://bugs.webkit.org/show_bug.cgi?id=76146
+
+ Reviewed by Eric Seidel.
+
+ Test based on http://samples.msdn.microsoft.com/ietestcenter/domtraversal/showdomtraversaltest.htm?ni_removeReferenceNode
+
+ * fast/dom/node-iterator-reference-node-removed-expected.txt: Added.
+ * fast/dom/node-iterator-reference-node-removed.html: Added.
+
2012-01-12 Joshua Bell <jsb...@chromium.org>
IndexedDB: Throw exception if IDBCursor.continue() called with key equal to current
Added: trunk/LayoutTests/fast/dom/node-iterator-reference-node-removed-expected.txt (0 => 104866)
--- trunk/LayoutTests/fast/dom/node-iterator-reference-node-removed-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/node-iterator-reference-node-removed-expected.txt 2012-01-12 23:18:36 UTC (rev 104866)
@@ -0,0 +1,16 @@
+This test removes the NodeItertor's current reference node, then continues to move through the document.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS iter.nextNode() is testDiv
+PASS iter.nextNode() is div1
+PASS iter.nextNode() is div2
+PASS iter.nextNode() is null
+PASS iter.previousNode() is div2
+PASS iter.previousNode() is div1
+PASS iter.nextNode() is div2
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/dom/node-iterator-reference-node-removed.html (0 => 104866)
--- trunk/LayoutTests/fast/dom/node-iterator-reference-node-removed.html (rev 0)
+++ trunk/LayoutTests/fast/dom/node-iterator-reference-node-removed.html 2012-01-12 23:18:36 UTC (rev 104866)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<script src=""
+<div id="testDiv" style="display:none;">
+ <div id="div1">aaa</div>
+ <div id="div2">bbb</div>
+</div>
+<script>
+description("This test removes the NodeItertor's current reference node, then continues to move through the document.");
+
+var testDiv = document.getElementById("testDiv");
+var div1 = document.getElementById("div1");
+var div2 = document.getElementById("div2");
+var iter = document.createNodeIterator(testDiv, NodeFilter.SHOW_ELEMENT, null, false);
+
+shouldBe("iter.nextNode()", "testDiv");
+shouldBe("iter.nextNode()", "div1");
+shouldBe("iter.nextNode()", "div2");
+shouldBe("iter.nextNode()", "null");
+shouldBe("iter.previousNode()", "div2");
+shouldBe("iter.previousNode()", "div1");
+testDiv.removeChild(div1);
+shouldBe("iter.nextNode()", "div2");
+</script>
+<script src=""
Modified: trunk/Source/WebCore/ChangeLog (104865 => 104866)
--- trunk/Source/WebCore/ChangeLog 2012-01-12 23:12:29 UTC (rev 104865)
+++ trunk/Source/WebCore/ChangeLog 2012-01-12 23:18:36 UTC (rev 104866)
@@ -1,3 +1,30 @@
+2012-01-12 Adam Barth <aba...@webkit.org>
+
+ NodeIterator loses track of the reference node when the reference node is removed from the document (IETC ni_removeReferenceNode)
+ https://bugs.webkit.org/show_bug.cgi?id=76146
+
+ Reviewed by Eric Seidel.
+
+ In the case where we're removing the reference node we can end up with
+ the wrong reference node. This patch makes sure we traverse outside of
+ the removed node's subtree.
+
+ This bug was caught by the following IE Test Center test:
+
+ http://samples.msdn.microsoft.com/ietestcenter/domtraversal/showdomtraversaltest.htm?ni_removeReferenceNode
+
+ Our new behavior also match Firefox.
+
+ I experimented a bit with adding ASSERT_NOT_REACHED to various branches
+ in NodeIterator::updateForNodeRemoval, and it seems our test coverage
+ for this function is relatively poor. In the future, we should
+ consider adding more tests for this complicated function.
+
+ Test: fast/dom/node-iterator-reference-node-removed.html
+
+ * dom/NodeIterator.cpp:
+ (WebCore::NodeIterator::updateForNodeRemoval):
+
2012-01-12 Joshua Bell <jsb...@chromium.org>
IndexedDB: Throw exception if IDBCursor.continue() called with key equal to current
Modified: trunk/Source/WebCore/dom/NodeIterator.cpp (104865 => 104866)
--- trunk/Source/WebCore/dom/NodeIterator.cpp 2012-01-12 23:12:29 UTC (rev 104865)
+++ trunk/Source/WebCore/dom/NodeIterator.cpp 2012-01-12 23:18:36 UTC (rev 104866)
@@ -179,12 +179,10 @@
if (referenceNode.isPointerBeforeNode) {
Node* node = removedNode->traverseNextNode(root());
if (node) {
- // Move out from under the node being removed if the reference node is
- // a descendant of the node being removed.
- if (willRemoveReferenceNodeAncestor) {
- while (node && node->isDescendantOf(removedNode))
- node = node->traverseNextNode(root());
- }
+ // Move out from under the node being removed if the new reference
+ // node is a descendant of the node being removed.
+ while (node && node->isDescendantOf(removedNode))
+ node = node->traverseNextNode(root());
if (node)
referenceNode.node = node;
} else {
@@ -217,6 +215,7 @@
if (node)
referenceNode.node = node;
} else {
+ // FIXME: This branch doesn't appear to have any LayoutTests.
node = removedNode->traverseNextNode(root());
// Move out from under the node being removed if the reference node is
// a descendant of the node being removed.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
