fix(js): honour NodeFilter.FILTER_REJECT in TreeWalker (#461)
`_accept` collapsed every non-ACCEPT filter result into a single boolean, so FILTER_REJECT behaved exactly like FILTER_SKIP: the node was not returned, but the walk still descended into its subtree. Per DOM 6.1, FILTER_REJECT rejects a node *and* its descendants. This was unreachable until #440 exposed the real NodeFilter constants, which made the canonical prune idiom if (node.tagName === 'SCRIPT') return NodeFilter.FILTER_REJECT; start yielding nodes from inside the pruned subtree instead of failing loudly — sanitizers and text extractors built on TreeWalker silently got extra content. Split the filter into a three-valued `_filter` (keeping `_accept` as the boolean wrapper the sibling/child movers use) and give nextNode a second traversal step for the reject case. That step lives in the DOM layer as `DomTree::next_after_subtree`, mirroring how #447 put `next_in_subtree` there: both share the ancestor climb, and REJECT costs no more JS/native crossings than SKIP. NodeIterator shares this walker but has no subtree pruning (DOM 6.2: FILTER_REJECT behaves as FILTER_SKIP), so `createNodeIterator` marks the walker `_rejectIsSkip` and keeps the plain step. Covered by its own test so the shared implementation cannot drift.
This commit is contained in:
1 parent
2974a71f3c
commit
1a5c27a592
4 files changed
+141
-9
No files matched your search
@@ -564,7 +564,28 @@ impl DomTree {
|
||||
if let Some(child) = current_node.first_child {
|
||||
return Some(child);
|
||||
}
|
||||
Self::climb_to_next_sibling(&inner, root, current)
|
||||
}
|
||||
|
||||
/// Returns the node after the whole subtree rooted at `current`, in document
|
||||
/// order, without leaving the subtree rooted at `root`.
|
||||
///
|
||||
/// This is `next_in_subtree` minus the descend-into-children step, which is
|
||||
/// what `NodeFilter.FILTER_REJECT` needs: it rejects a node *and* its
|
||||
/// descendants, unlike `FILTER_SKIP`, which only skips the node itself and
|
||||
/// is served by `next_in_subtree`.
|
||||
pub fn next_after_subtree(&self, root: NodeId, current: NodeId) -> Option<NodeId> {
|
||||
let inner = self.inner.borrow();
|
||||
Self::climb_to_next_sibling(&inner, root, current)
|
||||
}
|
||||
|
||||
/// Follow `current`'s next sibling, climbing ancestors until one has a next
|
||||
/// sibling — without stepping outside `root`.
|
||||
fn climb_to_next_sibling(
|
||||
inner: &DomTreeInner,
|
||||
root: NodeId,
|
||||
current: NodeId,
|
||||
) -> Option<NodeId> {
|
||||
let mut node_id = current;
|
||||
for _ in 0..=inner.nodes.len() {
|
||||
if node_id == root {
|
||||
@@ -1027,6 +1048,29 @@ mod tests {
|
||||
assert_eq!(tree.next_in_subtree(root, second), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_next_after_subtree_skips_descendants() {
|
||||
// Same shape as above: root > [first > nested, second]. Stepping past
|
||||
// `first` must land on `second`, not descend into `nested` — that is
|
||||
// what NodeFilter.FILTER_REJECT needs.
|
||||
let tree = DomTree::new();
|
||||
let root = tree.new_node(NodeData::Text { contents: "root".into() });
|
||||
let first = tree.new_node(NodeData::Text { contents: "first".into() });
|
||||
let nested = tree.new_node(NodeData::Text { contents: "nested".into() });
|
||||
let second = tree.new_node(NodeData::Text { contents: "second".into() });
|
||||
tree.append_child(tree.document(), root);
|
||||
tree.append_child(root, first);
|
||||
tree.append_child(first, nested);
|
||||
tree.append_child(root, second);
|
||||
|
||||
assert_eq!(tree.next_after_subtree(root, first), Some(second));
|
||||
// A leaf behaves identically to next_in_subtree: there is no subtree.
|
||||
assert_eq!(tree.next_after_subtree(root, nested), Some(second));
|
||||
assert_eq!(tree.next_after_subtree(root, second), None);
|
||||
// Rejecting the root itself exhausts the walk rather than escaping it.
|
||||
assert_eq!(tree.next_after_subtree(root, root), None);
|
||||
}
|
||||
|
||||
// Builds a chain of `depth` nested <div> elements under the document and
|
||||
// returns (outermost_div, innermost_div). Depth this large overflows a
|
||||
// recursive tree walk and aborts the process, so it guards the iterative
|
||||
|
||||
Vendored
+25
-9
@@ -2616,21 +2616,33 @@ class Document extends Node {
|
||||
currentNode: root,
|
||||
whatToShow: whatToShow,
|
||||
filter: filter || null,
|
||||
_accept(node) {
|
||||
// Three-valued per NodeFilter: 1 ACCEPT, 2 REJECT, 3 SKIP. REJECT and
|
||||
// SKIP both mean "don't return this node", but only REJECT prunes its
|
||||
// descendants, so nextNode() needs to tell them apart (issue #461).
|
||||
// A node filtered out by whatToShow is a SKIP: the spec never consults
|
||||
// the filter for it, and its descendants stay eligible.
|
||||
_filter(node) {
|
||||
const nodeType = node.nodeType;
|
||||
const show = (whatToShow >> (nodeType - 1)) & 1;
|
||||
if (!show) return false;
|
||||
if (!((whatToShow >> (nodeType - 1)) & 1)) return 3;
|
||||
if (this.filter) {
|
||||
if (typeof this.filter === 'function') return this.filter(node) === 1;
|
||||
if (this.filter.acceptNode) return this.filter.acceptNode(node) === 1;
|
||||
if (typeof this.filter === 'function') return this.filter(node);
|
||||
if (this.filter.acceptNode) return this.filter.acceptNode(node);
|
||||
}
|
||||
return true;
|
||||
return 1;
|
||||
},
|
||||
_accept(node) { return this._filter(node) === 1; },
|
||||
nextNode() {
|
||||
let node = _wrap(+_dom("next_in_subtree", this.root._nid, this.currentNode._nid));
|
||||
while (node) {
|
||||
if (this._accept(node)) { this.currentNode = node; return node; }
|
||||
node = _wrap(+_dom("next_in_subtree", this.root._nid, node._nid));
|
||||
const verdict = this._filter(node);
|
||||
if (verdict === 1) { this.currentNode = node; return node; }
|
||||
// FILTER_REJECT skips the node AND its subtree; FILTER_SKIP (and any
|
||||
// other non-accept value) skips only the node. NodeIterator has no
|
||||
// pruning at all, so `_rejectIsSkip` keeps it on the plain step.
|
||||
const step = (verdict === 2 && !this._rejectIsSkip)
|
||||
? "next_after_subtree"
|
||||
: "next_in_subtree";
|
||||
node = _wrap(+_dom(step, this.root._nid, node._nid));
|
||||
}
|
||||
return null;
|
||||
},
|
||||
@@ -2693,7 +2705,11 @@ class Document extends Node {
|
||||
return walker;
|
||||
}
|
||||
createNodeIterator(root, whatToShow, filter) {
|
||||
return this.createTreeWalker(root, whatToShow, filter);
|
||||
// Shares the TreeWalker implementation, but DOM 6.2 gives NodeIterator no
|
||||
// subtree pruning: FILTER_REJECT behaves exactly as FILTER_SKIP there.
|
||||
const iterator = this.createTreeWalker(root, whatToShow, filter);
|
||||
iterator._rejectIsSkip = true;
|
||||
return iterator;
|
||||
}
|
||||
getSelection() { return this.defaultView ? _selectionFor(this) : null; }
|
||||
get activeElement() { return globalThis.__obscura_focused || this.body; }
|
||||
|
||||
@@ -275,6 +275,15 @@ fn op_dom_inner(state: &OpState, cmd: String, arg1: String, arg2: String) -> Str
|
||||
.map(|id| id.index().to_string())
|
||||
.unwrap_or("-1".into())
|
||||
}
|
||||
// Step past a whole subtree rather than into it: NodeFilter.FILTER_REJECT
|
||||
// prunes the rejected node's descendants, unlike FILTER_SKIP.
|
||||
"next_after_subtree" => {
|
||||
let root = NodeId::new(arg1.parse::<u32>().unwrap_or(0));
|
||||
let current = NodeId::new(arg2.parse::<u32>().unwrap_or(0));
|
||||
dom.next_after_subtree(root, current)
|
||||
.map(|id| id.index().to_string())
|
||||
.unwrap_or("-1".into())
|
||||
}
|
||||
"child_nodes" => {
|
||||
let nid = arg1.parse::<u32>().unwrap_or(0);
|
||||
let ids: Vec<i32> = dom.children(NodeId::new(nid)).iter().map(|id| id.index() as i32).collect();
|
||||
|
||||
@@ -1377,6 +1377,69 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// Issue #461: FILTER_REJECT must prune the rejected node's whole subtree,
|
||||
/// while FILTER_SKIP only skips the node and leaves descendants eligible.
|
||||
/// Collapsing both into "not accepted" let a TreeWalker yield nodes from
|
||||
/// inside a subtree the page explicitly rejected.
|
||||
#[test]
|
||||
fn tree_walker_filter_reject_prunes_the_whole_subtree() {
|
||||
let mut rt = setup_runtime(
|
||||
r#"<div id="root"><section><p>deep</p></section><a></a></div>"#,
|
||||
);
|
||||
rt.run_page_init();
|
||||
let result = rt
|
||||
.evaluate(
|
||||
r#"
|
||||
const root = document.getElementById('root');
|
||||
function walk(verdict) {
|
||||
const w = document.createTreeWalker(root, NodeFilter.SHOW_ELEMENT, {
|
||||
acceptNode(node) {
|
||||
return node.tagName === 'SECTION' ? verdict : NodeFilter.FILTER_ACCEPT;
|
||||
}
|
||||
});
|
||||
const seen = [];
|
||||
let node;
|
||||
while ((node = w.nextNode())) seen.push(node.tagName);
|
||||
return seen;
|
||||
}
|
||||
return [walk(NodeFilter.FILTER_REJECT), walk(NodeFilter.FILTER_SKIP)];
|
||||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
// REJECT drops <p> with its <section> parent; SKIP drops only <section>.
|
||||
assert_eq!(result, serde_json::json!([["A"], ["P", "A"]]));
|
||||
}
|
||||
|
||||
/// Issue #461: NodeIterator has no subtree pruning — DOM 6.2 says
|
||||
/// FILTER_REJECT behaves as FILTER_SKIP there. The shared walker must not
|
||||
/// leak TreeWalker's pruning into it.
|
||||
#[test]
|
||||
fn node_iterator_treats_filter_reject_as_skip() {
|
||||
let mut rt = setup_runtime(
|
||||
r#"<div id="root"><section><p>deep</p></section><a></a></div>"#,
|
||||
);
|
||||
rt.run_page_init();
|
||||
let result = rt
|
||||
.evaluate(
|
||||
r#"
|
||||
const root = document.getElementById('root');
|
||||
const it = document.createNodeIterator(root, NodeFilter.SHOW_ELEMENT, {
|
||||
acceptNode(node) {
|
||||
return node.tagName === 'SECTION'
|
||||
? NodeFilter.FILTER_REJECT
|
||||
: NodeFilter.FILTER_ACCEPT;
|
||||
}
|
||||
});
|
||||
const seen = [];
|
||||
let node;
|
||||
while ((node = it.nextNode())) seen.push(node.tagName);
|
||||
return seen;
|
||||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(result, serde_json::json!(["P", "A"]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn append_child_flattens_document_fragment() {
|
||||
let mut rt = setup_runtime(r#"<main id="host"></main>"#);
|
||||
|
||||
Reference in new issue
Block a user