Some tweaks for traverse algo and others

[ArkadiuszMichalski]

trafficstars

[Edit: Behaviour for traverse is correct, so I I left only fixes] https://dom.spec.whatwg.org/#concept-NodeIterator-traverse And a few comments on this algorithm (all for 3.1 step):

• "If direction..." direction is passing argument, then can be treat as variable
• iterator collection was defined so use reference. BTW, it's collection (rooted at root, whose filter matches any node), but for iterator sense root should not be included to this collection (as first element)?
• having three /if/ conditions writing in one line (separate by dot) is quite readable, especially when first cooperate with second, and third is opposite to first. Something like we have here http://www.w3.org/TR/dom/#concept-nodeiterator-traverse is better.

[ArkadiuszMichalski]

nextNode() https://dom.spec.whatwg.org/#dom-treewalker-nextnode 3.2 If a node is following node and is not... < first node is unclear, maybe write: 3.2. If there is a node which/that is following node and is not...

pre-removing steps https://dom.spec.whatwg.org/#nodeiterator-pre-removing-steps We have access to /nodeIterator/ but sometimes missing them as context: nodeIterator's referenceNode (step 1.) nodeIterator's pointerBeforeReferenceNode (step 2.)

[triple-underscore]

I think the step 3.2 of nextNode() ( “ If a node is following node and is not following root, ...” ) also needs an addtional conditions such like: “, and node is not a descendant of node if result is FILTER_REJECT”.

[annevk]

BTW, it's collection (rooted at root, whose filter matches any node), but for iterator sense root should not be included to this collection (as first element)?

I don't quite understand this comment.

[annevk]

@triple-underscore do you have a test for that?

[triple-underscore]

do you have a test for that?

Consider the following DOM and script:

<div id="root"><div id="n1"><div id="n2"><div id="n3"></div></div></div></div>

<script>
const walker = document.createTreeWalker(
  document.getElementById('root'),
  NodeFilter.SHOW_ELEMENT,
  (e) => {
    return e.id === 'n2' ?
      NodeFilter.FILTER_REJECT : NodeFilter.FILTER_ACCEPT;
  }
);
let node;
while(node = walker.nextNode()){
  console.log(node.id);
}
</script>

Tracing the TreeWalker.nextNode() method in the current spec, the above nextNode() would return n1, then node n3; but UAs return only n1 (tested in FireFox, Chrome).

• The 1st call of nextNode(): (currentNode is 'root'.)

1. In the loop 3.1: Returns node n1, with filtering result FILTER_ACCEPT. (currentNode becomes n1.)

• The 2nd call of nextNode():

1. In the loop 3.1: Breaks at the node n2, with filtering result FILTER_REJECT. The node variable is set to node n2.
2. step 3.2: node variable is set to n2's first chlid, which is the node n3. (This does not seem to match the actual behavior.)
3. step 3.3, 3.4: The result of filtering n3 is FILTER_ACCEPT, and returns n3.

I think the step 3.2 is also contrary to the semantics of FILTER_REJECT, “reject all inclusive descendants”.

[ArkadiuszMichalski]

@annevk

BTW, it's collection (rooted at root, whose filter matches any node), but for iterator sense root should not be included to this collection (as first element)?

I don't quite understand this comment.

"Each NodeIterator object has an associated iterator collection, which is a collection rooted at the NodeIterator object’s root, whose filter matches any node."

I don't understand what "rooted" means here. There is no any description for that term. For collection we see: "A collection is an object that represents a list of nodes...."

From that prose I don't know if NodeIterator object’s root is part of iterator collection or not. It's important for nextNode() and previousNode(). It looks like it's part of the collection (in 0 idx) because we finally get this root, but it should be written directly. It is possible that it is not clear to me only :)

Btw: "The nextNode() method, when invoked, must return the result of traversing with this and next." << it realy hard read prose mixed with code, where the code doesn't have some distinction, like here for "next". This "next" jump to algorithm whera also "next" looks like normal text (not here because here has bold, but I saw other algorithms were passing value has no any style).

[annevk]

https://dom.spec.whatwg.org/#concept-collection does mention it consists of a root and a filter. I agree that we could make all that more explicit though.

As for "next", I'd support turning that into a string enumerated value or some such, e.g., "next".

[ArkadiuszMichalski]

I check this "rooted" term in various collection actions and:

1. In DOM for all collections is mention what its root (by sth like this: "rooted on this" or "object associated with them" in HTML spec). But in HTML sometimes it's missing: https://html.spec.whatwg.org/multipage/dom.html#dom-document-getelementsbyname (I don't check all cases).
2. Looks like root is only associated to collection (like filter) and later in prose was describe what is to be returned (descendants / children, with/without root, etc).

So it's basically correct, no need to add anything, everything comes from prose.

Looks like this bug can be close, all was fixed. Adding style for values should be cover by another bug ("next" will be better than the current next). << it can even be seen in this sentence.

Edit: wait, for nextNode() return root follows from the algo, but for previousNode() I can't determine it (for example if we make some next... and previous... why we still get root?).

[annevk]

I'm not sure I understand the question. The root is associated with the collection, so why can it not be determined? It doesn't change.

[ArkadiuszMichalski]

"A collection is an object that represents a list of nodes"

"When a collection is created, a filter and a root are associated with it."

"The collection then represents a view of the subtree rooted at the collection’s root, containing only nodes that match the given filter." << root can be part a collection or not?

"Each NodeIterator object has an associated iterator collection, which is a collection rooted at the NodeIterator object’s root, whose filter matches any node." << ok, so this mean that root is also part the collection?

Some information about this would be more cleare (even in green box).

[annevk]

Yeah, agreed that we should have better definitions for collections. Let's use this issue to keep track of that. If anyone is interested in working on this, that'd be appreciated.