From 066069baada6f370d8c1e364efc43b989edced71 Mon Sep 17 00:00:00 2001
From: Karl Seguin <k@openmymind.io>
Date: Wed, 28 Jan 2026 07:33:04 +0800
Subject: [PATCH] Add defensiveness around Parser.appendCallback

We're seeing an assertion in Page.appendNew fail because the node has a parent.
According to html5ever, this shouldn't be possible (appendNew is only called
from the Parser). BUT, it's possible we're mutating the node in a way that
we shouldn't...maybe there's JavaScript executing as we're parsing which is
mutating the node.

In release, this will be more defensive. In debug, this still crashes. It's
possible this is valid (like I said, maybe there's JS interleaved which is
mutating the node), but if so, I'd like to know the exact scenario that produces
this case.
---
 src/browser/URL.zig           |  2 +-
 src/browser/parser/Parser.zig | 12 ++++++++++++
 src/cdp/Node.zig              |  8 ++++----
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/src/browser/URL.zig b/src/browser/URL.zig
index c5177d95..356d2d55 100644
--- a/src/browser/URL.zig
+++ b/src/browser/URL.zig
@@ -95,7 +95,7 @@ pub fn resolve(allocator: Allocator, base: [:0]const u8, path: anytype, comptime
                 in_i += 2;
                 continue;
             }
-            if (out[in_i + 1] == '.' and out[in_i + 2] == '/') {  // always safe, because we added two whitespaces
+            if (out[in_i + 1] == '.' and out[in_i + 2] == '/') { // always safe, because we added two whitespaces
                 // /../
                 if (out_i > path_marker) {
                     // go back before the /
diff --git a/src/browser/parser/Parser.zig b/src/browser/parser/Parser.zig
index d2d952f4..49eb362b 100644
--- a/src/browser/parser/Parser.zig
+++ b/src/browser/parser/Parser.zig
@@ -24,6 +24,7 @@ const Page = @import("../Page.zig");
 const Node = @import("../webapi/Node.zig");
 const Element = @import("../webapi/Element.zig");
 const Allocator = std.mem.Allocator;
+const IS_DEBUG = @import("builtin").mode == .Debug;
 
 pub const ParsedNode = struct {
     node: *Node,
@@ -373,6 +374,17 @@ fn _appendCallback(self: *Parser, parent: *Node, node_or_text: h5e.NodeOrText) !
     switch (node_or_text.toUnion()) {
         .node => |cpn| {
             const child = getNode(cpn);
+            if (child._parent) |previous_parent| {
+                // html5ever says this can't happen, but we might be screwing up
+                // the node on our side. We shouldn't be, but we're seeing this
+                // in the wild, and I'm not sure why. In debug, let's crash so
+                // we can try to figure it out. In release, let's disconnect
+                // the child first.
+                if (comptime IS_DEBUG) {
+                    unreachable;
+                }
+                self._page.removeNode(previous_parent, child, .{ .will_be_reconnected = parent.isConnected() });
+            }
             try self.page.appendNew(parent, .{ .node = child });
         },
         .text => |txt| try self.page.appendNew(parent, .{ .text = txt }),
diff --git a/src/cdp/Node.zig b/src/cdp/Node.zig
index d11dd8bd..2997a438 100644
--- a/src/cdp/Node.zig
+++ b/src/cdp/Node.zig
@@ -356,7 +356,7 @@ test "cdp Node: Registry register" {
     }
 
     {
-        const dom_node = (try doc.querySelector(.wrap ("p"), page)).?.asNode();
+        const dom_node = (try doc.querySelector(.wrap("p"), page)).?.asNode();
         const node = try registry.register(dom_node);
         const n1b = registry.lookup_by_id.get(2).?;
         const n1c = registry.lookup_by_node.get(node.dom).?;
@@ -400,18 +400,18 @@ test "cdp Node: search list" {
         defer page._session.removePage();
         var doc = page.window._document;
 
-        const s1 = try search_list.create((try doc.querySelectorAll(.wrap ("a"), page))._nodes);
+        const s1 = try search_list.create((try doc.querySelectorAll(.wrap("a"), page))._nodes);
         try testing.expectEqual("1", s1.name);
         try testing.expectEqualSlices(u32, &.{ 1, 2 }, s1.node_ids);
 
         try testing.expectEqual(2, registry.lookup_by_id.count());
         try testing.expectEqual(2, registry.lookup_by_node.count());
 
-        const s2 = try search_list.create((try doc.querySelectorAll(.wrap ("#a1"), page))._nodes);
+        const s2 = try search_list.create((try doc.querySelectorAll(.wrap("#a1"), page))._nodes);
         try testing.expectEqual("2", s2.name);
         try testing.expectEqualSlices(u32, &.{1}, s2.node_ids);
 
-        const s3 = try search_list.create((try doc.querySelectorAll(.wrap ("#a2"), page))._nodes);
+        const s3 = try search_list.create((try doc.querySelectorAll(.wrap("#a2"), page))._nodes);
         try testing.expectEqual("3", s3.name);
         try testing.expectEqualSlices(u32, &.{2}, s3.node_ids);