From b4014e8c2481103a9d95fc7589be2bb6a5dcb92d Mon Sep 17 00:00:00 2001 From: Srikanth Date: Sun, 3 Aug 2025 20:27:32 +0530 Subject: [PATCH 1/4] Fix: Properly handle node ownership when using appendChild and insertBefore --- src/browser/dom/node.zig | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/browser/dom/node.zig b/src/browser/dom/node.zig index 66967e78..ef2a07e4 100644 --- a/src/browser/dom/node.zig +++ b/src/browser/dom/node.zig @@ -198,6 +198,20 @@ pub const Node = struct { // Methods pub fn _appendChild(self: *parser.Node, child: *parser.Node) !Union { + const self_owner = try parser.nodeOwnerDocument(self); + const new_node_owner = try parser.nodeOwnerDocument(child); + + if (new_node_owner == null or (new_node_owner.? != self_owner.?)) { + child.owner = self_owner; + // recursively set the owner for all children + var next: ?*parser.Node = child; + while (next) |n| { + next = try parser.nodeFirstChild(n) orelse break; + if (next) |child_next| { + child_next.owner = self_owner; + } + } + } // TODO: DocumentFragment special case const res = try parser.nodeAppendChild(self, child); return try Node.toInterface(res); @@ -290,6 +304,18 @@ pub const Node = struct { } pub fn _insertBefore(self: *parser.Node, new_node: *parser.Node, ref_node_: ?*parser.Node) !Union { + const self_owner = try parser.nodeOwnerDocument(self); + const new_node_owner = try parser.nodeOwnerDocument(new_node); + if (new_node_owner == null or (new_node_owner.? != self_owner.?)) { + new_node.owner = self_owner; + var next: ?*parser.Node = new_node; + while (next) |n| { + next = try parser.nodeFirstChild(n) orelse break; + if (next) |child_next| { + child_next.owner = self_owner; + } + } + } if (ref_node_) |ref_node| { return Node.toInterface(try parser.nodeInsertBefore(self, new_node, ref_node)); } From c533b10e19bd6fb08d720d95e955ce5b434e988e Mon Sep 17 00:00:00 2001 From: Srikanth Date: Mon, 4 Aug 2025 13:00:03 +0530 Subject: [PATCH 2/4] fix: traverse all children correctly --- src/browser/dom/node.zig | 43 ++++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/src/browser/dom/node.zig b/src/browser/dom/node.zig index ef2a07e4..08bd9815 100644 --- a/src/browser/dom/node.zig +++ b/src/browser/dom/node.zig @@ -202,16 +202,26 @@ pub const Node = struct { const new_node_owner = try parser.nodeOwnerDocument(child); if (new_node_owner == null or (new_node_owner.? != self_owner.?)) { + var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator); + defer arena.deinit(); + + const allocator = arena.allocator(); + var worklist = std.ArrayList(*parser.Node).init(allocator); + child.owner = self_owner; - // recursively set the owner for all children - var next: ?*parser.Node = child; - while (next) |n| { - next = try parser.nodeFirstChild(n) orelse break; - if (next) |child_next| { - child_next.owner = self_owner; + try worklist.append(child); + + // change ownerDocument of all children of a given node + while (worklist.pop()) |current_node| { + var maybe_child: ?*parser.Node = try parser.nodeFirstChild(current_node); + while (maybe_child) |child_node| { + child_node.owner = self_owner; + try worklist.append(child_node); + maybe_child = try parser.nodeNextSibling(child_node); } } } + // TODO: DocumentFragment special case const res = try parser.nodeAppendChild(self, child); return try Node.toInterface(res); @@ -306,15 +316,26 @@ pub const Node = struct { pub fn _insertBefore(self: *parser.Node, new_node: *parser.Node, ref_node_: ?*parser.Node) !Union { const self_owner = try parser.nodeOwnerDocument(self); const new_node_owner = try parser.nodeOwnerDocument(new_node); + if (new_node_owner == null or (new_node_owner.? != self_owner.?)) { + + var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator); + defer arena.deinit(); + const allocator = arena.allocator(); + + var worklist = std.ArrayList(*parser.Node).init(allocator); new_node.owner = self_owner; - var next: ?*parser.Node = new_node; - while (next) |n| { - next = try parser.nodeFirstChild(n) orelse break; - if (next) |child_next| { - child_next.owner = self_owner; + try worklist.append(new_node); + + while (worklist.pop()) |current_node| { + var maybe_child: ?*parser.Node = try parser.nodeFirstChild(current_node); + while (maybe_child) |child| { + child.owner = self_owner; + try worklist.append(child); + maybe_child = try parser.nodeNextSibling(child); } } + } if (ref_node_) |ref_node| { return Node.toInterface(try parser.nodeInsertBefore(self, new_node, ref_node)); From f1e513443bca80f899ed28fdb724f403904e1cfe Mon Sep 17 00:00:00 2001 From: Srikanth Date: Mon, 4 Aug 2025 14:27:39 +0530 Subject: [PATCH 3/4] refactor: use walker to traverse the nodes --- src/browser/dom/node.zig | 47 +++++++++++----------------------------- 1 file changed, 13 insertions(+), 34 deletions(-) diff --git a/src/browser/dom/node.zig b/src/browser/dom/node.zig index 08bd9815..af462de2 100644 --- a/src/browser/dom/node.zig +++ b/src/browser/dom/node.zig @@ -199,26 +199,15 @@ pub const Node = struct { pub fn _appendChild(self: *parser.Node, child: *parser.Node) !Union { const self_owner = try parser.nodeOwnerDocument(self); - const new_node_owner = try parser.nodeOwnerDocument(child); - - if (new_node_owner == null or (new_node_owner.? != self_owner.?)) { - var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator); - defer arena.deinit(); - - const allocator = arena.allocator(); - var worklist = std.ArrayList(*parser.Node).init(allocator); + const child_owner = try parser.nodeOwnerDocument(child); + if (child_owner == null or (child_owner.? != self_owner.?)) { + const w = Walker{}; child.owner = self_owner; - try worklist.append(child); - - // change ownerDocument of all children of a given node - while (worklist.pop()) |current_node| { - var maybe_child: ?*parser.Node = try parser.nodeFirstChild(current_node); - while (maybe_child) |child_node| { - child_node.owner = self_owner; - try worklist.append(child_node); - maybe_child = try parser.nodeNextSibling(child_node); - } + var current = child; + while (try w.get_next(child, current)) |current_node| { + current_node.owner = self_owner; + current = current_node; } } @@ -318,25 +307,15 @@ pub const Node = struct { const new_node_owner = try parser.nodeOwnerDocument(new_node); if (new_node_owner == null or (new_node_owner.? != self_owner.?)) { - - var arena = std.heap.ArenaAllocator.init(std.heap.page_allocator); - defer arena.deinit(); - const allocator = arena.allocator(); - - var worklist = std.ArrayList(*parser.Node).init(allocator); + const w = Walker{}; new_node.owner = self_owner; - try worklist.append(new_node); - - while (worklist.pop()) |current_node| { - var maybe_child: ?*parser.Node = try parser.nodeFirstChild(current_node); - while (maybe_child) |child| { - child.owner = self_owner; - try worklist.append(child); - maybe_child = try parser.nodeNextSibling(child); - } + var current = new_node; + while (try w.get_next(new_node, current)) |current_node| { + current_node.owner = self_owner; + current = current_node; } - } + if (ref_node_) |ref_node| { return Node.toInterface(try parser.nodeInsertBefore(self, new_node, ref_node)); } From 0a5f060d1b7b21054b89c1a47e107ce291627953 Mon Sep 17 00:00:00 2001 From: Srikanth Date: Mon, 4 Aug 2025 23:53:29 +0530 Subject: [PATCH 4/4] add tests and simplify walker traversal --- src/browser/dom/node.zig | 69 +++++++++++++++++++++++++++++++++++----- 1 file changed, 61 insertions(+), 8 deletions(-) diff --git a/src/browser/dom/node.zig b/src/browser/dom/node.zig index af462de2..3dd6a06f 100644 --- a/src/browser/dom/node.zig +++ b/src/browser/dom/node.zig @@ -201,13 +201,17 @@ pub const Node = struct { const self_owner = try parser.nodeOwnerDocument(self); const child_owner = try parser.nodeOwnerDocument(child); + // If the node to be inserted has a different ownerDocument than the parent node, + // modern browsers automatically adopt the node and its descendants into + // the parent's ownerDocument. + // This process is known as adoption. + // (7.1) https://dom.spec.whatwg.org/#concept-node-insert if (child_owner == null or (child_owner.? != self_owner.?)) { const w = Walker{}; - child.owner = self_owner; var current = child; - while (try w.get_next(child, current)) |current_node| { - current_node.owner = self_owner; - current = current_node; + while (true) { + current.owner = self_owner; + current = try w.get_next(child, current) orelse break; } } @@ -306,13 +310,17 @@ pub const Node = struct { const self_owner = try parser.nodeOwnerDocument(self); const new_node_owner = try parser.nodeOwnerDocument(new_node); + // If the node to be inserted has a different ownerDocument than the parent node, + // modern browsers automatically adopt the node and its descendants into + // the parent's ownerDocument. + // This process is known as adoption. + // (7.1) https://dom.spec.whatwg.org/#concept-node-insert if (new_node_owner == null or (new_node_owner.? != self_owner.?)) { const w = Walker{}; - new_node.owner = self_owner; var current = new_node; - while (try w.get_next(new_node, current)) |current_node| { - current_node.owner = self_owner; - current = current_node; + while(true) { + current.owner = self_owner; + current = try w.get_next(new_node, current) orelse break; } } @@ -746,3 +754,48 @@ test "Browser.DOM.node" { .{ "Node.NOTATION_NODE", "12" }, }, .{}); } + +test "Browser.DOM.node.owner" { + var runner = try testing.jsRunner(testing.tracking_allocator, .{ .html= + \\
+ \\

+ \\ I am the original reference node. + \\

+ \\
" + }); + + defer runner.deinit(); + + try runner.testCases(&.{ + .{ + \\ const parser = new DOMParser(); + \\ const newDoc = parser.parseFromString('

Hey

Marked
', 'text/html'); + + \\ const newNode = newDoc.getElementById('new-node'); + + \\ const parent = document.getElementById('target-container'); + \\ const referenceNode = document.getElementById('reference-node'); + + \\ parent.insertBefore(newNode, referenceNode); + \\ const k = document.getElementById('new-node'); + \\ const ptag = k.querySelector('p'); + \\ const spanTag = k.querySelector('span'); + \\ const anotherDoc = parser.parseFromString('
', 'text/html'); + \\ const anotherNewNode = anotherDoc.getElementById('another-new-node'); + \\ + \\ parent.appendChild(anotherNewNode) + , + "[object HTMLDivElement]", + }, + + .{"parent.ownerDocument === newNode.ownerDocument", "true" }, + .{"parent.ownerDocument === anotherNewNode.ownerDocument", "true"}, + .{"newNode.firstChild.nodeName", "P"}, + .{"ptag.ownerDocument === parent.ownerDocument", "true"}, + .{"spanTag.ownerDocument === parent.ownerDocument", "true"}, + .{"parent.contains(newNode)", "true"}, + .{"parent.contains(anotherNewNode)", "true"}, + .{"anotherDoc.contains(anotherNewNode)", "false"}, + .{"newDoc.contains(newNode)", "false"}, + }, .{}); +} \ No newline at end of file