From 9d271ce4ae200d79beb2b883ece68ada44bbad82 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Wed, 29 Apr 2026 09:09:07 +0800 Subject: [PATCH] Fix more DOM assumptions that callback and document.write can break As we've seen before, both document.write and custom elements callbacks can introduce otherwise impossible states. See https://github.com/lightpanda-io/browser/pull/2172 for just one example. The issue with both is that they can trigger code during DOM manipulation that renders the state of that DOM manipulation invalid. This commit started with a fairly easy case to understand (one that actually happened in production). It goes something like: ```
');
``` Here we see that the written script deletes the node which is in the process of being rendered. These issues are almost always easy to fix, but they're hard to predict. After fixing this issue, I asked Claude to check for other cases, and it was able to find / reproduce / fix two more involving custom elements callback. --- src/browser/Frame.zig | 24 ++++-- .../mutation_during_callback.html | 86 +++++++++++++++++++ src/browser/tests/document/write.html | 22 +++++ src/browser/webapi/Document.zig | 9 +- src/browser/webapi/Element.zig | 14 ++- 5 files changed, 144 insertions(+), 11 deletions(-) create mode 100644 src/browser/tests/custom_elements/mutation_during_callback.html diff --git a/src/browser/Frame.zig b/src/browser/Frame.zig index 12bb8c38..1369cde4 100644 --- a/src/browser/Frame.zig +++ b/src/browser/Frame.zig @@ -2885,12 +2885,24 @@ pub fn insertAllChildrenBefore(self: *Frame, fragment: *Node, parent: *Node, ref // Check if child was connected BEFORE removing it from fragment const child_was_connected = child.isConnected(); self.removeNode(fragment, child, .{ .will_be_reconnected = dest_connected }); - try self.insertNodeRelative( - parent, - child, - .{ .before = ref_node }, - .{ .child_already_connected = child_was_connected }, - ); + // A callback fired by a previous iteration's insert (e.g. a custom + // element's connectedCallback) may have detached ref_node from + // parent. In that case, fall back to append so the remaining + // children still land in `parent` in source order. + if (ref_node._parent == parent) { + try self.insertNodeRelative( + parent, + child, + .{ .before = ref_node }, + .{ .child_already_connected = child_was_connected }, + ); + } else { + try self.appendNode( + parent, + child, + .{ .child_already_connected = child_was_connected }, + ); + } } } diff --git a/src/browser/tests/custom_elements/mutation_during_callback.html b/src/browser/tests/custom_elements/mutation_during_callback.html new file mode 100644 index 00000000..bca44926 --- /dev/null +++ b/src/browser/tests/custom_elements/mutation_during_callback.html @@ -0,0 +1,86 @@ + + + + + + + + + + + diff --git a/src/browser/tests/document/write.html b/src/browser/tests/document/write.html index 6cb5bbb6..7322882f 100644 --- a/src/browser/tests/document/write.html +++ b/src/browser/tests/document/write.html @@ -120,6 +120,28 @@ } + +
+ +
+ + + diff --git a/src/browser/webapi/Document.zig b/src/browser/webapi/Document.zig index a30d32cd..8ffec5ce 100644 --- a/src/browser/webapi/Document.zig +++ b/src/browser/webapi/Document.zig @@ -713,13 +713,20 @@ fn writeInternal(self: *Document, text: []const []const u8, append_newline: bool // Determine insertion point: // - If _write_insertion_point is set and still parented correctly, continue from there // - Otherwise, start after the script (first write, or previous insertion point was removed) + // parseFragment above can synchronously execute a parser-blocking script + // (e.g.