Skip to content

Adopt full subtrees during cross-document insertion - #682

Open
olavoasantos wants to merge 1 commit into
correct-child-replace-withfrom
adopt-inserted-subtrees
Open

olavoasantos wants to merge 1 commit into
correct-child-replace-withfrom
adopt-inserted-subtrees

Conversation

@olavoasantos

@olavoasantos olavoasantos commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Problem

Cross-document insertion updated ownerDocument only on the inserted root. Descendants and attached attributes retained the source document, and initialized HTMLTemplateElement.content was outside the ordinary child tree, so later mutations could resolve hooks through the wrong Window.

Impact

Important. After inserting a subtree into another document, a descendant attribute, text, or child mutation could be dispatched through the source window rather than the destination. The local tree would appear inserted while its Remote DOM effects were sent to the wrong receiver.

Reproduction

const source = new Window().document;
const destination = new Window().document;
const root = source.createElement('section');
const child = source.createElement('span');

child.setAttribute('data-state', 'before');
root.appendChild(child);
destination.body.appendChild(root);

child.setAttribute('data-state', 'after');

Before this change, root.ownerDocument changed but child and its attached attribute still belonged to source, so the final mutation could use the source window’s hook set. It now adopts the root, descendants, and attached attributes into destination before later mutations run.

Change

Build an iterative adoption snapshot before insertion commits, covering the subtree, attached attributes, and already initialized template-content subtrees. Apply that snapshot to the destination document during insertion, and share the same adoption routine with Document.adoptNode(). The snapshot deduplicates malformed template host/content cycles and preflights traversal so an error leaves links, ownership, connectivity, and hooks unchanged.

Tests

Regression coverage uses two windows to verify nested elements, text, and attached attributes receive the destination owner document and route later attribute, text, and insertion effects only through destination hooks. It also covers initialized and untouched template content, Document.adoptNode(), document-fragment roots, malformed template cycles, and failed preflight preservation. The 6,000-deep initialized-template adoption case is iterative and has an explicit 20-second test budget.

Stack

The 6,000-deep adoption regression test has an explicit 20-second CI budget; the full coverage run passes.

Validation

Fresh GitHub CI on restacked head 9788a1c passes:

  • lint;
  • type-check and build;
  • unit tests with coverage;
  • bundle-size checks;
  • Playwright end-to-end tests;
  • the classified Web Platform Test suite.

@olavoasantos
olavoasantos force-pushed the adopt-inserted-subtrees branch from 978a2ed to 61bd1af Compare September 3, 2026 15:29
@olavoasantos
olavoasantos force-pushed the adopt-inserted-subtrees branch 2 times, most recently from ff2de3a to 9788a1c Compare September 4, 2026 14:47
@olavoasantos
olavoasantos marked this pull request as ready for review September 14, 2026 18:41
@henrytao-me

Copy link
Copy Markdown
Member

The full-subtree insertion fix looks right at 9788a1c against #681 6676e1d, but Document.adoptNode() needs an effects-ordering correction before landing.

Document.ts:124-132 collects the adoption snapshot, calls public removeChild(), then applies the snapshot. That removal runs hooks and custom-element reactions before returning, so user code can invalidate the snapshot's membership.

Minimal polyfill reproduction:

const source = new Window();
const destination = new Window();

source.customElements.define(
  'x-moving',
  class extends source.HTMLElement {
    disconnectedCallback() {
      this.appendChild(source.document.createElement('span'));
    }
  },
);

const root = source.document.createElement('x-moving');
source.document.body.appendChild(root);
destination.document.adoptNode(root);

root.ownerDocument === destination.document; // true
root.lastChild.ownerDocument === destination.document; // false on head

The equivalent native case and the actual base finish with both nodes owned by destination. Head leaves the newly appended child source-owned underneath a destination-owned root because the child was absent from the snapshot.

The inverse also corrupts ownership: if disconnectedCallback() moves an existing child into a third document, native/base leave it owned by that third document, but head's saved snapshot overwrites its owner with destination even though its parent remains in the third document.

Both new failures also reproduce from the source Window's removeChild hook, without custom elements. Later attribute/text mutations actually route through the wrong Window's hook set. The old callback-visible source ownership was already incorrect on base; these final ownership corruptions are introduced by applying the frozen snapshot afterward.

Please retain the iterative snapshot and preflight, but commit detachment and adoption before either removal hooks or lifecycle callbacks run, matching the insertion path's committed-state-before-effects ordering. Deferring only custom-element reactions misses hook reentrancy; resnapshotting after callbacks retains the wrong callback-visible context. The DOM operation is [CEReactions] and completes its adoption steps before the callback boundary.

Focused regression coverage:

  • Append a source-created child during removal, through both disconnectedCallback and removeChild hooks.
  • Transfer an existing child into a third document through both paths.
  • Assert exact final parent/owner consistency and later attribute/text hook routing, not just root ownership.
  • Preserve explicit adoptNode() preflight failures: original error, unchanged parent/owners/connectivity, and no hooks. The current head passes the template-content traversal-error control.

Validation: 247 polyfill tests and 59 focused core tests pass; eight of nine added tests fail the actual base. The locked-TypeScript polyfill check, formatting, and reported CI pass. Isolated Chrome/native comparisons match all 40 ordinary operation/connectivity combinations (49/58 overall). Eight targeted hook scenarios expose four new explicit-adoption routing failures while confirming two implicit-insertion routing fixes. Existing inert-template-document and other adoptNode contract gaps are separate, not requests to expand this correction. No production/candidate fix, complete locked-repository/full-build validation, real multi-receiver integration, live Shell/admin validation, or approval performed.

@olavoasantos
olavoasantos force-pushed the adopt-inserted-subtrees branch from 9788a1c to 0dffbfb Compare September 16, 2026 13:56
@olavoasantos
olavoasantos force-pushed the adopt-inserted-subtrees branch from 0dffbfb to cf11b70 Compare September 16, 2026 14:08
@olavoasantos

Copy link
Copy Markdown
Contributor Author

Agreed. adoptNode() now preflights the snapshot, detaches the root, and commits ownership before removal hooks or disconnected reactions run. Reentrant appends are adopted by the destination operation, and transfers into a third document are no longer overwritten by a stale snapshot. The tests cover both hook and callback paths plus later hook routing.

@olavoasantos
olavoasantos force-pushed the adopt-inserted-subtrees branch from cf11b70 to 08b54c9 Compare September 16, 2026 20:13
@olavoasantos
olavoasantos removed this pull request from stack #699 September 16, 2026 20:14
@olavoasantos
olavoasantos added this pull request to stack #716 September 16, 2026 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants