From e60424a40209c059839c56b40ba755f8a8763e2f Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Sat, 21 Mar 2026 19:39:49 +0800 Subject: [PATCH] Add validation to replaceChildren Extract Document.replaceChildren, Element.replaceChildren and DocumentFragment.replaceChildren into a common helper, Node.replaceChildren. Fixes an infinite loop in WPT test: /dom/nodes/ParentNode-replaceChildren.html --- .../tests/document/replace_children.html | 1 + .../tests/element/replace_children.html | 139 ++++++++++++++++++ src/browser/webapi/Document.zig | 39 ++--- src/browser/webapi/DocumentFragment.zig | 20 +-- src/browser/webapi/Element.zig | 19 +-- src/browser/webapi/Node.zig | 43 ++++++ 6 files changed, 195 insertions(+), 66 deletions(-) create mode 100644 src/browser/tests/element/replace_children.html diff --git a/src/browser/tests/document/replace_children.html b/src/browser/tests/document/replace_children.html index 010f3b0d..dca85db9 100644 --- a/src/browser/tests/document/replace_children.html +++ b/src/browser/tests/document/replace_children.html @@ -342,3 +342,4 @@ testing.expectEqual('html', doc.lastChild.nodeName); } + diff --git a/src/browser/tests/element/replace_children.html b/src/browser/tests/element/replace_children.html new file mode 100644 index 00000000..fdadcb77 --- /dev/null +++ b/src/browser/tests/element/replace_children.html @@ -0,0 +1,139 @@ + + + + + element.replaceChildren Tests + + +
Original content
+ + + + + + + + + + + + + + + + + + + diff --git a/src/browser/webapi/Document.zig b/src/browser/webapi/Document.zig index 6d2244fb..45510a89 100644 --- a/src/browser/webapi/Document.zig +++ b/src/browser/webapi/Document.zig @@ -548,35 +548,8 @@ pub fn prepend(self: *Document, nodes: []const Node.NodeOrText, page: *Page) !vo } pub fn replaceChildren(self: *Document, nodes: []const Node.NodeOrText, page: *Page) !void { - try validateDocumentNodes(self, nodes, true); - - page.domChanged(); - const parent = self.asNode(); - - // Remove all existing children - var it = parent.childrenIterator(); - while (it.next()) |child| { - page.removeNode(parent, child, .{ .will_be_reconnected = false }); - } - - // Append new children - const parent_is_connected = parent.isConnected(); - for (nodes) |node_or_text| { - const child = try node_or_text.toNode(page); - - // DocumentFragments are special - append all their children - if (child.is(Node.DocumentFragment)) |_| { - try page.appendAllChildren(child, parent); - continue; - } - - var child_connected = false; - if (child._parent) |previous_parent| { - child_connected = child.isConnected(); - page.removeNode(previous_parent, child, .{ .will_be_reconnected = parent_is_connected }); - } - try page.appendNode(parent, child, .{ .child_already_connected = child_connected }); - } + try validateDocumentNodes(self, nodes, false); + return self.asNode().replaceChildren(nodes, page); } pub fn elementFromPoint(self: *Document, x: f64, y: f64, page: *Page) !?*Element { @@ -896,6 +869,10 @@ fn validateDocumentNodes(self: *Document, nodes: []const Node.NodeOrText, compti if (has_doctype) { return error.HierarchyError; } + if (has_element) { + // Doctype cannot be inserted if document already has an element + return error.HierarchyError; + } has_doctype = true; }, .cdata => |cd| switch (cd._type) { @@ -918,6 +895,10 @@ fn validateDocumentNodes(self: *Document, nodes: []const Node.NodeOrText, compti if (has_doctype) { return error.HierarchyError; } + if (has_element) { + // Doctype cannot be inserted if document already has an element + return error.HierarchyError; + } has_doctype = true; }, .cdata => |cd| switch (cd._type) { diff --git a/src/browser/webapi/DocumentFragment.zig b/src/browser/webapi/DocumentFragment.zig index 004ee916..cab73cb4 100644 --- a/src/browser/webapi/DocumentFragment.zig +++ b/src/browser/webapi/DocumentFragment.zig @@ -143,25 +143,7 @@ pub fn prepend(self: *DocumentFragment, nodes: []const Node.NodeOrText, page: *P } pub fn replaceChildren(self: *DocumentFragment, nodes: []const Node.NodeOrText, page: *Page) !void { - page.domChanged(); - var parent = self.asNode(); - - var it = parent.childrenIterator(); - while (it.next()) |child| { - page.removeNode(parent, child, .{ .will_be_reconnected = false }); - } - - const parent_is_connected = parent.isConnected(); - for (nodes) |node_or_text| { - const child = try node_or_text.toNode(page); - - // If the new children has already a parent, remove from it. - if (child._parent) |p| { - page.removeNode(p, child, .{ .will_be_reconnected = true }); - } - - try page.appendNode(parent, child, .{ .child_already_connected = parent_is_connected }); - } + return self.asNode().replaceChildren(nodes, page); } pub fn getInnerHTML(self: *DocumentFragment, writer: *std.Io.Writer, page: *Page) !void { diff --git a/src/browser/webapi/Element.zig b/src/browser/webapi/Element.zig index 2b5db1f7..160e31f4 100644 --- a/src/browser/webapi/Element.zig +++ b/src/browser/webapi/Element.zig @@ -784,24 +784,7 @@ pub fn getDataset(self: *Element, page: *Page) !*DOMStringMap { } pub fn replaceChildren(self: *Element, nodes: []const Node.NodeOrText, page: *Page) !void { - page.domChanged(); - var parent = self.asNode(); - - var it = parent.childrenIterator(); - while (it.next()) |child| { - page.removeNode(parent, child, .{ .will_be_reconnected = false }); - } - - const parent_is_connected = parent.isConnected(); - for (nodes) |node_or_text| { - var child_connected = false; - const child = try node_or_text.toNode(page); - if (child._parent) |previous_parent| { - child_connected = child.isConnected(); - page.removeNode(previous_parent, child, .{ .will_be_reconnected = parent_is_connected }); - } - try page.appendNode(parent, child, .{ .child_already_connected = child_connected }); - } + return self.asNode().replaceChildren(nodes, page); } pub fn replaceWith(self: *Element, nodes: []const Node.NodeOrText, page: *Page) !void { diff --git a/src/browser/webapi/Node.zig b/src/browser/webapi/Node.zig index 2eca4047..0e7c2ffe 100644 --- a/src/browser/webapi/Node.zig +++ b/src/browser/webapi/Node.zig @@ -1005,6 +1005,49 @@ pub fn getElementsByClassName(self: *Node, class_name: []const u8, page: *Page) return collections.NodeLive(.class_name).init(self, class_names.items, page); } +/// Shared implementation of replaceChildren for Element, Document, and DocumentFragment. +/// Validates all nodes, removes existing children, then appends new children. +pub fn replaceChildren(self: *Node, nodes: []const NodeOrText, page: *Page) !void { + // First pass: validate all nodes and collect them + // We need to collect because DocumentFragments contribute their children, not themselves + var children_to_add: std.ArrayList(*Node) = .empty; + + for (nodes) |node_or_text| { + const child = try node_or_text.toNode(page); + + // DocumentFragments contribute their children, not themselves + if (child.is(DocumentFragment)) |frag| { + var frag_it = frag.asNode().childrenIterator(); + while (frag_it.next()) |frag_child| { + try validateNodeInsertion(self, frag_child); + try children_to_add.append(page.call_arena, frag_child); + } + } else { + try validateNodeInsertion(self, child); + try children_to_add.append(page.call_arena, child); + } + } + + page.domChanged(); + + // Remove all existing children + var it = self.childrenIterator(); + while (it.next()) |child| { + page.removeNode(self, child, .{ .will_be_reconnected = false }); + } + + // Append new children + const parent_is_connected = self.isConnected(); + for (children_to_add.items) |child| { + var child_connected = false; + if (child._parent) |previous_parent| { + child_connected = child.isConnected(); + page.removeNode(previous_parent, child, .{ .will_be_reconnected = parent_is_connected }); + } + try page.appendNode(self, child, .{ .child_already_connected = child_connected }); + } +} + // Writes a JSON representation of the node and its children pub fn jsonStringify(self: *const Node, writer: *std.json.Stringify) !void { // stupid json api requires this to be const,