From 1a5c27a5924b6840ed1b621876e8d8202ff4c2ce Mon Sep 17 00:00:00 2001 From: Andrey Mnatsakanov Date: Wed, 22 Jul 2026 18:57:00 +0200 Subject: [PATCH 1/2] fix(js): honour NodeFilter.FILTER_REJECT in TreeWalker (#461) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_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. --- crates/obscura-dom/src/tree.rs | 44 +++++++++++++++++++++ crates/obscura-js/js/bootstrap.js | 34 ++++++++++++----- crates/obscura-js/src/ops.rs | 9 +++++ crates/obscura-js/src/runtime.rs | 63 +++++++++++++++++++++++++++++++ 4 files changed, 141 insertions(+), 9 deletions(-) diff --git a/crates/obscura-dom/src/tree.rs b/crates/obscura-dom/src/tree.rs index 725cca7..e428ebf 100644 --- a/crates/obscura-dom/src/tree.rs +++ b/crates/obscura-dom/src/tree.rs @@ -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 { + 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 { 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
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 diff --git a/crates/obscura-js/js/bootstrap.js b/crates/obscura-js/js/bootstrap.js index 1b27126..ff1ca2a 100644 --- a/crates/obscura-js/js/bootstrap.js +++ b/crates/obscura-js/js/bootstrap.js @@ -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; } diff --git a/crates/obscura-js/src/ops.rs b/crates/obscura-js/src/ops.rs index c578ac8..aeb4405 100644 --- a/crates/obscura-js/src/ops.rs +++ b/crates/obscura-js/src/ops.rs @@ -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::().unwrap_or(0)); + let current = NodeId::new(arg2.parse::().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::().unwrap_or(0); let ids: Vec = dom.children(NodeId::new(nid)).iter().map(|id| id.index() as i32).collect(); diff --git a/crates/obscura-js/src/runtime.rs b/crates/obscura-js/src/runtime.rs index 740c0ac..6e2f001 100644 --- a/crates/obscura-js/src/runtime.rs +++ b/crates/obscura-js/src/runtime.rs @@ -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#"

deep

"#, + ); + 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

with its

parent; SKIP drops only
. + 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#"

deep

"#, + ); + 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#"
"#); From a8c0a19f6782db009bccf6504abf7dae572d5c88 Mon Sep 17 00:00:00 2001 From: Andrey Mnatsakanov Date: Wed, 22 Jul 2026 18:59:10 +0200 Subject: [PATCH 2/2] fix(js): walk reverse document order in TreeWalker.previousNode() (#462) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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.
, 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. --- crates/obscura-js/js/bootstrap.js | 40 +++++++++--- crates/obscura-js/src/runtime.rs | 100 ++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 10 deletions(-) diff --git a/crates/obscura-js/js/bootstrap.js b/crates/obscura-js/js/bootstrap.js index ff1ca2a..8e70cb3 100644 --- a/crates/obscura-js/js/bootstrap.js +++ b/crates/obscura-js/js/bootstrap.js @@ -2646,18 +2646,38 @@ class Document extends Node { } 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() { let node = this.currentNode; - if (node === this.root) return null; - let sibling = node.previousSibling; - if (sibling) { - while (sibling.lastChild) sibling = sibling.lastChild; - if (this._accept(sibling)) { this.currentNode = sibling; return sibling; } - } - let parent = node.parentNode; - if (parent && parent !== this.root && this._accept(parent)) { - this.currentNode = parent; - return parent; + while (node !== this.root) { + let sibling = node.previousSibling; + while (sibling) { + node = sibling; + let verdict = this._filter(node); + // Descend to the deepest last descendant, but never into a rejected + // subtree — that is what makes REJECT prune backwards as well. + while (verdict !== 2 && node.lastChild) { + node = node.lastChild; + 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; }, diff --git a/crates/obscura-js/src/runtime.rs b/crates/obscura-js/src/runtime.rs index 6e2f001..172a70e 100644 --- a/crates/obscura-js/src/runtime.rs +++ b/crates/obscura-js/src/runtime.rs @@ -1410,6 +1410,106 @@ mod tests { 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#"
"#); + 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 , the previous sibling's deepest last child is skipped, so + // the walk must keep going up to 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#""#, + ); + 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#"

deep

"#, + ); + 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(); + //

lives inside the rejected

, so the backward walk from + // must jump straight to . + assert_eq!(result, serde_json::json!(["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.