fix(js): implement a real NodeIterator instead of aliasing TreeWalker (#467)
createNodeIterator returned a TreeWalker with a flag on it. The two are
different interfaces, and the alias got two things wrong.
An iterator's pointer starts *before* its root, so the first nextNode()
returns the root itself; a TreeWalker starts *at* its root and never
returns it. So createNodeIterator(el, SHOW_ELEMENT) silently skipped
exactly the element it was rooted at:
<div id=root><a></a></div> -> ["A"] (Chrome: ["DIV","A"])
It also exposed the wrong surface: no referenceNode,
pointerBeforeReferenceNode or detach (older library code still calls
detach and hit "not a function"), while leaking currentNode, firstChild,
parentNode and the rest of TreeWalker's movers.
Implemented DOM 6.2 "traverse" directly. Keeping the pointer as
(referenceNode, pointerBeforeReferenceNode) rather than a single cursor
is what makes a direction change re-yield the current node instead of
stepping over it, and a failed traversal leaves both untouched so the
iterator stays resumable either way.
The backward step is a new `DomTree::prev_in_subtree`, mirroring
`next_in_subtree`. It can live entirely in the DOM layer because a
NodeIterator prunes nothing -- unlike the TreeWalker's backward walk
(#462), where FILTER_REJECT forces the filter to be consulted at every
step of the descent.
FILTER_REJECT still behaves as FILTER_SKIP here, which the existing test
keeps covering; its expectation gains the root node the alias used to
drop. The TreeWalker's now-dead `_rejectIsSkip` branch is removed.
This commit is contained in:
1 parent
485497393d
commit
ab3ca26046
4 files changed
+224
-11
No files matched your search
@@ -579,6 +579,41 @@ impl DomTree {
|
|||||||
Self::climb_to_next_sibling(&inner, root, current)
|
Self::climb_to_next_sibling(&inner, root, current)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Returns the node before `current` in document order, without leaving the
|
||||||
|
/// subtree rooted at `root`. `root` has no predecessor within its own
|
||||||
|
/// subtree, but it is itself reachable as one — a NodeIterator can return
|
||||||
|
/// its root, unlike a TreeWalker.
|
||||||
|
///
|
||||||
|
/// A NodeIterator applies no subtree pruning (DOM 6.2: FILTER_REJECT
|
||||||
|
/// behaves as FILTER_SKIP), so unlike the TreeWalker's backward walk the
|
||||||
|
/// whole step fits here instead of being interleaved with filter calls.
|
||||||
|
pub fn prev_in_subtree(&self, root: NodeId, current: NodeId) -> Option<NodeId> {
|
||||||
|
let inner = self.inner.borrow();
|
||||||
|
if current == root {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
let current_node = inner.nodes.get(current.index())?.as_ref()?;
|
||||||
|
|
||||||
|
let Some(prev) = current_node.prev_sibling else {
|
||||||
|
// No previous sibling: the parent immediately precedes `current`.
|
||||||
|
return current_node.parent;
|
||||||
|
};
|
||||||
|
|
||||||
|
// Otherwise it is the previous sibling's deepest last descendant.
|
||||||
|
let mut node_id = prev;
|
||||||
|
for _ in 0..=inner.nodes.len() {
|
||||||
|
let node = inner.nodes.get(node_id.index())?.as_ref()?;
|
||||||
|
match node.last_child {
|
||||||
|
Some(child) => node_id = child,
|
||||||
|
None => return Some(node_id),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Same defense in depth as the forward walk: a malformed tree must not
|
||||||
|
// spin here.
|
||||||
|
None
|
||||||
|
}
|
||||||
|
|
||||||
/// Follow `current`'s next sibling, climbing ancestors until one has a next
|
/// Follow `current`'s next sibling, climbing ancestors until one has a next
|
||||||
/// sibling — without stepping outside `root`.
|
/// sibling — without stepping outside `root`.
|
||||||
fn climb_to_next_sibling(
|
fn climb_to_next_sibling(
|
||||||
@@ -1071,6 +1106,29 @@ mod tests {
|
|||||||
assert_eq!(tree.next_after_subtree(root, root), None);
|
assert_eq!(tree.next_after_subtree(root, root), None);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_prev_in_subtree_reverses_document_order() {
|
||||||
|
// root > [first > nested, second]; document order is root, first,
|
||||||
|
// nested, second, so the reverse walk must retrace it exactly.
|
||||||
|
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);
|
||||||
|
|
||||||
|
// Previous sibling's deepest last descendant, not the sibling itself.
|
||||||
|
assert_eq!(tree.prev_in_subtree(root, second), Some(nested));
|
||||||
|
assert_eq!(tree.prev_in_subtree(root, nested), Some(first));
|
||||||
|
// No previous sibling: the parent precedes it, and root is returnable.
|
||||||
|
assert_eq!(tree.prev_in_subtree(root, first), Some(root));
|
||||||
|
// Root has no predecessor inside its own subtree.
|
||||||
|
assert_eq!(tree.prev_in_subtree(root, root), None);
|
||||||
|
}
|
||||||
|
|
||||||
// Builds a chain of `depth` nested <div> elements under the document and
|
// Builds a chain of `depth` nested <div> elements under the document and
|
||||||
// returns (outermost_div, innermost_div). Depth this large overflows a
|
// returns (outermost_div, innermost_div). Depth this large overflows a
|
||||||
// recursive tree walk and aborts the process, so it guards the iterative
|
// recursive tree walk and aborts the process, so it guards the iterative
|
||||||
|
|||||||
Vendored
+57
-10
@@ -2637,11 +2637,8 @@ class Document extends Node {
|
|||||||
const verdict = this._filter(node);
|
const verdict = this._filter(node);
|
||||||
if (verdict === 1) { this.currentNode = node; return node; }
|
if (verdict === 1) { this.currentNode = node; return node; }
|
||||||
// FILTER_REJECT skips the node AND its subtree; FILTER_SKIP (and any
|
// FILTER_REJECT skips the node AND its subtree; FILTER_SKIP (and any
|
||||||
// other non-accept value) skips only the node. NodeIterator has no
|
// other non-accept value) skips only the node.
|
||||||
// pruning at all, so `_rejectIsSkip` keeps it on the plain step.
|
const step = verdict === 2 ? "next_after_subtree" : "next_in_subtree";
|
||||||
const step = (verdict === 2 && !this._rejectIsSkip)
|
|
||||||
? "next_after_subtree"
|
|
||||||
: "next_in_subtree";
|
|
||||||
node = _wrap(+_dom(step, this.root._nid, node._nid));
|
node = _wrap(+_dom(step, this.root._nid, node._nid));
|
||||||
}
|
}
|
||||||
return null;
|
return null;
|
||||||
@@ -2724,12 +2721,62 @@ class Document extends Node {
|
|||||||
};
|
};
|
||||||
return walker;
|
return walker;
|
||||||
}
|
}
|
||||||
|
// A real NodeIterator (DOM 6.2), not a TreeWalker in disguise (issue #467).
|
||||||
|
// The two differ in more than naming: an iterator's pointer starts *before*
|
||||||
|
// its root, so the first nextNode() returns the root itself, and it exposes
|
||||||
|
// referenceNode/pointerBeforeReferenceNode/detach rather than a TreeWalker's
|
||||||
|
// currentNode and child/sibling movers.
|
||||||
createNodeIterator(root, whatToShow, filter) {
|
createNodeIterator(root, whatToShow, filter) {
|
||||||
// Shares the TreeWalker implementation, but DOM 6.2 gives NodeIterator no
|
// whatToShow is unsigned long; default SHOW_ALL only when the arg is
|
||||||
// subtree pruning: FILTER_REJECT behaves exactly as FILTER_SKIP there.
|
// omitted. An explicit 0 (show nothing) must stay 0, not become SHOW_ALL.
|
||||||
const iterator = this.createTreeWalker(root, whatToShow, filter);
|
whatToShow = (whatToShow === undefined) ? 0xFFFFFFFF : (whatToShow >>> 0);
|
||||||
iterator._rejectIsSkip = true;
|
return {
|
||||||
return iterator;
|
root: root,
|
||||||
|
referenceNode: root,
|
||||||
|
pointerBeforeReferenceNode: true,
|
||||||
|
whatToShow: whatToShow,
|
||||||
|
filter: filter || null,
|
||||||
|
// NodeIterator prunes nothing: FILTER_REJECT behaves as FILTER_SKIP, so
|
||||||
|
// unlike the TreeWalker only "accepted or not" matters here.
|
||||||
|
_accept(node) {
|
||||||
|
if (!((whatToShow >> (node.nodeType - 1)) & 1)) return false;
|
||||||
|
if (this.filter) {
|
||||||
|
if (typeof this.filter === 'function') return this.filter(node) === 1;
|
||||||
|
if (this.filter.acceptNode) return this.filter.acceptNode(node) === 1;
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
},
|
||||||
|
// DOM 6.2 "traverse". The pointer sits either before or after
|
||||||
|
// referenceNode, which is why reversing direction re-yields the current
|
||||||
|
// node instead of stepping over it.
|
||||||
|
_traverse(forward) {
|
||||||
|
let node = this.referenceNode;
|
||||||
|
let before = this.pointerBeforeReferenceNode;
|
||||||
|
for (;;) {
|
||||||
|
if (forward === before) {
|
||||||
|
// Consume the pointer's side without moving: it flips to the other
|
||||||
|
// side of the node it already references.
|
||||||
|
before = !before;
|
||||||
|
} else {
|
||||||
|
const step = forward ? "next_in_subtree" : "prev_in_subtree";
|
||||||
|
const next = _wrap(+_dom(step, this.root._nid, node._nid));
|
||||||
|
// A failed traversal leaves referenceNode and the pointer
|
||||||
|
// untouched, so the iterator can be resumed in either direction.
|
||||||
|
if (!next) return null;
|
||||||
|
node = next;
|
||||||
|
}
|
||||||
|
if (this._accept(node)) break;
|
||||||
|
}
|
||||||
|
this.referenceNode = node;
|
||||||
|
this.pointerBeforeReferenceNode = before;
|
||||||
|
return node;
|
||||||
|
},
|
||||||
|
nextNode() { return this._traverse(true); },
|
||||||
|
previousNode() { return this._traverse(false); },
|
||||||
|
// Legacy no-op since DOM4, but older library code still calls it and
|
||||||
|
// used to hit "detach is not a function".
|
||||||
|
detach() {},
|
||||||
|
};
|
||||||
}
|
}
|
||||||
getSelection() { return this.defaultView ? _selectionFor(this) : null; }
|
getSelection() { return this.defaultView ? _selectionFor(this) : null; }
|
||||||
get activeElement() { return globalThis.__obscura_focused || this.body; }
|
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())
|
.map(|id| id.index().to_string())
|
||||||
.unwrap_or("-1".into())
|
.unwrap_or("-1".into())
|
||||||
}
|
}
|
||||||
|
// Reverse document order within a subtree, for NodeIterator's backward
|
||||||
|
// walk (which prunes nothing, so the whole step fits in the DOM layer).
|
||||||
|
"prev_in_subtree" => {
|
||||||
|
let root = NodeId::new(arg1.parse::<u32>().unwrap_or(0));
|
||||||
|
let current = NodeId::new(arg2.parse::<u32>().unwrap_or(0));
|
||||||
|
dom.prev_in_subtree(root, current)
|
||||||
|
.map(|id| id.index().to_string())
|
||||||
|
.unwrap_or("-1".into())
|
||||||
|
}
|
||||||
// Step past a whole subtree rather than into it: NodeFilter.FILTER_REJECT
|
// Step past a whole subtree rather than into it: NodeFilter.FILTER_REJECT
|
||||||
// prunes the rejected node's descendants, unlike FILTER_SKIP.
|
// prunes the rejected node's descendants, unlike FILTER_SKIP.
|
||||||
"next_after_subtree" => {
|
"next_after_subtree" => {
|
||||||
|
|||||||
@@ -1538,7 +1538,106 @@ mod tests {
|
|||||||
"#,
|
"#,
|
||||||
)
|
)
|
||||||
.unwrap();
|
.unwrap();
|
||||||
assert_eq!(result, serde_json::json!(["P", "A"]));
|
// The rejected <section> is skipped but not pruned, so <p> still shows.
|
||||||
|
// The leading root is #467: an iterator yields the node it is rooted at.
|
||||||
|
assert_eq!(result, serde_json::json!(["DIV", "P", "A"]));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Issue #467: a NodeIterator starts *before* its root, so the first
|
||||||
|
/// nextNode() returns the root itself. Aliasing createTreeWalker silently
|
||||||
|
/// dropped exactly the element the iterator was rooted at.
|
||||||
|
#[test]
|
||||||
|
fn node_iterator_yields_the_root_node_first() {
|
||||||
|
let mut rt = setup_runtime(r#"<div id="root"><a></a></div>"#);
|
||||||
|
let result = rt
|
||||||
|
.evaluate(
|
||||||
|
r#"
|
||||||
|
const root = document.getElementById('root');
|
||||||
|
const it = document.createNodeIterator(root, NodeFilter.SHOW_ELEMENT);
|
||||||
|
const seen = [];
|
||||||
|
let node;
|
||||||
|
while ((node = it.nextNode())) seen.push(node.tagName);
|
||||||
|
return seen;
|
||||||
|
"#,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(result, serde_json::json!(["DIV", "A"]));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Issue #467: the NodeIterator interface surface, and that TreeWalker-only
|
||||||
|
/// members are not exposed on it.
|
||||||
|
#[test]
|
||||||
|
fn node_iterator_exposes_its_own_interface() {
|
||||||
|
let mut rt = setup_runtime(r#"<div id="root"><a></a></div>"#);
|
||||||
|
let result = rt
|
||||||
|
.evaluate(
|
||||||
|
r#"
|
||||||
|
const root = document.getElementById('root');
|
||||||
|
const it = document.createNodeIterator(root, NodeFilter.SHOW_ELEMENT);
|
||||||
|
const before = [it.referenceNode === root, it.pointerBeforeReferenceNode];
|
||||||
|
it.nextNode();
|
||||||
|
return [
|
||||||
|
before,
|
||||||
|
typeof it.detach,
|
||||||
|
it.detach() === undefined,
|
||||||
|
typeof it.previousNode,
|
||||||
|
it.root === root,
|
||||||
|
it.whatToShow,
|
||||||
|
// TreeWalker-only members must not leak onto a NodeIterator.
|
||||||
|
typeof it.currentNode,
|
||||||
|
typeof it.firstChild,
|
||||||
|
typeof it.parentNode,
|
||||||
|
// The pointer advanced past the root it just returned.
|
||||||
|
[it.referenceNode.tagName, it.pointerBeforeReferenceNode],
|
||||||
|
];
|
||||||
|
"#,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
result,
|
||||||
|
serde_json::json!([
|
||||||
|
[true, true],
|
||||||
|
"function",
|
||||||
|
true,
|
||||||
|
"function",
|
||||||
|
true,
|
||||||
|
1,
|
||||||
|
"undefined",
|
||||||
|
"undefined",
|
||||||
|
"undefined",
|
||||||
|
["DIV", false]
|
||||||
|
])
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Issue #467: previousNode() retraces the iterator, and the root is the
|
||||||
|
/// last node it yields going backwards.
|
||||||
|
#[test]
|
||||||
|
fn node_iterator_previous_node_retraces_the_walk() {
|
||||||
|
let mut rt = setup_runtime(r#"<div id="root"><a><b></b></a><c></c></div>"#);
|
||||||
|
let result = rt
|
||||||
|
.evaluate(
|
||||||
|
r#"
|
||||||
|
const root = document.getElementById('root');
|
||||||
|
const it = document.createNodeIterator(root, NodeFilter.SHOW_ELEMENT);
|
||||||
|
const forward = [];
|
||||||
|
let node;
|
||||||
|
while ((node = it.nextNode())) forward.push(node.tagName);
|
||||||
|
const backward = [];
|
||||||
|
while ((node = it.previousNode())) backward.push(node.tagName);
|
||||||
|
return [forward, backward];
|
||||||
|
"#,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
// Forward ends on <c>; going back re-yields <c> (the pointer sits after
|
||||||
|
// it), then the rest in reverse, root included.
|
||||||
|
assert_eq!(
|
||||||
|
result,
|
||||||
|
serde_json::json!([
|
||||||
|
["DIV", "A", "B", "C"],
|
||||||
|
["C", "B", "A", "DIV"]
|
||||||
|
])
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
Reference in new issue
Block a user