TreeWalker change to forbid leaving the subtree (bug 16758) regressed us on Acid3

[Eric Seidel (no email)]

I think bug 16758 regressed us on the Acid3

We added a failure:
Test 10: FAIL (failed to handle regrafting correctly)

    function () {
      // test 10: walking outside a tree
      var doc = getTestDocument();
      var p = doc.createElement('p');
      doc.body.appendChild(p);
      var b = doc.body;
      var w = document.createTreeWalker(b, 0xFFFFFFFF, null, true);
      assert(w.currentNode == b, "basic use of TreeWalker failed: currentNode");
      assert(w.lastChild() == p, "basic use of TreeWalker failed: lastChild()");
      assert(w.previousNode() == b, "basic use of TreeWalker failed: previousNode()");
      doc.documentElement.removeChild(b);
      assert(w.lastChild() == p, "TreeWalker failed after removing the current node from the tree");
      assert(w.nextNode() == null, "failed to walk into the end of a subtree");
      doc.documentElement.appendChild(p);
      assert(w.previousNode() == doc.getElementsByTagName('title')[0], "failed to handle regrafting correctly");
      p.appendChild(b);
      assert(w.nextNode() == p, "couldn't retrace steps");
      assert(w.nextNode() == b, "couldn't step back into root");
      assert(w.previousNode() == null, "root didn't retake its rootish position");
      return 1;
    },

[Sam Weinig]

I believe I introduced this issue by explicitly checking that the returned node of the TreeWalker methods isDescendantOf(rootNode).  Therefore, if the rootNode is removed and currentNode is re-inserted, this check will always fail and cause null to be returned.  I am not sure what the correct behavior is however.

[Ian 'Hixie' Hickson]

TreeWalkers are exclusively dependent on their currentNode (unlike Ranges, which are dependent just on the Document/DocumentFragment/Attr they were originally bound to). You can TreeWalk right from one Document to another, if the Node in question is adopt()ed in between, for instance. You can similarly keep walking even if the node is removed from the root and made an orphan, and keep walking if the node is grafted above it, or whatever.

[Sam Weinig]

Created attachment 18306 [details]
fix

[Eric Seidel (no email)]

Comment on attachment 18306 [details]
fix

Sam said he would post a few more tests.

For example:
- what about walking between documents (adopting a node during the walk)
- what about tree manipulations during the walk?
- should firstChild/lastChild/or previousSibling ever be able to walk "past" the root?

It's really the test which I care about more than the patch.  The patch looks fine.  The question is just if we have and are testing the correct behavior.

[Eric Seidel (no email)]

Comment on attachment 18306 [details]
fix

r- until the additional test cases are posted.  We could land this, but it's best to wait.

[Darin Adler]

My patch in bug 3492 fixes this. I didn't find this bug because I searched for TreeWalker in bug titles only.

[Darin Adler]

I think this is effectively a duplicate of bug 3492.

*** This bug has been marked as a duplicate of 3492 ***

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.