This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.
The https://dom.spec.whatwg.org/#dom-treewalker-nextnode algorithm seems wrong to me. It only ever seems to descend to the first child of a node, or to its sibling, but never returns to a higher level. In contrast, its dual https://dom.spec.whatwg.org/#dom-treewalker-previousnode does "Set node to its parent." when no more siblings are found. This also would be consistent to the http://www.w3.org/TR/DOM-Level-2-Traversal-Range/traversal.html#Traversal-TreeWalker-nextNode, which says the method should "Moves the TreeWalker to the next visible node in document order relative to the current node". Btw, where did the http://www.w3.org/TR/DOM-Level-2-Traversal-Range/traversal.html#Traversal-overview sections go? They seemed quite helpful, what's the reason they were omitted from the current spec?
Doesn't step 3.2 handle the scenario you're talking about? Those sections disappeared because they'd have to be written from scratch and nobody has taken the time to do so thus far. If you're willing to write an introduction that'd be appreciated. I hope you accept my apologies for responding so late. We moved to GitHub a while back and I've been poor at addressing the remaining Bugzilla feedback.
> Doesn't step 3.2 handle the scenario you're talking about? No, that only seems to handle siblings. Or at least it looked like that, I don't know what exactly I thought a year ago :-) However, step 3.2 still seems buggy. I'm not exactly sure what "following" means here, but in either case: * If "following" refers to "following siblings", then the Treewalker `nextNode()` algorithm never steps upwards * If "following" is taken literally for the linked definition ("following in tree order"), it would include the siblings of the parent nodes, but it would also mean that any descendants of a node are following it and the phrase "not following root" doesn't make sense if we intend to walk the subtree of root. > We moved to GitHub a while back and I've been poor at addressing the remaining Bugzilla feedback. That's OK, if we should move this to Github for convenience I'll open an issue over there.
It's the second of those definitions for following, the one that is linked. But also, the definition of root here is the root of the subtree being walked. Not the root of the tree. So I think that should work, unless I'm missing something.
Let's take the following tree: html body div p p div (which is also the tree order from top to bottom). If we now create a TreeWalker on `body` (accepting any element nodes), * the first `nextNode()` call will set the current node to the first `div` (substep 1, subsubstep 3) * the second `nextNode()` call will set the current node to the first `p` (same steps) * the third `nextNode()` call will not go into substep 1, as the `p` has no child. In substep 2 it checks whether there's a node following the current node (both the second `p` and second `div` match this), and whether that node is **not following root** - but both of the candidates do follow `body`! So `null` is returned and the tree walker stops. So the phrase "not following root" needs to be replaced by "is a [descendant](https://dom.spec.whatwg.org/#concept-tree-descendant) of root".
(In reply to Bergi from comment #4) > So the phrase "not following root" needs to be replaced by "is a > [descendant](https://dom.spec.whatwg.org/#concept-tree-descendant) of root". But this also will wrong when we set currentNode to outside root (what is allowed and should continue travering). Looks like condition should consider two cases.
> when we set currentNode to outside root (what is allowed and should continue travering) Ah, didn't know that is valid, I thought it should find `null` in that case. To allow this, the condition would need to be `isDescendant(node, root) == isDescendant(candidate, root)`. Not sure how to best express that in prose.
Something like this perhaps: === If currentNode is a descendant of root, a node is following /node/, and /node/ is a descendant of root, then set /node/ to the first node following /node/. Otherwise, if currentNode is not a descendant of root, a node is following /node/, and /node is not a descendant of root, then set /node to the first node following /node/. Otherwise, return null. === Not sure if that can be shortened in prose.
By the way, how would like to appear in the acknowledgments?
(In reply to Anne from comment #7) > Otherwise, if currentNode is not a descendant of root, a node is following > /node/, and /node is not a descendant of root, then set /node to the first > node following /node/. "and /node/ is not a descendant of root" << it's needed here? Btw., I think for this two conditions you should cache currentNode before run any filter (it may even be in step 1.) because we can change currentNode inside filter and looks like it does not affect the current call nextNode().
I mean sth like this: 1. Let /node/ and /currentNode/ be the value of the currentNode attribute. For testing "following node" we can also use loop, but it's a bit longer than prose (replace all starting with 3.2 - after first loop): 2. Let /candidateNode/ and /matchedNode/ be null. 3. While /matchedNode/ is null and there is a node that is following /node/ then run these subsubsteps: 3.1. Set /candidateNode/ to that following node. 3.2. If /currentNode/ is not a descendant of root then set /node/ and /matchedNode/ to the /candidateNode/. 3.3. Otherwise, if /candidateNode/ is descendant of root then set /node/ and /matchedNode/ to the /candidateNode/. 3.4. Otherwise, set /node/ to the /candidateNode/. 4. If matchedNode is null then return null. 5. Filter /node/ and set /result/ to the return value. 6. If /result/ is FILTER_ACCEPT, then set the currentNode attribute to /node/ and return /node/. 7. Run these substeps again.
Moving this to https://github.com/whatwg/dom/pull/612. I'm still interested in how you'd like to be acknowledged Bergi, many years later. :-)