fix(js): walk reverse document order in TreeWalker.previousNode() (#462)
previousNode() considered exactly one candidate — the previous sibling's deepest last child — and returned null as soon as the filter rejected it, so a backward walk died mid-tree. This is the backward mirror of #432, which was fixed forward-only in #447. <div id=root><a><b></b></a><c></c></div>, SHOW_ELEMENT, B -> FILTER_SKIP forward: ["A","C"] backward: [] (Chrome: ["A"]) Replaced with the DOM 6.1 algorithm verbatim: scan previous siblings, descend to the deepest last descendant of each, then step up to the parent, continuing until a node is accepted or root is reached. The descent stops on FILTER_REJECT, which is what makes rejection prune subtrees in the backward direction too. Unlike nextNode this stays in JS rather than adding a DOM traversal op: the descent has to consult the filter at every step to honour REJECT, so there is no run of JS/native crossings for a native helper to collapse. Depends on the three-valued `_filter` from #461.
This commit is contained in:
1 parent
1a5c27a592
commit
a8c0a19f67
2 files changed
+130
-10
No files matched your search
Vendored
+30
-10
@@ -2646,18 +2646,38 @@ class Document extends Node {
|
|||||||
}
|
}
|
||||||
return null;
|
return null;
|
||||||
},
|
},
|
||||||
|
// DOM 6.1 "previousNode", implemented as specified (issue #462). The old
|
||||||
|
// version looked at exactly one candidate — the previous sibling's
|
||||||
|
// deepest last child — and returned null the moment it was filtered out,
|
||||||
|
// so a backward walk died mid-tree the way nextNode used to before #432.
|
||||||
|
//
|
||||||
|
// Unlike nextNode this stays in JS rather than using a DOM traversal op:
|
||||||
|
// the descent into last children has to stop on FILTER_REJECT, so the
|
||||||
|
// filter is consulted at every step anyway and there is no run of
|
||||||
|
// crossings for a native helper to collapse.
|
||||||
previousNode() {
|
previousNode() {
|
||||||
let node = this.currentNode;
|
let node = this.currentNode;
|
||||||
if (node === this.root) return null;
|
while (node !== this.root) {
|
||||||
let sibling = node.previousSibling;
|
let sibling = node.previousSibling;
|
||||||
if (sibling) {
|
while (sibling) {
|
||||||
while (sibling.lastChild) sibling = sibling.lastChild;
|
node = sibling;
|
||||||
if (this._accept(sibling)) { this.currentNode = sibling; return sibling; }
|
let verdict = this._filter(node);
|
||||||
}
|
// Descend to the deepest last descendant, but never into a rejected
|
||||||
let parent = node.parentNode;
|
// subtree — that is what makes REJECT prune backwards as well.
|
||||||
if (parent && parent !== this.root && this._accept(parent)) {
|
while (verdict !== 2 && node.lastChild) {
|
||||||
this.currentNode = parent;
|
node = node.lastChild;
|
||||||
return parent;
|
verdict = this._filter(node);
|
||||||
|
}
|
||||||
|
if (verdict === 1) { this.currentNode = node; return node; }
|
||||||
|
sibling = node.previousSibling;
|
||||||
|
}
|
||||||
|
const parent = node.parentNode;
|
||||||
|
// Reaching root (or a detached node) ends the walk: root is never
|
||||||
|
// returned by a backward traversal.
|
||||||
|
if (!parent || node === this.root) return null;
|
||||||
|
node = parent;
|
||||||
|
if (node === this.root) return null;
|
||||||
|
if (this._filter(node) === 1) { this.currentNode = node; return node; }
|
||||||
}
|
}
|
||||||
return null;
|
return null;
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -1410,6 +1410,106 @@ mod tests {
|
|||||||
assert_eq!(result, serde_json::json!([["A"], ["P", "A"]]));
|
assert_eq!(result, serde_json::json!([["A"], ["P", "A"]]));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Issue #462: previousNode() must walk reverse document order until a node
|
||||||
|
/// is accepted, not give up as soon as the first candidate is filtered out.
|
||||||
|
#[test]
|
||||||
|
fn previous_node_walks_reverse_document_order() {
|
||||||
|
let mut rt = setup_runtime(r#"<div id="root"><a><b></b></a><c></c></div>"#);
|
||||||
|
rt.run_page_init();
|
||||||
|
let result = rt
|
||||||
|
.evaluate(
|
||||||
|
r#"
|
||||||
|
const root = document.getElementById('root');
|
||||||
|
const w = document.createTreeWalker(root, NodeFilter.SHOW_ELEMENT, {
|
||||||
|
acceptNode(node) {
|
||||||
|
return node.tagName === 'B'
|
||||||
|
? NodeFilter.FILTER_SKIP
|
||||||
|
: NodeFilter.FILTER_ACCEPT;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
const forward = [];
|
||||||
|
let node;
|
||||||
|
while ((node = w.nextNode())) forward.push(node.tagName);
|
||||||
|
const backward = [];
|
||||||
|
while ((node = w.previousNode())) backward.push(node.tagName);
|
||||||
|
return [forward, backward];
|
||||||
|
"#,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
// From <c>, the previous sibling's deepest last child <b> is skipped, so
|
||||||
|
// the walk must keep going up to <a> instead of returning null.
|
||||||
|
assert_eq!(result, serde_json::json!([["A", "C"], ["A"]]));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Issue #462: a backward walk must retrace a forward walk exactly, and stop
|
||||||
|
/// at the root without ever returning it.
|
||||||
|
#[test]
|
||||||
|
fn previous_node_retraces_a_full_forward_walk() {
|
||||||
|
let mut rt = setup_runtime(
|
||||||
|
r#"<div id="root"><section><p>one</p><span></span></section><a><b></b></a></div>"#,
|
||||||
|
);
|
||||||
|
rt.run_page_init();
|
||||||
|
let result = rt
|
||||||
|
.evaluate(
|
||||||
|
r#"
|
||||||
|
const root = document.getElementById('root');
|
||||||
|
const w = document.createTreeWalker(root, NodeFilter.SHOW_ELEMENT);
|
||||||
|
const forward = [];
|
||||||
|
let node;
|
||||||
|
while ((node = w.nextNode())) forward.push(node.tagName);
|
||||||
|
const backward = [];
|
||||||
|
while ((node = w.previousNode())) backward.push(node.tagName);
|
||||||
|
backward.reverse();
|
||||||
|
// previousNode never yields root, and never yields the node the
|
||||||
|
// forward walk ended on, so compare against forward minus its last.
|
||||||
|
// A failed traversal leaves currentNode untouched (DOM 6.1), so
|
||||||
|
// it stays on the last node previousNode did return.
|
||||||
|
return [forward, backward, w.currentNode.tagName];
|
||||||
|
"#,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
result,
|
||||||
|
serde_json::json!([
|
||||||
|
["SECTION", "P", "SPAN", "A", "B"],
|
||||||
|
["SECTION", "P", "SPAN", "A"],
|
||||||
|
"SECTION"
|
||||||
|
])
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Issue #462: FILTER_REJECT prunes a subtree in the backward direction too
|
||||||
|
/// — the descent into a rejected node's last children must stop.
|
||||||
|
#[test]
|
||||||
|
fn previous_node_honours_filter_reject_subtree_pruning() {
|
||||||
|
let mut rt = setup_runtime(
|
||||||
|
r#"<div id="root"><a></a><section><p>deep</p></section><c></c></div>"#,
|
||||||
|
);
|
||||||
|
rt.run_page_init();
|
||||||
|
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_REJECT
|
||||||
|
: NodeFilter.FILTER_ACCEPT;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
while (w.nextNode()) { /* advance to the last accepted node */ }
|
||||||
|
const backward = [];
|
||||||
|
let node;
|
||||||
|
while ((node = w.previousNode())) backward.push(node.tagName);
|
||||||
|
return backward;
|
||||||
|
"#,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
// <p> lives inside the rejected <section>, so the backward walk from <c>
|
||||||
|
// must jump straight to <a>.
|
||||||
|
assert_eq!(result, serde_json::json!(["A"]));
|
||||||
|
}
|
||||||
|
|
||||||
/// Issue #461: NodeIterator has no subtree pruning — DOM 6.2 says
|
/// Issue #461: NodeIterator has no subtree pruning — DOM 6.2 says
|
||||||
/// FILTER_REJECT behaves as FILTER_SKIP there. The shared walker must not
|
/// FILTER_REJECT behaves as FILTER_SKIP there. The shared walker must not
|
||||||
/// leak TreeWalker's pruning into it.
|
/// leak TreeWalker's pruning into it.
|
||||||
|
|||||||
Reference in new issue
Block a user