From ee97f3b888cb4ce5e7f4a22716375d71bb383bd4 Mon Sep 17 00:00:00 2001 From: Francis Bouvier Date: Sat, 11 Jul 2026 16:05:28 +0200 Subject: [PATCH 1/5] webapi: TreeWalker sibling traversal follows the spec algorithm Fixes WPT /dom/traversal/TreeWalker-previousSiblingLastChildSkip.html and 59 subtests of TreeWalker.html (701/761 -> 760/761): TreeWalker.previousSibling()/nextSibling() only scanned the current node's direct siblings, but the spec's "traverse siblings" algorithm also: - descends into a skipped (FILTER_SKIP) sibling's children - only a rejected sibling excludes its whole subtree - so from B2 with B1 skipped, previousSibling() must return B1's last child; - climbs to the parent when the siblings are exhausted and continues from the parent's siblings, stopping at the root or at an accepted parent. Both directions now share the spec's traverseSiblings implementation. Coverage: - /dom/traversal/TreeWalker-previousSiblingLastChildSkip.html 0/1 -> 1/1 (fully green) - /dom/traversal/TreeWalker.html 701/761 -> 760/761 Co-Authored-By: Claude Fable 5 --- src/browser/webapi/DOMTreeWalker.zig | 57 ++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 16 deletions(-) diff --git a/src/browser/webapi/DOMTreeWalker.zig b/src/browser/webapi/DOMTreeWalker.zig index 6c5d616a4..71e7b0507 100644 --- a/src/browser/webapi/DOMTreeWalker.zig +++ b/src/browser/webapi/DOMTreeWalker.zig @@ -174,27 +174,52 @@ pub fn lastChild(self: *DOMTreeWalker, frame: *Frame) !?*Node { } pub fn previousSibling(self: *DOMTreeWalker, frame: *Frame) !?*Node { - var node = self.previousSiblingOrNull(self._current); - while (node) |n| { - if (try self.acceptNode(n, frame) == NodeFilter.FILTER_ACCEPT) { - self._current = n; - return n; - } - node = self.previousSiblingOrNull(n); - } - return null; + return self.traverseSiblings(.previous, frame); } pub fn nextSibling(self: *DOMTreeWalker, frame: *Frame) !?*Node { - var node = self.nextSiblingOrNull(self._current); - while (node) |n| { - if (try self.acceptNode(n, frame) == NodeFilter.FILTER_ACCEPT) { - self._current = n; - return n; + return self.traverseSiblings(.next, frame); +} + +// The spec's "traverse siblings" algorithm: a skipped (but not rejected) +// sibling's children are still candidates, and when the siblings run out the +// walk climbs to the parent and continues from its siblings, stopping at the +// root or at an accepted parent. +fn traverseSiblings(self: *DOMTreeWalker, comptime direction: enum { next, previous }, frame: *Frame) !?*Node { + var node = self._current; + if (node == self._root) return null; + + while (true) { + var sibling: ?*Node = if (direction == .next) + self.nextSiblingOrNull(node) + else + self.previousSiblingOrNull(node); + + while (sibling) |sib| { + node = sib; + const result = try self.acceptNode(node, frame); + if (result == NodeFilter.FILTER_ACCEPT) { + self._current = node; + return node; + } + sibling = if (direction == .next) + self.firstChildOrNull(node) + else + self.lastChildOrNull(node); + if (result == NodeFilter.FILTER_REJECT or sibling == null) { + sibling = if (direction == .next) + self.nextSiblingOrNull(node) + else + self.previousSiblingOrNull(node); + } + } + + node = node.parentNode() orelse return null; + if (node == self._root) return null; + if (try self.acceptNode(node, frame) == NodeFilter.FILTER_ACCEPT) { + return null; } - node = self.nextSiblingOrNull(n); } - return null; } pub fn previousNode(self: *DOMTreeWalker, frame: *Frame) !?*Node { From 7581db26d4e47d806347f6b4f5ad8fd3a9450b6a Mon Sep 17 00:00:00 2001 From: Francis Bouvier Date: Sat, 11 Jul 2026 16:07:21 +0200 Subject: [PATCH 2/5] webapi: TreeWalker methods throw when called from their own filter Fixes the last failing subtest of WPT /dom/traversal/TreeWalker.html ("Recursive filters need to throw"): per the DOM traversal "filter" algorithm, a NodeFilter that re-enters the walker (calling parentNode() etc. from inside the callback) must get an InvalidStateError. DOMTreeWalker tracks an active flag around the filter invocation and throws InvalidStateError when a traversal starts while it is set. Coverage: /dom/traversal/TreeWalker.html 760/761 -> 761/761 (fully green). Co-Authored-By: Claude Fable 5 --- src/browser/webapi/DOMTreeWalker.zig | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/browser/webapi/DOMTreeWalker.zig b/src/browser/webapi/DOMTreeWalker.zig index 71e7b0507..b6ec9c2d2 100644 --- a/src/browser/webapi/DOMTreeWalker.zig +++ b/src/browser/webapi/DOMTreeWalker.zig @@ -33,6 +33,9 @@ _root: *Node, _what_to_show: u32, _filter: NodeFilter, _current: *Node, +// Set while the filter callback runs; a walker method called from inside +// its own filter must throw InvalidStateError. +_active: bool = false, pub fn init(root: *Node, what_to_show: u32, filter: ?FilterOpts, frame: *Frame) !*DOMTreeWalker { const node_filter = try NodeFilter.init(filter); @@ -334,7 +337,11 @@ pub fn nextNode(self: *DOMTreeWalker, frame: *Frame) !?*Node { } // Helper methods -fn acceptNode(self: *const DOMTreeWalker, node: *Node, frame: *Frame) !i32 { +fn acceptNode(self: *DOMTreeWalker, node: *Node, frame: *Frame) !i32 { + if (self._active) { + return error.InvalidStateError; + } + // First check whatToShow if (!NodeFilter.shouldShow(node, self._what_to_show)) { return NodeFilter.FILTER_SKIP; @@ -344,6 +351,8 @@ fn acceptNode(self: *const DOMTreeWalker, node: *Node, frame: *Frame) !i32 { // For TreeWalker, REJECT means reject node and its descendants // SKIP means skip node but check its descendants // ACCEPT means accept the node + self._active = true; + defer self._active = false; return try self._filter.acceptNode(node, frame.js.local.?); } From f64952bd35f01d90b545a703919c29718f5e8e62 Mon Sep 17 00:00:00 2001 From: Francis Bouvier Date: Sat, 11 Jul 2026 16:15:27 +0200 Subject: [PATCH 3/5] webapi: NodeFilter follows WebIDL callback interface semantics Fixes the 5 failing subtests of WPT /dom/traversal/TreeWalker-acceptNode-filter.html (7/12 -> 12/12): - A filter object was converted eagerly at createTreeWalker time, so an object without a callable acceptNode threw "invalid argument" at creation. Per WebIDL any object converts to the NodeFilter callback interface; the TypeError belongs at invocation time. - The acceptNode member was cached at conversion. Per "call a user object's operation" it must be looked up with a fresh Get on every traversal, rethrowing errors from a throwing getter. - The callback was invoked with the default this; the spec requires the filter object itself as the this value. NodeFilter now stores the raw function or object (js.Object.Global) and performs the per-invocation lookup, callability check and this-binding in acceptNode. Coverage: /dom/traversal/TreeWalker-acceptNode-filter.html 7/12 -> 12/12 (fully green). Co-Authored-By: Claude Fable 5 --- src/browser/webapi/DOMNodeIterator.zig | 2 +- src/browser/webapi/DOMTreeWalker.zig | 2 +- src/browser/webapi/NodeFilter.zig | 45 +++++++++++++++----------- 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/browser/webapi/DOMNodeIterator.zig b/src/browser/webapi/DOMNodeIterator.zig index 9951155ce..6d23cefd1 100644 --- a/src/browser/webapi/DOMNodeIterator.zig +++ b/src/browser/webapi/DOMNodeIterator.zig @@ -77,7 +77,7 @@ pub fn getWhatToShow(self: *const DOMNodeIterator) u32 { } pub fn getFilter(self: *const DOMNodeIterator) ?FilterOpts { - return self._filter._original_filter; + return self._filter._opts; } pub fn nextNode(self: *DOMNodeIterator, frame: *Frame) !?*Node { diff --git a/src/browser/webapi/DOMTreeWalker.zig b/src/browser/webapi/DOMTreeWalker.zig index b6ec9c2d2..1949f1292 100644 --- a/src/browser/webapi/DOMTreeWalker.zig +++ b/src/browser/webapi/DOMTreeWalker.zig @@ -69,7 +69,7 @@ pub fn getWhatToShow(self: *const DOMTreeWalker) u32 { } pub fn getFilter(self: *const DOMTreeWalker) ?FilterOpts { - return self._filter._original_filter; + return self._filter._opts; } pub fn getCurrentNode(self: *const DOMTreeWalker) *Node { diff --git a/src/browser/webapi/NodeFilter.zig b/src/browser/webapi/NodeFilter.zig index b0abe9e23..598e4889f 100644 --- a/src/browser/webapi/NodeFilter.zig +++ b/src/browser/webapi/NodeFilter.zig @@ -21,32 +21,24 @@ const Node = @import("Node.zig"); const NodeFilter = @This(); -_func: ?js.Function.Global, -_original_filter: ?FilterOpts, +_opts: ?FilterOpts, pub const FilterOpts = union(enum) { function: js.Function.Global, - object: struct { - pub const js_as_object = true; - acceptNode: js.Function.Global, - }, + // Any object is a valid callback interface; whether its acceptNode + // member is callable is only checked when the filter is invoked. + object: js.Object.Global, }; pub fn init(opts_: ?FilterOpts) !NodeFilter { - const opts = opts_ orelse return .{ ._func = null, ._original_filter = null }; - const func = switch (opts) { - .function => |func| func, - .object => |obj| obj.acceptNode, - }; - return .{ - ._func = func, - ._original_filter = opts_, - }; + return .{ ._opts = opts_ }; } pub fn deinit(self: *const NodeFilter) void { - if (self._func) |func| { - func.release(); + const opts = self._opts orelse return; + switch (opts) { + .function => |func| func.release(), + .object => |obj| obj.release(), } } @@ -71,8 +63,23 @@ pub const SHOW_DOCUMENT_FRAGMENT: u32 = 0x400; pub const SHOW_NOTATION: u32 = 0x800; pub fn acceptNode(self: *const NodeFilter, node: *Node, local: *const js.Local) !i32 { - const func = self._func orelse return FILTER_ACCEPT; - return local.toLocal(func).callRethrow(i32, .{node}); + const opts = self._opts orelse return FILTER_ACCEPT; + switch (opts) { + .function => |func| return local.toLocal(func).callRethrow(i32, .{node}), + .object => |obj| { + // Per WebIDL "call a user object's operation": the acceptNode + // member is looked up on every invocation (rethrowing getter + // errors), must be callable (TypeError otherwise), and is + // invoked with the filter object as its this value. + const filter_obj = obj.local(local); + const member = try filter_obj.get("acceptNode"); + if (!member.isFunction()) { + return error.TypeError; + } + const func = js.Function{ .local = local, .handle = @ptrCast(member.handle) }; + return func.callWithThisRethrow(i32, filter_obj, .{node}); + }, + } } pub fn shouldShow(node: *const Node, what_to_show: u32) bool { From 20df3d582081c3dba548be1cbee949f796540064 Mon Sep 17 00:00:00 2001 From: Francis Bouvier Date: Sat, 11 Jul 2026 16:22:50 +0200 Subject: [PATCH 4/5] webapi: implement NodeIterator pre-removing steps Fixes WPT /dom/traversal/NodeIterator-removal.html (0/23 -> 23/23) and /dom/nodes/moveBefore/moveBefore-nodeiterator.html: removing a node that is an inclusive ancestor of a live NodeIterator's reference node must move the reference per the DOM pre-removing steps. We never adjusted iterators, so the reference kept pointing into the detached subtree. The frame keeps an intrusive list of live NodeIterators (mirroring _live_ranges; iterators are slab-allocated for the frame lifetime, so they are never unlinked) and Frame.removeNode runs the steps while the tree is still intact: - removing the root or an ancestor of the root leaves the iterator untouched (matching browsers and the WPT model); - with the pointer before the reference, the reference moves to the first node following the removed subtree, if any; - otherwise (or when there is no such node) it moves to the node immediately preceding the removed node in tree order, clearing the before-pointer in the fallthrough case. Coverage: - /dom/traversal/NodeIterator-removal.html 0/23 -> 23/23 (fully green) - /dom/nodes/moveBefore/moveBefore-nodeiterator.html 0/1 -> 1/1 Co-Authored-By: Claude Fable 5 --- src/browser/Frame.zig | 13 ++++++++ src/browser/webapi/DOMNodeIterator.zig | 42 +++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/browser/Frame.zig b/src/browser/Frame.zig index 23b7122cc..31d25fa9c 100644 --- a/src/browser/Frame.zig +++ b/src/browser/Frame.zig @@ -51,6 +51,7 @@ const Performance = @import("webapi/Performance.zig"); const Screen = @import("webapi/Screen.zig"); const VisualViewport = @import("webapi/VisualViewport.zig"); const AbstractRange = @import("webapi/AbstractRange.zig"); +const DOMNodeIterator = @import("webapi/DOMNodeIterator.zig"); const Worker = @import("webapi/Worker.zig"); const CSSStyleSheet = @import("webapi/css/CSSStyleSheet.zig"); const CustomElementDefinition = @import("webapi/CustomElementDefinition.zig"); @@ -189,6 +190,9 @@ _http_owner: HttpClient.Owner = .{}, // List of active live ranges (for mutation updates per DOM spec) _live_ranges: std.DoublyLinkedList = .{}, +// Live NodeIterators for the DOM pre-removing steps. Iterators are +// slab-allocated (frame lifetime) and never unlinked. +_live_node_iterators: std.DoublyLinkedList = .{}, // List of open BroadcastChannels, used to route postMessage between same-named // channels in this frame's origin @@ -2379,6 +2383,15 @@ const RemoveNodeOpts = struct { notify_observers: bool = true, }; pub fn removeNode(self: *Frame, parent: *Node, child: *Node, opts: RemoveNodeOpts) void { + // NodeIterator pre-removing steps must run while the tree is intact. + if (self._live_node_iterators.first != null) { + var it: ?*std.DoublyLinkedList.Node = self._live_node_iterators.first; + while (it) |link| : (it = link.next) { + const iterator: *DOMNodeIterator = @fieldParentPtr("_iterator_link", link); + iterator.nodeWillBeRemoved(child); + } + } + // Capture siblings before removing const previous_sibling = child.previousSibling(); const next_sibling = child.nextSibling(); diff --git a/src/browser/webapi/DOMNodeIterator.zig b/src/browser/webapi/DOMNodeIterator.zig index 6d23cefd1..818dc3d2d 100644 --- a/src/browser/webapi/DOMNodeIterator.zig +++ b/src/browser/webapi/DOMNodeIterator.zig @@ -16,6 +16,7 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . +const std = @import("std"); const lp = @import("lightpanda"); const js = @import("../js/js.zig"); @@ -35,16 +36,55 @@ _filter: NodeFilter, _reference_node: *Node, _pointer_before_reference_node: bool, _active: bool = false, +// Intrusive link for Frame._live_node_iterators (pre-removing steps). +// Iterators are slab-allocated and live as long as the frame, so they are +// never unlinked. +_iterator_link: std.DoublyLinkedList.Node = .{}, pub fn init(root: *Node, what_to_show: u32, filter: ?FilterOpts, frame: *Frame) !*DOMNodeIterator { const node_filter = try NodeFilter.init(filter); - return frame._factory.create(DOMNodeIterator{ + const iterator = try frame._factory.create(DOMNodeIterator{ ._root = root, ._filter = node_filter, ._reference_node = root, ._what_to_show = what_to_show, ._pointer_before_reference_node = true, }); + frame._live_node_iterators.append(&iterator._iterator_link); + return iterator; +} + +// DOM "node iterator pre-removing steps", run while the tree still contains +// to_be_removed. +pub fn nodeWillBeRemoved(self: *DOMNodeIterator, to_be_removed: *Node) void { + // Removing the root or one of its ancestors leaves the iterator alone. + if (to_be_removed == self._root or to_be_removed.contains(self._root)) return; + if (to_be_removed != self._reference_node and !to_be_removed.contains(self._reference_node)) return; + + if (self._pointer_before_reference_node) { + // The first node following to_be_removed's subtree, if any. + var node = to_be_removed; + while (true) { + if (node.nextSibling()) |sibling| { + self._reference_node = sibling; + return; + } + node = node.parentNode() orelse break; + } + self._pointer_before_reference_node = false; + } + + // The node immediately preceding to_be_removed in tree order: the + // previous sibling's last inclusive descendant, or the parent. + if (to_be_removed.previousSibling()) |prev| { + var node = prev; + while (node.lastChild()) |child| { + node = child; + } + self._reference_node = node; + } else { + self._reference_node = to_be_removed.parentNode() orelse self._root; + } } pub fn deinit(self: *DOMNodeIterator, page: *Page) void { From fe5012c48206be44ea8c204b08f76559f3f30281 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 16 Jul 2026 09:55:40 +0800 Subject: [PATCH 5/5] fix potential UAF --- src/browser/webapi/DOMNodeIterator.zig | 54 ++++++++++++++------------ src/browser/webapi/DOMTreeWalker.zig | 2 - src/browser/webapi/NodeFilter.zig | 8 +--- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/browser/webapi/DOMNodeIterator.zig b/src/browser/webapi/DOMNodeIterator.zig index 818dc3d2d..f7ef1ec34 100644 --- a/src/browser/webapi/DOMNodeIterator.zig +++ b/src/browser/webapi/DOMNodeIterator.zig @@ -36,9 +36,7 @@ _filter: NodeFilter, _reference_node: *Node, _pointer_before_reference_node: bool, _active: bool = false, -// Intrusive link for Frame._live_node_iterators (pre-removing steps). -// Iterators are slab-allocated and live as long as the frame, so they are -// never unlinked. +_frame_loader_id: u32, _iterator_link: std.DoublyLinkedList.Node = .{}, pub fn init(root: *Node, what_to_show: u32, filter: ?FilterOpts, frame: *Frame) !*DOMNodeIterator { @@ -48,23 +46,48 @@ pub fn init(root: *Node, what_to_show: u32, filter: ?FilterOpts, frame: *Frame) ._filter = node_filter, ._reference_node = root, ._what_to_show = what_to_show, + ._frame_loader_id = frame._loader_id, ._pointer_before_reference_node = true, }); frame._live_node_iterators.append(&iterator._iterator_link); return iterator; } +pub fn deinit(self: *DOMNodeIterator, page: *Page) void { + if (page.findFrameByLoaderId(self._frame_loader_id)) |frame| { + frame._live_node_iterators.remove(&self._iterator_link); + } + self._filter.deinit(); + page.factory.destroy(self); +} + +pub fn releaseRef(self: *DOMNodeIterator, page: *Page) void { + self._rc.release(self, page); +} + +pub fn acquireRef(self: *DOMNodeIterator) void { + self._rc.acquire(); +} + +pub fn getRoot(self: *const DOMNodeIterator) *Node { + return self._root; +} + // DOM "node iterator pre-removing steps", run while the tree still contains // to_be_removed. pub fn nodeWillBeRemoved(self: *DOMNodeIterator, to_be_removed: *Node) void { - // Removing the root or one of its ancestors leaves the iterator alone. - if (to_be_removed == self._root or to_be_removed.contains(self._root)) return; - if (to_be_removed != self._reference_node and !to_be_removed.contains(self._reference_node)) return; + if (to_be_removed.contains(self._root)) { + // Removing the root or one of its ancestors leaves the iterator alone. + return; + } + if (to_be_removed != self._reference_node and to_be_removed.contains(self._reference_node) == false) { + return; + } if (self._pointer_before_reference_node) { // The first node following to_be_removed's subtree, if any. var node = to_be_removed; - while (true) { + while (node != self._root) { if (node.nextSibling()) |sibling| { self._reference_node = sibling; return; @@ -87,23 +110,6 @@ pub fn nodeWillBeRemoved(self: *DOMNodeIterator, to_be_removed: *Node) void { } } -pub fn deinit(self: *DOMNodeIterator, page: *Page) void { - self._filter.deinit(); - page.factory.destroy(self); -} - -pub fn releaseRef(self: *DOMNodeIterator, page: *Page) void { - self._rc.release(self, page); -} - -pub fn acquireRef(self: *DOMNodeIterator) void { - self._rc.acquire(); -} - -pub fn getRoot(self: *const DOMNodeIterator) *Node { - return self._root; -} - pub fn getReferenceNode(self: *const DOMNodeIterator) *Node { return self._reference_node; } diff --git a/src/browser/webapi/DOMTreeWalker.zig b/src/browser/webapi/DOMTreeWalker.zig index 1949f1292..0e07dab95 100644 --- a/src/browser/webapi/DOMTreeWalker.zig +++ b/src/browser/webapi/DOMTreeWalker.zig @@ -33,8 +33,6 @@ _root: *Node, _what_to_show: u32, _filter: NodeFilter, _current: *Node, -// Set while the filter callback runs; a walker method called from inside -// its own filter must throw InvalidStateError. _active: bool = false, pub fn init(root: *Node, what_to_show: u32, filter: ?FilterOpts, frame: *Frame) !*DOMTreeWalker { diff --git a/src/browser/webapi/NodeFilter.zig b/src/browser/webapi/NodeFilter.zig index 598e4889f..8e44afe4f 100644 --- a/src/browser/webapi/NodeFilter.zig +++ b/src/browser/webapi/NodeFilter.zig @@ -25,8 +25,7 @@ _opts: ?FilterOpts, pub const FilterOpts = union(enum) { function: js.Function.Global, - // Any object is a valid callback interface; whether its acceptNode - // member is callable is only checked when the filter is invoked. + // Per spec, the validity of this has to be checked in each acceptNode call. object: js.Object.Global, }; @@ -67,10 +66,7 @@ pub fn acceptNode(self: *const NodeFilter, node: *Node, local: *const js.Local) switch (opts) { .function => |func| return local.toLocal(func).callRethrow(i32, .{node}), .object => |obj| { - // Per WebIDL "call a user object's operation": the acceptNode - // member is looked up on every invocation (rethrowing getter - // errors), must be callable (TypeError otherwise), and is - // invoked with the filter object as its this value. + // Per spec, the acceptNode member is looked up on every invocation const filter_obj = obj.local(local); const member = try filter_obj.get("acceptNode"); if (!member.isFunction()) {