From f93403d3dc13d5f6b0e6e76195eb9305be8ca777 Mon Sep 17 00:00:00 2001
From: Karl Seguin <k@openmymind.io>
Date: Sun, 14 Dec 2025 16:16:54 +0800
Subject: [PATCH] Remove thread local

Rework node.isConnected(), this now [correctly] returns true as long as a node
is part of _a_ document (it doesn't have to be the 'main' document). This
requires changes around id lookup optimization.
---
 src/browser/Page.zig                         |  64 ++++++----
 src/browser/tests/domparser.html             | 119 +++++++++++++++++++
 src/browser/tests/element/html/template.html |   9 +-
 src/browser/tests/node/is_connected.html     | 108 +++++++++++++++++
 src/browser/tests/node/node_iterator.html    |   1 -
 src/browser/tests/node/normalize.html        |   1 +
 src/browser/tests/node/owner.html            |  36 ++++++
 src/browser/webapi/Node.zig                  |  35 ++++--
 8 files changed, 335 insertions(+), 38 deletions(-)
 create mode 100644 src/browser/tests/node/is_connected.html
 create mode 100644 src/browser/tests/node/owner.html

diff --git a/src/browser/Page.zig b/src/browser/Page.zig
index f6eb4b06..0fd81e81 100644
--- a/src/browser/Page.zig
+++ b/src/browser/Page.zig
@@ -64,7 +64,6 @@ const NavigationKind = @import("webapi/navigation/root.zig").NavigationKind;
 const timestamp = @import("../datetime.zig").timestamp;
 const milliTimestamp = @import("../datetime.zig").milliTimestamp;
 
-pub threadlocal var current: *Page = undefined;
 var default_url = URL{ ._raw = "about:blank" };
 pub var default_location: Location = Location{ ._url = &default_url };
 
@@ -171,7 +170,6 @@ pub fn init(arena: Allocator, call_arena: Allocator, session: *Session) !*Page {
 
     page.scheduler = Scheduler.init(page.arena);
     try page.reset(true);
-    current = page;
     return page;
 }
 
@@ -794,19 +792,25 @@ pub fn domChanged(self: *Page) void {
     };
 }
 
-pub fn getElementIdMap(page: *Page, node: *Node) *std.StringHashMapUnmanaged(*Element) {
-    if (node.is(ShadowRoot)) |shadow_root| {
-        return &shadow_root._elements_by_id;
-    }
-
-    var parent = node._parent;
-    while (parent) |n| {
-        if (n.is(ShadowRoot)) |shadow_root| {
+fn getElementIdMap(page: *Page, node: *Node) *std.StringHashMapUnmanaged(*Element) {
+    // Walk up the tree checking for ShadowRoot and tracking the root
+    var current = node;
+    while (true) {
+        if (current.is(ShadowRoot)) |shadow_root| {
             return &shadow_root._elements_by_id;
         }
-        parent = n._parent;
+
+        const parent = current._parent orelse {
+            if (current._type == .document) {
+                return &current._type.document._elements_by_id;
+            }
+            // Detached nodes should not have IDs registered
+            std.debug.assert(false);
+            return &page.document._elements_by_id;
+        };
+
+        current = parent;
     }
-    return &page.document._elements_by_id;
 }
 
 pub fn addElementId(self: *Page, parent: *Node, element: *Element, id: []const u8) !void {
@@ -823,8 +827,18 @@ pub fn removeElementId(self: *Page, element: *Element, id: []const u8) void {
 }
 
 pub fn getElementByIdFromNode(self: *Page, node: *Node, id: []const u8) ?*Element {
-    const id_map = self.getElementIdMap(node);
-    return id_map.get(id);
+    if (node.isConnected() or node.isInShadowTree()) {
+        const id_map = self.getElementIdMap(node);
+        return id_map.get(id);
+    }
+    var tw = @import("webapi/TreeWalker.zig").Full.Elements.init(node, .{});
+    while (tw.next()) |el| {
+        const element_id = el.getAttributeSafe("id") orelse continue;
+        if (std.mem.eql(u8, element_id, id)) {
+            return el;
+        }
+    }
+    return null;
 }
 
 pub fn registerMutationObserver(self: *Page, observer: *MutationObserver) !void {
@@ -1509,6 +1523,8 @@ pub fn removeNode(self: *Page, parent: *Node, child: *Node, opts: RemoveNodeOpts
     }
     // grab this before we null the parent
     const was_connected = child.isConnected();
+    // Capture the ID map before disconnecting, so we can remove IDs from the correct document
+    const id_map = if (was_connected) self.getElementIdMap(child) else null;
 
     child._parent = null;
     child._child_link = .{};
@@ -1537,7 +1553,7 @@ pub fn removeNode(self: *Page, parent: *Node, child: *Node, opts: RemoveNodeOpts
     var tw = @import("webapi/TreeWalker.zig").Full.Elements.init(child, .{});
     while (tw.next()) |el| {
         if (el.getAttributeSafe("id")) |id| {
-            self.removeElementId(el, id);
+            _ = id_map.?.remove(id);
         }
 
         Element.Html.Custom.invokeDisconnectedCallbackOnElement(el, self);
@@ -1588,7 +1604,10 @@ const InsertNodeRelative = union(enum) {
     after: *Node,
     before: *Node,
 };
-const InsertNodeOpts = struct { child_already_connected: bool = false };
+const InsertNodeOpts = struct {
+    child_already_connected: bool = false,
+    adopting_to_new_document: bool = false,
+};
 pub fn insertNodeRelative(self: *Page, parent: *Node, child: *Node, relative: InsertNodeRelative, opts: InsertNodeOpts) !void {
     return self._insertNodeRelative(false, parent, child, relative, opts);
 }
@@ -1666,22 +1685,21 @@ pub fn _insertNodeRelative(self: *Page, comptime from_parser: bool, parent: *Nod
 
     if (comptime from_parser) {
         if (child.is(Element)) |el| {
-            if (el.getAttributeSafe("id")) |id| {
-                try self.addElementId(parent, el, id);
-            }
-
             // Invoke connectedCallback for custom elements during parsing
             // For main document parsing, we know nodes are connected (fast path)
             // For fragment parsing (innerHTML), we need to check connectivity
-            if (self._parse_mode == .document or child.isConnected()) {
+            if (child.isConnected() or child.isInShadowTree()) {
+                if (el.getAttributeSafe("id")) |id| {
+                    try self.addElementId(parent, el, id);
+                }
                 try Element.Html.Custom.invokeConnectedCallbackOnElement(true, el, self);
             }
         }
         return;
     }
 
-    if (opts.child_already_connected) {
-        // The child is already connected, we don't have to reconnect it
+    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
         return;
     }
 
diff --git a/src/browser/tests/domparser.html b/src/browser/tests/domparser.html
index 66014388..7f65ebd9 100644
--- a/src/browser/tests/domparser.html
+++ b/src/browser/tests/domparser.html
@@ -1,5 +1,6 @@
 <!DOCTYPE html>
 <script src="testing.js"></script>
+<body></body>
 
 <script id=basic>
 {
@@ -118,3 +119,121 @@
   });
 }
 </script>
+
+<script id=getElementById>
+{
+  const doc = new DOMParser().parseFromString('<div id="new-node">new-node</div>', 'text/html');
+  testing.expectEqual('new-node', doc.getElementById('new-node').textContent);
+}
+</script>
+
+<script id=getElementById_isolationBetweenDocuments>
+{
+  const parser = new DOMParser();
+  const doc = parser.parseFromString('<div id="shared-id">From parsed doc</div>', 'text/html');
+
+  // Create element with same ID in main document
+  const mainEl = document.createElement('div');
+  mainEl.id = 'shared-id';
+  mainEl.textContent = 'From main doc';
+  document.body.appendChild(mainEl);
+
+  // Each document should find its own element
+  const mainFound = document.getElementById('shared-id');
+  testing.expectEqual('From main doc', mainFound.textContent);
+
+  const parsedFound = doc.getElementById('shared-id');
+  testing.expectEqual('From parsed doc', parsedFound.textContent);
+
+  // Clean up
+  mainEl.remove();
+}
+</script>
+
+<script id=getElementById_afterSettingId>
+{
+  const parser = new DOMParser();
+  const doc = parser.parseFromString('<div>No ID initially</div>', 'text/html');
+
+  const div = doc.querySelector('div');
+
+  // Should not find it yet
+  testing.expectEqual(null, doc.getElementById('new-id'));
+
+  // Set ID via JavaScript
+  div.id = 'new-id';
+
+  // Should now find it
+  const found = doc.getElementById('new-id');
+  testing.expectEqual(div, found);
+}
+</script>
+
+<script id=getElementById_afterRemovingId>
+{
+  const parser = new DOMParser();
+  const doc = parser.parseFromString('<div id="remove-me">Content</div>', 'text/html');
+
+  // Should find it initially
+  const div = doc.getElementById('remove-me');
+  testing.expectEqual('Content', div.textContent);
+
+  // Remove the ID
+  div.removeAttribute('id');
+
+  // Should not find it anymore
+  testing.expectEqual(null, doc.getElementById('remove-me'));
+}
+</script>
+
+<script id=getElementById_afterChangingId>
+{
+  const parser = new DOMParser();
+  const doc = parser.parseFromString('<div id="old-id">Content</div>', 'text/html');
+
+  const div = doc.querySelector('div');
+
+  // Change the ID
+  div.id = 'new-id';
+
+  // Should not find old ID
+  testing.expectEqual(null, doc.getElementById('old-id'));
+
+  // Should find new ID
+  const found = doc.getElementById('new-id');
+  testing.expectEqual(div, found);
+}
+</script>
+
+<script id=getElementById_detachedFromParsedDoc>
+{
+  const parser = new DOMParser();
+  const doc = parser.parseFromString('<div id="will-detach">Content</div>', 'text/html');
+
+  const div = doc.querySelector('div');
+
+  // Should find it while connected
+  testing.expectEqual(div, doc.getElementById('will-detach'));
+
+  // Remove it from the document
+  div.remove();
+
+  // Should not find it after removal
+  testing.expectEqual(null, doc.getElementById('will-detach'));
+}
+</script>
+
+<script id=getElementById_multipleDocuments>
+{
+  const parser = new DOMParser();
+  const doc1 = parser.parseFromString('<div id="doc1-el">Doc 1</div>', 'text/html');
+  const doc2 = parser.parseFromString('<div id="doc2-el">Doc 2</div>', 'text/html');
+
+  // Each document should only find its own element
+  testing.expectEqual('Doc 1', doc1.getElementById('doc1-el').textContent);
+  testing.expectEqual(null, doc1.getElementById('doc2-el'));
+
+  testing.expectEqual('Doc 2', doc2.getElementById('doc2-el').textContent);
+  testing.expectEqual(null, doc2.getElementById('doc1-el'));
+}
+</script>
diff --git a/src/browser/tests/element/html/template.html b/src/browser/tests/element/html/template.html
index 52db20fd..2a92534f 100644
--- a/src/browser/tests/element/html/template.html
+++ b/src/browser/tests/element/html/template.html
@@ -17,6 +17,13 @@
 
 <template id="empty"></template>
 
+<script id=ids>
+  testing.expectEqual(null, document.querySelector('#inner1'));
+  testing.expectEqual(null, document.getElementById('inner1'));
+  testing.expectEqual('First', document.getElementById('nested').content.getElementById('inner1').textContent);
+  testing.expectEqual('First', document.getElementById('nested').content.querySelector('#inner1').textContent);
+</script>
+
 <script id=content_property>
 {
   const template = $('#basic');
@@ -205,4 +212,4 @@
   // NOT the DocumentFragment, so it should be empty
   testing.expectEqual('', template.textContent);
 }
-</script>
+</script> -->
diff --git a/src/browser/tests/node/is_connected.html b/src/browser/tests/node/is_connected.html
new file mode 100644
index 00000000..35c53600
--- /dev/null
+++ b/src/browser/tests/node/is_connected.html
@@ -0,0 +1,108 @@
+<!DOCTYPE html>
+<script src="../testing.js"></script>
+<body>
+<div id="test-content">
+  <p id="p1">Connected paragraph</p>
+</div>
+
+<script id=isConnected_elementsInMainDocument>
+{
+  const p1 = document.getElementById('p1');
+  testing.expectEqual(true, p1.isConnected);
+
+  const body = document.body;
+  testing.expectEqual(true, body.isConnected);
+
+  const html = document.documentElement;
+  testing.expectEqual(true, html.isConnected);
+}
+</script>
+
+<script id=isConnected_detachedElement>
+{
+  const div = document.createElement('div');
+  testing.expectEqual(false, div.isConnected);
+
+  // Even if it has content, still not connected
+  div.textContent = 'Hello';
+  testing.expectEqual(false, div.isConnected);
+}
+</script>
+
+<script id=isConnected_afterAppending>
+{
+  const div = document.createElement('div');
+  testing.expectEqual(false, div.isConnected);
+
+  document.body.appendChild(div);
+  testing.expectEqual(true, div.isConnected);
+
+  // Remove it
+  div.remove();
+  testing.expectEqual(false, div.isConnected);
+}
+</script>
+
+<script id=isConnected_nestedDetachedElements>
+{
+  const parent = document.createElement('div');
+  const child = document.createElement('span');
+
+  parent.appendChild(child);
+
+  // Neither should be connected
+  testing.expectEqual(false, parent.isConnected);
+  testing.expectEqual(false, child.isConnected);
+
+  // Add parent to document
+  document.body.appendChild(parent);
+
+  // Now both should be connected
+  testing.expectEqual(true, parent.isConnected);
+  testing.expectEqual(true, child.isConnected);
+}
+</script>
+
+<script id=isConnected_parsedDocument>
+{
+  const parser = new DOMParser();
+  const doc = parser.parseFromString('<div id="parsed">Parsed content</div>', 'text/html');
+
+  // Use querySelector instead of getElementById for now
+  const div = doc.querySelector('div');
+  // CRITICAL: Elements in a parsed document should be connected to their own document
+  testing.expectEqual(true, div.isConnected);
+
+  const body = doc.body;
+  testing.expectEqual(true, body.isConnected);
+}
+</script>
+
+<script id=isConnected_documentNode>
+{
+  // The document itself should be connected
+  testing.expectEqual(true, document.isConnected);
+
+  // A parsed document should also be connected (to itself)
+  const parser = new DOMParser();
+  const doc = parser.parseFromString('<div>test</div>', 'text/html');
+  testing.expectEqual(true, doc.isConnected);
+}
+</script>
+
+<script id=isConnected_removedFromParsedDoc>
+{
+  const parser = new DOMParser();
+  const doc = parser.parseFromString('<div id="test">Content</div>', 'text/html');
+
+  // Use querySelector instead of getElementById for now
+  const div = doc.querySelector('div');
+  testing.expectEqual(true, div.isConnected);
+
+  // Remove it from the parsed document
+  div.remove();
+  testing.expectEqual(false, div.isConnected);
+}
+</script>
+
+</body>
diff --git a/src/browser/tests/node/node_iterator.html b/src/browser/tests/node/node_iterator.html
index 992df5ad..36472cf4 100644
--- a/src/browser/tests/node/node_iterator.html
+++ b/src/browser/tests/node/node_iterator.html
@@ -470,4 +470,3 @@
     testing.expectEqual(null, iterator.nextNode());
   }
 </script>
-
diff --git a/src/browser/tests/node/normalize.html b/src/browser/tests/node/normalize.html
index ead599a6..b85f8324 100644
--- a/src/browser/tests/node/normalize.html
+++ b/src/browser/tests/node/normalize.html
@@ -1,3 +1,4 @@
+<!DOCTYPE html>
 <script src="../testing.js"></script>
 <div id=container></div>
 
diff --git a/src/browser/tests/node/owner.html b/src/browser/tests/node/owner.html
new file mode 100644
index 00000000..1dd2dff2
--- /dev/null
+++ b/src/browser/tests/node/owner.html
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<script src="../testing.js"></script>
+
+<div id="target-container">
+  <p id="reference-node">
+  I am the original reference node.
+  </p>
+</div>
+
+
+<script id=owner>
+  const parser = new DOMParser();
+  const newDoc = parser.parseFromString('<div id="new-node"><p>Hey</p><span>Marked</span></div>', 'text/html');
+  const newNode = newDoc.getElementById('new-node');
+  const parent = $('#target-container');
+  const referenceNode = $('#reference-node');
+
+  parent.insertBefore(newNode, referenceNode);
+  const k = $('#new-node');
+  const ptag = k.querySelector('p');
+  const spanTag = k.querySelector('span');
+  const anotherDoc = parser.parseFromString('<div id="another-new-node"></div>', 'text/html');
+  const anotherNewNode = anotherDoc.getElementById('another-new-node');
+  testing.expectEqual('[object HTMLDivElement]', parent.appendChild(anotherNewNode).toString());
+
+
+  testing.expectEqual(newNode.ownerDocument, parent.ownerDocument);
+  testing.expectEqual(anotherNewNode.ownerDocument, parent.ownerDocument);
+  testing.expectEqual('P', newNode.firstChild.nodeName);
+  testing.expectEqual(parent.ownerDocument, ptag.ownerDocument);
+  testing.expectEqual(parent.ownerDocument, spanTag.ownerDocument);
+  testing.expectEqual(true, parent.contains(newNode));
+  testing.expectEqual(true, parent.contains(anotherNewNode));
+  testing.expectEqual(false, anotherDoc.contains(anotherNewNode));
+  testing.expectEqual(false, newDoc.contains(newNode));
+</script>
diff --git a/src/browser/webapi/Node.zig b/src/browser/webapi/Node.zig
index 564bf599..532a335c 100644
--- a/src/browser/webapi/Node.zig
+++ b/src/browser/webapi/Node.zig
@@ -202,6 +202,10 @@ pub fn appendChild(self: *Node, child: *Node, page: *Page) !*Node {
     // then we can remove + add a bit more efficiently (we don't have to fully
     // disconnect then reconnect)
     const child_connected = child.isConnected();
+    // Check if we're adopting the node to a different document
+    const child_root = child.getRootNode(null);
+    const parent_root = self.getRootNode(null);
+    const adopting_to_new_document = child_connected and child_root != parent_root;
 
     if (child._parent) |parent| {
         // we can signal removeNode that the child will remain connected
@@ -209,7 +213,10 @@ pub fn appendChild(self: *Node, child: *Node, page: *Page) !*Node {
         page.removeNode(parent, child, .{ .will_be_reconnected = self.isConnected() });
     }
 
-    try page.appendNode(self, child, .{ .child_already_connected = child_connected });
+    try page.appendNode(self, child, .{
+        .child_already_connected = child_connected,
+        .adopting_to_new_document = adopting_to_new_document,
+    });
     return child;
 }
 
@@ -319,19 +326,14 @@ pub fn isInShadowTree(self: *Node) bool {
 }
 
 pub fn isConnected(self: *const Node) bool {
-    const target = Page.current.document.asNode();
-    if (self == target) {
-        return true;
+    // Walk up to find the root node
+    var root = self;
+    while (root._parent) |parent| {
+        root = parent;
     }
 
-    var node = self._parent;
-    while (node) |n| {
-        if (n == target) {
-            return true;
-        }
-        node = n._parent;
-    }
-    return false;
+    // A node is connected if its root is a document
+    return root._type == .document;
 }
 
 const GetRootNodeOpts = struct {
@@ -432,6 +434,10 @@ pub fn insertBefore(self: *Node, new_node: *Node, ref_node_: ?*Node, page: *Page
     }
 
     const child_already_connected = new_node.isConnected();
+    // Check if we're adopting the node to a different document
+    const child_root = new_node.getRootNode(null);
+    const parent_root = self.getRootNode(null);
+    const adopting_to_new_document = child_already_connected and child_root != parent_root;
 
     page.domChanged();
     const will_be_reconnected = self.isConnected();
@@ -443,7 +449,10 @@ pub fn insertBefore(self: *Node, new_node: *Node, ref_node_: ?*Node, page: *Page
         self,
         new_node,
         .{ .before = ref_node },
-        .{ .child_already_connected = child_already_connected },
+        .{
+            .child_already_connected = child_already_connected,
+            .adopting_to_new_document = adopting_to_new_document,
+        },
     );
 
     return new_node;