From 3a4bdaf4c63b9cd42e6c169ff0306accfb00e3e3 Mon Sep 17 00:00:00 2001 From: Francis Bouvier Date: Sat, 11 Jul 2026 13:57:31 +0200 Subject: [PATCH] webapi: replace-all queues one combined mutation record Fixes WPT /dom/nodes/MutationObserver-textContent.html (1/4) and the remaining failures of ParentNode-replaceChildren.html (25/29): the DOM "replace all" algorithm (Element.textContent setter, ParentNode.replaceChildren) must queue a single tree mutation record with all removedNodes and addedNodes (and null previous/next sibling). We emitted one record per removed child plus one per added child, so observers saw 2+ records where the spec requires exactly one. Frame.removeNode/appendNode gain a notify_observers opt (default true); Node.replaceChildren suppresses the per-node records and queues the combined record itself. Removing an added child from its previous parent still notifies that parent's observers separately, as the spec requires. Coverage: - /dom/nodes/MutationObserver-textContent.html 1/4 -> 4/4 (fully green) - /dom/nodes/ParentNode-replaceChildren.html 25/29 -> 29/29 (fully green) - /dom/nodes/MutationObserver-childList.html 29/38 -> 30/38 Co-Authored-By: Claude Fable 5 --- src/browser/Frame.zig | 12 ++++++++++-- src/browser/webapi/Node.zig | 17 +++++++++++++++-- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/browser/Frame.zig b/src/browser/Frame.zig index 54dc7a4c7..6adba3bbf 100644 --- a/src/browser/Frame.zig +++ b/src/browser/Frame.zig @@ -2364,6 +2364,9 @@ pub fn dupeSSO(self: *Frame, value: []const u8) !String { const RemoveNodeOpts = struct { will_be_reconnected: bool, + // Set to false when the caller queues its own combined mutation record + // (e.g. replaceChildren's single "replace all" record). + notify_observers: bool = true, }; pub fn removeNode(self: *Frame, parent: *Node, child: *Node, opts: RemoveNodeOpts) void { // Capture siblings before removing @@ -2398,7 +2401,7 @@ pub fn removeNode(self: *Frame, parent: *Node, child: *Node, opts: RemoveNodeOpt slotting.removalSteps(parent, child, self); - if (observers.hasMutationObservers(self)) { + if (opts.notify_observers and observers.hasMutationObservers(self)) { const removed = [_]*Node{child}; observers.notifyChildListChange(self, parent, &.{}, &removed, previous_sibling, next_sibling); } @@ -2497,6 +2500,9 @@ const InsertNodeRelative = union(enum) { const InsertNodeOpts = struct { child_already_connected: bool = false, adopting_to_new_document: bool = false, + // Set to false when the caller queues its own combined mutation record + // (e.g. replaceChildren's single "replace all" record). + notify_observers: bool = true, }; pub fn insertNodeRelative(self: *Frame, parent: *Node, child: *Node, relative: InsertNodeRelative, opts: InsertNodeOpts) !void { return self._insertNodeRelative(false, parent, child, relative, opts); @@ -2590,7 +2596,9 @@ pub fn _insertNodeRelative(self: *Frame, comptime from_parser: bool, parent: *No } } - self.notifyChildInserted(parent, child); + if (opts.notify_observers) { + self.notifyChildInserted(parent, child); + } if (opts.child_already_connected and !opts.adopting_to_new_document) { // The child is already connected in the same document, we don't have to reconnect it. diff --git a/src/browser/webapi/Node.zig b/src/browser/webapi/Node.zig index 8cd407988..bbe5d1f9e 100644 --- a/src/browser/webapi/Node.zig +++ b/src/browser/webapi/Node.zig @@ -1282,10 +1282,19 @@ pub fn replaceChildren(self: *Node, nodes: []const NodeOrText, frame: *Frame) !v frame.domChanged(); + // Per the "replace all" algorithm, observers get one combined mutation + // record with all removed and added nodes, so per-node notification is + // suppressed here. + const notify = Frame.observers.hasMutationObservers(frame); + var removed: std.ArrayList(*Node) = .empty; + // Remove all existing children var it = self.childrenIterator(); while (it.next()) |child| { - frame.removeNode(self, child, .{ .will_be_reconnected = false }); + if (notify) { + try removed.append(frame.call_arena, child); + } + frame.removeNode(self, child, .{ .will_be_reconnected = false, .notify_observers = false }); } // Append new children @@ -1296,7 +1305,11 @@ pub fn replaceChildren(self: *Node, nodes: []const NodeOrText, frame: *Frame) !v child_connected = child.isConnected(); frame.removeNode(previous_parent, child, .{ .will_be_reconnected = parent_is_connected }); } - try frame.appendNode(self, child, .{ .child_already_connected = child_connected }); + try frame.appendNode(self, child, .{ .child_already_connected = child_connected, .notify_observers = false }); + } + + if (notify and (removed.items.len > 0 or children_to_add.items.len > 0)) { + Frame.observers.notifyChildListChange(frame, self, children_to_add.items, removed.items, null, null); } }