From c0ae2ea137856c6da5079fcf4cd8de95464527e4 Mon Sep 17 00:00:00 2001
From: Scott Taylor <scott.c.taylor@mac.com>
Date: Mon, 14 Sep 2026 22:30:25 -0400
Subject: [PATCH] dom: upgrade autonomous custom-element clones in place

Cloning reused the synchronous createElement construction path, which
rejects a result with a parent, attributes, or children. A reparenting
constructor left its instance in the source tree while the clone received
an HTMLUnknownElement fallback with different identity.

Queue an upgrade reaction for autonomous clones instead. Their copied
attributes and descendants are present when construction runs, and super()
returns the copied node. A failed upgrade retains that node rather than
substituting a second element. Keep synchronous createElement validation.

Capture initial upgrade reactions before construction and distinguish the
precustomized state so constructor-time DOM mutations do not enqueue
custom-element lifecycle reactions prematurely.

Add Chromium-checked regressions for identity, reparenting, copied state,
attribute reaction order, importNode and failed upgrades.
---
 src/browser/CustomElementReactions.zig        |   6 +
 src/browser/frame/node_factory.zig            |  10 ++
 .../tests/custom_elements/clone_upgrade.html  | 164 ++++++++++++++++++
 src/browser/webapi/CustomElementRegistry.zig  |  30 ++--
 src/browser/webapi/element/html/Custom.zig    |  14 +-
 5 files changed, 206 insertions(+), 18 deletions(-)
 create mode 100644 src/browser/tests/custom_elements/clone_upgrade.html

diff --git a/src/browser/CustomElementReactions.zig b/src/browser/CustomElementReactions.zig
index 60878a47d..e0b5bdaeb 100644
--- a/src/browser/CustomElementReactions.zig
+++ b/src/browser/CustomElementReactions.zig
@@ -41,6 +41,7 @@ const Frame = @import("Frame.zig");
 const Element = @import("webapi/Element.zig");
 const Document = @import("webapi/Document.zig");
 const Custom = @import("webapi/element/html/Custom.zig");
+const CustomElementDefinition = @import("webapi/CustomElementDefinition.zig");
 
 const String = lp.String;
 const Allocator = std.mem.Allocator;
@@ -105,6 +106,10 @@ fn route(self: *Self, frame: *Frame, reaction: Reaction) !void {
     }
 }
 
+pub fn enqueueUpgrade(self: *Self, frame: *Frame, element: *Custom, definition: *CustomElementDefinition) !void {
+    try self.route(frame, .{ .upgrade = .{ .element = element, .definition = definition } });
+}
+
 pub fn enqueueConnected(self: *Self, frame: *Frame, element: *Element) !void {
     try self.route(frame, .{ .connected = element });
 }
@@ -144,6 +149,7 @@ pub fn enqueueAttributeChanged(
 }
 
 pub const Reaction = union(enum) {
+    upgrade: struct { element: *Custom, definition: *CustomElementDefinition },
     connected: *Element,
     disconnected: *Element,
     move: *Element,
diff --git a/src/browser/frame/node_factory.zig b/src/browser/frame/node_factory.zig
index 8ebc709bd..44b1ced41 100644
--- a/src/browser/frame/node_factory.zig
+++ b/src/browser/frame/node_factory.zig
@@ -39,6 +39,7 @@ const IFrame = Element.Html.IFrame;
 
 pub fn createElementNS(document: *const Node.Document, namespace: Element.Namespace, name: []const u8, attribute_iterator: anytype) !*Node {
     const from_parser = @TypeOf(attribute_iterator) == Parser.AttributeIterator;
+    const from_clone = @TypeOf(attribute_iterator) == *Element.Attribute.List or @TypeOf(attribute_iterator) == *const Element.Attribute.List;
     const frame = frameOf(document);
 
     switch (namespace) {
@@ -855,6 +856,15 @@ pub fn createElementNS(document: *const Node.Document, namespace: Element.Namesp
                     return node;
                 }
 
+                if (from_clone) {
+                    const node = try createHtmlElementT(document, Element.Html.Custom, namespace, attribute_iterator, .{
+                        ._tag_name = tag_name,
+                        ._definition = null,
+                    });
+                    try realm._ce_reactions.enqueueUpgrade(realm, node.as(Element).is(Element.Html.Custom).?, definition.?);
+                    return node;
+                }
+
                 // https://dom.spec.whatwg.org/#concept-create-element, the
                 // synchronous branch. super() has to create its own element
                 const constructed = constructForToken(realm, definition.?, tag_name, from_parser) catch {
diff --git a/src/browser/tests/custom_elements/clone_upgrade.html b/src/browser/tests/custom_elements/clone_upgrade.html
new file mode 100644
index 000000000..e1a789168
--- /dev/null
+++ b/src/browser/tests/custom_elements/clone_upgrade.html
@@ -0,0 +1,164 @@
+<!DOCTYPE html>
+<script src="../testing.js"></script>
+<body>
+
+<script id=clone_constructor_sees_copied_state>
+    {
+        let cloning = false;
+        let constructed;
+        let observed;
+        class CloneStateElement extends HTMLElement {
+            constructor() {
+                super();
+                if (cloning) {
+                    constructed = this;
+                    observed = [this.getAttribute('title'), this.children.length, this.textContent];
+                    this.setAttribute('from-constructor', 'yes');
+                    this.appendChild(document.createElement('span'));
+                }
+            }
+        }
+        customElements.define('clone-state-element', CloneStateElement);
+        const original = new CloneStateElement();
+        original.title = 'copied';
+        original.appendChild(document.createElement('b')).textContent = 'child';
+        cloning = true;
+        const copy = original.cloneNode(true);
+        testing.expectEqual(true, copy === constructed);
+        testing.expectEqual(true, copy instanceof CloneStateElement);
+        testing.expectEqual(['copied', 1, 'child'], observed);
+        testing.expectEqual('yes', copy.getAttribute('from-constructor'));
+        testing.expectEqual(2, copy.children.length);
+        testing.expectEqual(1, original.children.length);
+    }
+</script>
+
+<script id=reparented_clone_preserves_identity>
+    {
+        const source = document.body.appendChild(document.createElement('div'));
+        const constructed = [];
+        let cloning = false;
+        let connections = 0;
+        class ReparentedCloneElement extends HTMLElement {
+            constructor() {
+                super();
+                if (cloning) {
+                    constructed.push(this);
+                    if (constructed.length <= 8) source.appendChild(this);
+                }
+            }
+            connectedCallback() {
+                if (cloning) connections++;
+            }
+        }
+        customElements.define('reparented-clone-element', ReparentedCloneElement);
+        source.appendChild(new ReparentedCloneElement()).title = 'first';
+        source.appendChild(new ReparentedCloneElement()).title = 'second';
+        cloning = true;
+        const copy = source.cloneNode(true);
+        cloning = false;
+        testing.expectEqual(2, constructed.length);
+        testing.expectEqual(0, copy.children.length);
+        testing.expectEqual(4, source.children.length);
+        testing.expectEqual(true, source.children[2] === constructed[0]);
+        testing.expectEqual(true, source.children[3] === constructed[1]);
+        testing.expectEqual('first', constructed[0].title);
+        testing.expectEqual('second', constructed[1].title);
+        testing.expectEqual(0, connections);
+    }
+</script>
+
+<script id=clone_reactions_use_preconstruction_attributes>
+    {
+        let cloning = false;
+        const events = [];
+        class CloneReactionElement extends HTMLElement {
+            static get observedAttributes() { return ['title']; }
+            constructor() {
+                super();
+                if (cloning) {
+                    events.push(['constructor', this.title]);
+                    this.title = 'changed';
+                }
+            }
+            attributeChangedCallback(name, oldValue, newValue) {
+                events.push([name, oldValue, newValue]);
+            }
+        }
+        customElements.define('clone-reaction-element', CloneReactionElement);
+        const original = new CloneReactionElement();
+        original.title = 'copied';
+        events.length = 0;
+        cloning = true;
+        const copy = original.cloneNode();
+        cloning = false;
+        testing.expectEqual([['constructor', 'copied'], ['title', null, 'copied']], events);
+        testing.expectEqual('changed', copy.title);
+        testing.expectEqual('copied', original.title);
+    }
+</script>
+
+<script id=import_upgrades_the_copied_element>
+    {
+        let constructed;
+        let observed;
+        class ImportedCloneElement extends HTMLElement {
+            constructor() {
+                super();
+                constructed = this;
+                observed = [this.title, this.textContent, this.ownerDocument === document];
+            }
+        }
+        customElements.define('imported-clone-element', ImportedCloneElement);
+        const foreign = document.implementation.createHTMLDocument('foreign');
+        const original = foreign.createElement('imported-clone-element');
+        original.title = 'copied';
+        original.appendChild(foreign.createElement('b')).textContent = 'child';
+        const copy = document.importNode(original, true);
+        testing.expectEqual(true, copy === constructed);
+        testing.expectEqual(true, copy instanceof ImportedCloneElement);
+        testing.expectEqual(['copied', 'child', true], observed);
+        testing.expectEqual(true, original.ownerDocument === foreign);
+        testing.expectEqual(1, original.children.length);
+    }
+</script>
+
+<script id=failed_clone_upgrade_keeps_the_copied_node>
+    {
+        let cloning = false;
+        let constructed;
+        let attempts = 0;
+        let reported = 0;
+        const onError = (event) => {
+            if (event.message.includes('clone upgrade failed')) {
+                reported++;
+                event.preventDefault();
+            }
+        };
+        window.addEventListener('error', onError);
+        class FailedCloneElement extends HTMLElement {
+            constructor() {
+                super();
+                if (cloning) {
+                    constructed = this;
+                    attempts++;
+                    this.setAttribute('from-constructor', 'yes');
+                    throw new Error('clone upgrade failed');
+                }
+            }
+        }
+        customElements.define('failed-clone-element', FailedCloneElement);
+        const original = new FailedCloneElement();
+        original.title = 'copied';
+        cloning = true;
+        const copy = original.cloneNode();
+        customElements.upgrade(copy);
+        window.removeEventListener('error', onError);
+        testing.expectEqual(true, copy === constructed);
+        testing.expectEqual(true, copy instanceof FailedCloneElement);
+        testing.expectEqual('copied', copy.title);
+        testing.expectEqual('yes', copy.getAttribute('from-constructor'));
+        testing.expectEqual(1, attempts);
+        testing.expectEqual(1, reported);
+    }
+</script>
diff --git a/src/browser/webapi/CustomElementRegistry.zig b/src/browser/webapi/CustomElementRegistry.zig
index 4afcd6b7f..628afb42b 100644
--- a/src/browser/webapi/CustomElementRegistry.zig
+++ b/src/browser/webapi/CustomElementRegistry.zig
@@ -208,6 +208,21 @@ pub fn upgradeCustomElement(custom: *Custom, definition: *CustomElementDefinitio
     custom._disconnected_callback_invoked = false;
 
     const node = custom.asNode();
+    const element = custom.asElement();
+    for (element.attributeEntries()) |*attr| {
+        const name = lp.String.wrap(attr.name());
+        if (definition.isAttributeObserved(name)) {
+            Custom.enqueueAttributeChangedCallbackOnElement(element, name, null, .wrap(attr.value()), null, frame);
+        }
+    }
+    if (node.isConnected()) {
+        try Custom.enqueueConnectedCallbackOnElement(false, element, frame);
+    }
+
+    // During construction the element is precustomized, not yet custom.
+    custom._upgrade_in_progress = true;
+    defer custom._upgrade_in_progress = false;
+
     const prev_upgrading = frame._upgrading_element;
     const prev_consumed = frame._upgrading_consumed;
     frame._upgrading_element = node;
@@ -251,21 +266,6 @@ pub fn upgradeCustomElement(custom: *Custom, definition: *CustomElementDefinitio
         frame.window.reportError(exc, frame) catch {};
         return error.CustomElementUpgradeFailed;
     }
-
-    // Enqueue attributeChangedCallback for existing observed attributes
-    const element = custom.asElement();
-    for (element.attributeEntries()) |*attr| {
-        const name = lp.String.wrap(attr.name());
-        if (definition.isAttributeObserved(name)) {
-            Custom.enqueueAttributeChangedCallbackOnElement(element, name, null, .wrap(attr.value()), null, frame);
-        }
-    }
-
-    if (node.isConnected()) {
-        Custom.enqueueConnectedCallbackOnElement(false, element, frame) catch |err| {
-            log.warn(.bug, "ce_reactions enqueue fail", .{ .err = err });
-        };
-    }
 }
 
 fn upgradeFailed(custom: *Custom) void {
diff --git a/src/browser/webapi/element/html/Custom.zig b/src/browser/webapi/element/html/Custom.zig
index 58fcd4737..4276651ad 100644
--- a/src/browser/webapi/element/html/Custom.zig
+++ b/src/browser/webapi/element/html/Custom.zig
@@ -44,6 +44,7 @@ _definition: ?*CustomElementDefinition,
 _connected_callback_invoked: bool = false,
 _disconnected_callback_invoked: bool = false,
 _upgrade_failed: bool = false, // a failed upgrade is never retried
+_upgrade_in_progress: bool = false,
 
 pub fn asElement(self: *Custom) *Element {
     return Factory.protoOf(self).asElement();
@@ -62,6 +63,7 @@ pub fn asNode(self: *Custom) *Node {
 pub fn enqueueConnectedCallbackOnElement(comptime from_parser: bool, element: *Element, frame: *Frame) error{OutOfMemory}!void {
     // Autonomous custom element
     if (element.is(Custom)) |custom| {
+        if (custom._upgrade_in_progress) return;
         // Upgrade if a definition exists but isn't yet attached
         if (custom._definition == null) {
             if (custom._upgrade_failed) {
@@ -124,7 +126,7 @@ pub fn enqueueConnectedCallbackOnElement(comptime from_parser: bool, element: *E
 
 pub fn enqueueDisconnectedCallbackOnElement(element: *Element, frame: *Frame) void {
     if (element.is(Custom)) |custom| {
-        if (custom._definition == null) return;
+        if (custom._definition == null or custom._upgrade_in_progress) return;
         if (custom._disconnected_callback_invoked) return;
         custom._disconnected_callback_invoked = true;
         custom._connected_callback_invoked = false;
@@ -157,7 +159,7 @@ pub fn enqueueDisconnectedCallbackOnElement(element: *Element, frame: *Frame) vo
 // moves with it.
 pub fn enqueueMoveCallbackOnElement(element: *Element, frame: *Frame) void {
     const eligible = if (element.is(Custom)) |custom|
-        custom._definition != null
+        custom._definition != null and !custom._upgrade_in_progress
     else
         frame.getCustomizedBuiltInDefinition(element) != null;
 
@@ -191,7 +193,7 @@ pub fn enqueueShadowTreeCallbacks(host: *Element, comptime reaction: enum { conn
 
 pub fn enqueueAdoptedCallbackOnElement(element: *Element, old_document: *Document, new_document: *Document, frame: *Frame) void {
     if (element.is(Custom)) |custom| {
-        if (custom._definition == null) return;
+        if (custom._definition == null or custom._upgrade_in_progress) return;
     } else {
         if (frame.getCustomizedBuiltInDefinition(element) == null) return;
     }
@@ -202,6 +204,7 @@ pub fn enqueueAdoptedCallbackOnElement(element: *Element, old_document: *Documen
 
 pub fn enqueueAttributeChangedCallbackOnElement(element: *Element, name: String, old_value: ?String, new_value: ?String, namespace: ?String, frame: *Frame) void {
     if (element.is(Custom)) |custom| {
+        if (custom._upgrade_in_progress) return;
         const definition = custom._definition orelse return;
         if (!definition.isAttributeObserved(name)) return;
     } else {
@@ -217,6 +220,11 @@ pub fn enqueueAttributeChangedCallbackOnElement(element: *Element, name: String,
 // Filtering already happened at enqueue time, so just fire unconditionally.
 pub fn fireReaction(reaction: Reaction, frame: *Frame) void {
     switch (reaction) {
+        .upgrade => |u| {
+            if (u.element._definition != null or u.element._upgrade_failed) return;
+            const CustomElementRegistry = @import("../../CustomElementRegistry.zig");
+            CustomElementRegistry.upgradeCustomElement(u.element, u.definition, frame) catch {};
+        },
         .connected => |el| {
             if (el.is(Custom)) |custom| {
                 custom.invokeCallback("connectedCallback", .{}, frame);