fix(js): descend on FILTER_SKIP in TreeWalker child/sibling movers (#469)

firstChild/lastChild/nextSibling/previousSibling stepped straight to the
next sibling whenever a node was not accepted, so a FILTER_SKIP node hid
its children instead of exposing them:

    <div id=root><section><a></a></section></div>, SECTION -> FILTER_SKIP
    w.firstChild()   // null (Chrome: <a>)

They also used the boolean `_accept`, so FILTER_REJECT (which must not
descend) and FILTER_SKIP (which must) were indistinguishable — the same
root cause #461 fixed for nextNode().

Replaced all four with the spec's "traverse children" and "traverse
siblings" algorithms, parameterised by direction so forward and backward
share one implementation and cannot drift: `edge` is first/last child and
`step` is next/previous sibling. Both consult the three-valued `_filter`
from #461, so SKIP descends, REJECT prunes, and the climb back out stops
at root or the node the walk started from.

parentNode() is left alone here: its deviations are different (it checks
only the immediate parent, and excludes root, which the spec permits it
to return), so it belongs in its own change.
This commit is contained in:
Andrey Mnatsakanov committed 2026-07-24 08:33:01 +02:00
1 parent 485497393d
commit 49d4b91348
2 files changed
+141 -27

No files matched your search

+48 -27
View File
@@ -2681,38 +2681,59 @@ class Document extends Node {
}
return null;
},
firstChild() {
let child = this.currentNode.firstChild;
while (child) {
if (this._accept(child)) { this.currentNode = child; return child; }
child = child.nextSibling;
// DOM 6.1 "traverse children" (issue #469). The movers used to step
// straight to the next sibling when a node was not accepted, so a
// FILTER_SKIP node hid its children instead of exposing them. `edge` and
// `step` pick the direction: first/next for forward, last/previous for
// backward.
_traverseChildren(edge, step) {
let node = this.currentNode[edge];
while (node) {
const verdict = this._filter(node);
if (verdict === 1) { this.currentNode = node; return node; }
// Only SKIP leaves the children eligible; REJECT prunes the subtree.
if (verdict === 3) {
const child = node[edge];
if (child) { node = child; continue; }
}
// Subtree exhausted: step sideways, climbing out without passing
// root or the node the walk started from.
while (node) {
const sibling = node[step];
if (sibling) { node = sibling; break; }
const parent = node.parentNode;
if (!parent || parent === this.root || parent === this.currentNode) return null;
node = parent;
}
}
return null;
},
lastChild() {
let child = this.currentNode.lastChild;
while (child) {
if (this._accept(child)) { this.currentNode = child; return child; }
child = child.previousSibling;
// DOM 6.1 "traverse siblings" (issue #469).
_traverseSiblings(edge, step) {
let node = this.currentNode;
if (node === this.root) return null;
for (;;) {
let sibling = node[step];
while (sibling) {
node = sibling;
const verdict = this._filter(node);
if (verdict === 1) { this.currentNode = node; return node; }
// Descend into a skipped sibling's subtree; a rejected one is
// off-limits, and a childless one has nothing to descend into.
sibling = node[edge];
if (verdict === 2 || !sibling) sibling = node[step];
}
node = node.parentNode;
if (!node || node === this.root) return null;
// An accepted parent is where the walk would go next, so there is no
// sibling to return.
if (this._filter(node) === 1) return null;
}
return null;
},
nextSibling() {
let sibling = this.currentNode.nextSibling;
while (sibling) {
if (this._accept(sibling)) { this.currentNode = sibling; return sibling; }
sibling = sibling.nextSibling;
}
return null;
},
previousSibling() {
let sibling = this.currentNode.previousSibling;
while (sibling) {
if (this._accept(sibling)) { this.currentNode = sibling; return sibling; }
sibling = sibling.previousSibling;
}
return null;
},
firstChild() { return this._traverseChildren('firstChild', 'nextSibling'); },
lastChild() { return this._traverseChildren('lastChild', 'previousSibling'); },
nextSibling() { return this._traverseSiblings('firstChild', 'nextSibling'); },
previousSibling() { return this._traverseSiblings('lastChild', 'previousSibling'); },
parentNode() {
let parent = this.currentNode.parentNode;
if (parent && parent !== this.root && this._accept(parent)) {
+93
View File
@@ -1541,6 +1541,99 @@ mod tests {
assert_eq!(result, serde_json::json!(["P", "A"]));
}
/// Issue #469: FILTER_SKIP leaves a skipped node's children eligible, so
/// firstChild()/lastChild() must descend into them. FILTER_REJECT must not.
#[test]
fn tree_walker_child_movers_descend_on_skip_but_not_on_reject() {
let mut rt = setup_runtime(
r#"<div id="root"><section><a></a><b></b></section></div>"#,
);
let result = rt
.evaluate(
r#"
const root = document.getElementById('root');
function mover(verdict, method) {
const w = document.createTreeWalker(root, NodeFilter.SHOW_ELEMENT, {
acceptNode(node) {
return node.tagName === 'SECTION' ? verdict : NodeFilter.FILTER_ACCEPT;
}
});
const found = w[method]();
return found ? found.tagName : null;
}
return [
mover(NodeFilter.FILTER_SKIP, 'firstChild'),
mover(NodeFilter.FILTER_SKIP, 'lastChild'),
mover(NodeFilter.FILTER_REJECT, 'firstChild'),
mover(NodeFilter.FILTER_REJECT, 'lastChild'),
];
"#,
)
.unwrap();
// SKIP descends into <section>; REJECT prunes it and finds nothing else.
assert_eq!(result, serde_json::json!(["A", "B", null, null]));
}
/// Issue #469: nextSibling()/previousSibling() must descend into a skipped
/// sibling's subtree rather than stepping straight over it.
#[test]
fn tree_walker_sibling_movers_descend_into_skipped_siblings() {
let mut rt = setup_runtime(
r#"<div id="root"><p id="start"></p><section><a></a></section><q></q></div>"#,
);
let result = rt
.evaluate(
r#"
const root = document.getElementById('root');
function mover(verdict, method, from) {
const w = document.createTreeWalker(root, NodeFilter.SHOW_ELEMENT, {
acceptNode(node) {
return node.tagName === 'SECTION' ? verdict : NodeFilter.FILTER_ACCEPT;
}
});
w.currentNode = document.getElementById(from);
const found = w[method]();
return found ? found.tagName : null;
}
return [
// <section> is skipped, so its child <a> is the next sibling.
mover(NodeFilter.FILTER_SKIP, 'nextSibling', 'start'),
// Rejected: the subtree is off-limits, so skip past to <q>.
mover(NodeFilter.FILTER_REJECT, 'nextSibling', 'start'),
];
"#,
)
.unwrap();
assert_eq!(result, serde_json::json!(["A", "Q"]));
}
/// Issue #469: the backward sibling mover descends to *last* children.
#[test]
fn tree_walker_previous_sibling_descends_to_last_child() {
let mut rt = setup_runtime(
r#"<div id="root"><section><a></a><b></b></section><p id="start"></p></div>"#,
);
let result = rt
.evaluate(
r#"
const root = document.getElementById('root');
const w = document.createTreeWalker(root, NodeFilter.SHOW_ELEMENT, {
acceptNode(node) {
return node.tagName === 'SECTION'
? NodeFilter.FILTER_SKIP
: NodeFilter.FILTER_ACCEPT;
}
});
w.currentNode = document.getElementById('start');
const found = w.previousSibling();
return found ? found.tagName : null;
"#,
)
.unwrap();
// Reverse order descends to <section>'s last child, not its first.
assert_eq!(result, serde_json::json!("B"));
}
#[test]
fn append_child_flattens_document_fragment() {
let mut rt = setup_runtime(r#"<main id="host"></main>"#);