fix(dom): resolve <slot> projection when flattening shadow DOM (PER-10812)

[ninadbstack]

Problem

forceShadowAsLightDOM: true flattens shadow roots into light DOM, but never resolved <slot> projection.

Slotted content lives in the host's light DOM and is only rendered at the slot's position by the shadow root. Dropping the root left an inert <slot> in the output and appended the slotted content after the flattened shadow content — so it rendered outside its container.

Reproduced live on a Salesforce LWC page (PER-10812): the Yes/No tiles rendered below the card instead of inside it.

before                                   after
<c-card>                                 <c-card>
  <div class="card">                       <div class="card">
    <div class="content"><slot></slot>       <div class="content">
  <div class="tiles">Yes / No</div>            <div class="tiles">Yes / No</div>

Fix

• A <slot> belonging to a shadow root emits its assignedNodes({ flatten: true }) at that position and is not cloned. Reading the browser's own assignment handles named slots, ordering, fallback content and forwarded slots without reimplementing the matching rules.
• The light-DOM walk is skipped for a flattened host, so projected nodes aren't emitted twice.

Behaviour change

Content assigned to no slot is now dropped, matching what the browser renders for it: nothing. Previously it rendered, misplaced, after the container.

Such elements have no clone, so serialize-canvas/inputs/video/frames now skip an element whose clone lookup comes back empty rather than failing to resolve it. Without this, an unprojected <canvas>/<input>/<video> under a flattened host throws and fails the whole snapshot.

Tests

New packages/dom/test/slot-projection.test.js (13 specs) on a fixture mirroring the real LWC card — asserting position, not just presence:

• default and named slot routing; no cross-slot leakage
• inert <slot> elements dropped
• each slotted node emitted exactly once
• content for an unexposed slot dropped; document order preserved; fallback content used
• slots forwarded through nested components, including behind a wrapper
• bare text nodes and <noscript>
• no throw for unprojected media under a flattened host
• non-flattened hosts and disableShadowDOM unchanged

serialize-dom.test.js had a spec asserting <slot name="title"></slot> survives, which pinned the bug in place; it now asserts the projection.

Suite: 527 passing, verified against the live portal page (slots 2 → 0, content nested at the slot position, replay pixel-identical to the live render).

🤖 Generated with Claude Code

[coderabbitai]

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Central YAML (base), Workspace UI (inherited)

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: cc10e9d5-0f73-49eb-b1cf-c60d933c4748

📥 Commits

Reviewing files that changed from the base of the PR and between db20253 and 0f0913a.

📒 Files selected for processing (7)

• packages/dom/src/clone-dom.js
• packages/dom/src/serialize-canvas.js
• packages/dom/src/serialize-frames.js
• packages/dom/src/serialize-inputs.js
• packages/dom/src/serialize-video.js
• packages/dom/test/serialize-dom.test.js
• packages/dom/test/slot-projection.test.js

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

_{Comment @coderabbitai help to get the list of available commands.}

[ninadbstack]

Claude Code Review (automated) — 2 inline finding(s). Full report in the PR comment below. Verdict: Passed.

[ninadbstack]

[Medium] Optional chaining is a partial fix — the accessible-frame branch above still assumes a non-null cloneEl

When a flattened host projects no <iframe>, cloneEl (line 75) is null, but the loop doesn't bail out. It enters the frame.contentDocument && frame.contentDocument.documentElement branch (a bare iframe's contentDocument points at about:blank), runs a full recursive serializeDOM(), adds that frame's resources to the resource set, then throws a TypeError on the unguarded cloneEl.setAttribute('srcdoc', …) — silently swallowed by the adjacent empty catch { }. Sandboxed-iframe warnings are emitted for content that never renders too. Output HTML is unaffected, so this is wasted recursion, spurious resources and a masked exception rather than a visual regression.

Suggestion: Bail out once right after cloneEl is computed on line 75, matching the pattern this PR already applies in serialize-canvas.js, serialize-inputs.js and serialize-video.js:

let cloneEl = clone.querySelector(`[data-percy-element-id="${percyElementId}"]`);
// no counterpart in the clone means the iframe wasn't rendered — e.g. light
// DOM content no <slot> projected, which the browser doesn't render
if (!cloneEl) continue;

Both ?. guards then become unnecessary. Worth also strengthening the 'does not throw for unprojected media elements in a flattened host' test to assert the recursive serialization does not run, rather than only that nothing throws — as written it tolerates this defect.

Reviewer: stack-code-reviewer

[ninadbstack]

[Low] Unquoted attribute-selector template, inconsistent with the sibling serializers

serialize-inputs.js, serialize-video.js, serialize-frames.js, serialize-cssom.js and serialize-dom.js all quote the attribute value. This line matches the pre-existing unquoted selector in createAndInsertImageElement in the same file, so it's locally consistent — but it copies a latent fragility: an unquoted CSS attribute value must be a valid CSS identifier. Today uid() returns _${random} and is always valid, so it works; if the ID format ever changes, querySelector throws a SyntaxError instead of returning null, turning a correct "not projected, skip" into a reported canvas-serialization error.

Suggestion:

Suggested change

	if (!clone.querySelector(`[data-percy-element-id=${percyElementId}]`)) continue;
	if (!clone.querySelector(`[data-percy-element-id="${percyElementId}"]`)) continue;

Reviewer: stack-code-reviewer

[ninadbstack]

Claude Code PR Review

PR: #2437 • Head: 0f0913a • Reviewers: stack-code-reviewer

Summary

Resolves <slot> projection when forceShadowAsLightDOM flattens shadow roots into light DOM (PER-10812). A <slot> reached during the clone walk is now replaced in place by its assignedNodes({ flatten: true }), the inert <slot> itself is dropped, and the host's light-DOM walk is skipped for a flattened host so projected nodes aren't emitted a second time outside their container. Light children assigned to no slot are dropped, matching what the browser renders for them. Three downstream serializers (canvas, inputs, video) gain a "no counterpart in the clone → skip" guard, and serialize-frames gains optional chaining, since unprojected elements no longer have clones. Adds a 378-line slot-projection.test.js and updates one assertion in serialize-dom.test.js.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	No credentials, tokens or URLs introduced.
High	Security	Authentication/authorization checks present	N/A	Browser-side DOM serialization; no auth surface.
High	Security	Input validation and sanitization	Pass	srcdoc still routed through the existing Trusted Types policy (p.createHTML); no new sink.
High	Security	No IDOR — resource ownership validated	N/A	No resource ownership in this layer.
High	Security	No SQL injection (parameterized queries)	N/A	No database access.
High	Correctness	Logic is correct, handles edge cases	Pass	Projection model matches spec: slot → assignedNodes({flatten:true}), inert slot dropped, unslotted light children dropped (the browser renders none). getRootNode()?.host guard verified correct for closed roots — ShadowRoot.host is unaffected by mode: 'closed'; only Element.shadowRoot is gated.
High	Correctness	Error handling is explicit, no swallowed exceptions	Fail	serialize-frames.js:150 — see Finding 1. A null cloneEl reaches cloneEl.setAttribute('srcdoc', …) and the resulting TypeError is swallowed by the adjacent empty catch { }; only the next line is chain-guarded. Reachable, but the impact is wasted work and a masked exception, not wrong output.
High	Correctness	No race conditions or concurrency issues	Pass	Synchronous single-pass tree walk; no shared mutable state introduced.
Medium	Testing	New code has corresponding tests	Pass	New slot-projection.test.js covers default/named slots, dedup, fallback content, orphan slot names, nested/forwarded slots, closed shadow roots, and a slotted shadow host.
Medium	Testing	Error paths and edge cases tested	Fail	The unprojected-media test asserts only not.toThrow(), which is exactly how the swallowed TypeError in Finding 1 presents — the test tolerates the defect rather than catching it.
Medium	Testing	Existing tests still pass (no regressions)	Pass	All Test @percy/* checks green on 0f0913a, including @percy/dom. The one updated assertion in serialize-dom.test.js correctly reflects the intended new behavior.
Medium	Performance	No N+1 queries or unbounded data fetching	Fail	Finding 1: for an unprojected iframe, a full recursive serializeDOM() runs and its resources are added to the resource set before the discarded write — work and uploaded assets for content that is never rendered.
Medium	Performance	Long-running tasks use background jobs	N/A	Not applicable to in-page serialization.
Medium	Quality	Follows existing codebase patterns	Fail	Finding 1: three serializers in this same PR use an early if (!cloneEl) continue;; serialize-frames.js alone uses ?. instead. Finding 2: the new canvas selector is unquoted where every sibling serializer quotes it.
Medium	Quality	Changes are focused (single concern)	Pass	All changes trace to the one projection fix and its downstream consequences.
Low	Quality	Meaningful names, no dead code	Pass	flattenedHost is clear. Minor: getRootNode() never returns null, so the ?. on it is unreachable.
Low	Quality	Comments explain why, not what	Pass	Comments explain the rendering model and cite PER-10812 — genuinely useful. The three duplicated guard comments read slightly ungrammatically ("light DOM content no <slot> projected").
Low	Quality	No unnecessary dependencies added	Pass	No dependency changes.

Findings

• File: packages/dom/src/serialize-frames.js:150

• Severity: Medium

• Reviewer: stack-code-reviewer

• Issue: cloneEl?. is a partial fix. When a flattened host projects no <iframe>, cloneEl is null, but the loop does not bail out — it enters the frame.contentDocument && frame.contentDocument.documentElement branch (a bare iframe's contentDocument points at about:blank), runs a full recursive serializeDOM(), adds that frame's resources to the resource set, then throws a TypeError on cloneEl.setAttribute('srcdoc', …) which the adjacent empty catch { } silently swallows. Sandboxed-iframe warnings are also emitted for content that never renders. Output HTML is unaffected, so this is wasted recursion, spurious resources and a masked exception rather than a visual regression.

• Suggestion: Bail out once, right after cloneEl is computed (line 75), matching the pattern this PR already applies in serialize-canvas.js, serialize-inputs.js and serialize-video.js:

  let cloneEl = clone.querySelector(`[data-percy-element-id="${percyElementId}"]`);
  // no counterpart in the clone means the iframe wasn't rendered — e.g. light
  // DOM content no <slot> projected, which the browser doesn't render
  if (!cloneEl) continue;

  The two ?. guards then become unnecessary. Consider also strengthening the 'does not throw for unprojected media elements in a flattened host' test to assert the recursive serialization does not run, rather than only that nothing throws.

• File: packages/dom/src/serialize-canvas.js:59

• Severity: Low

• Reviewer: stack-code-reviewer

• Issue: The new guard uses an unquoted attribute-selector template, `[data-percy-element-id=${percyElementId}]`, while serialize-inputs.js, serialize-video.js, serialize-frames.js, serialize-cssom.js and serialize-dom.js all quote the value. It matches the pre-existing unquoted selector in createAndInsertImageElement in the same file, so it is locally consistent, but it copies a latent fragility: an unquoted CSS attribute value must be a valid CSS identifier. Today uid() returns _${random} and is always valid, so this works — but if the ID format ever changes, querySelector throws a SyntaxError instead of returning null, turning a correct "not projected, skip" into a reported canvas-serialization error.

• Suggestion: Quote the value: `[data-percy-element-id="${percyElementId}"]`. Optionally reuse the single lookup rather than querying again inside createAndInsertImageElement a few lines later.

Notes verified, no action needed

• Closed shadow roots. getRootNode()?.host is correct for CDP-captured closed roots: ShadowRoot.host is not gated by mode: 'closed' (only Element.shadowRoot is), and getClosedShadowRoot hands back the real ShadowRoot reference. The new closed-root test confirms it.
• Skipping the light-DOM walk for every flattened host is browser-accurate. A host with a shadow root renders none of its light children except through a <slot>, so dropping unassigned children matches what the browser paints. This is a deliberate and correct behavior change from the previous unconditional clone.
• if (!isChrome()) return; in the new tests is pre-existing convention in this package — serialize-dom.test.js already guards on navigator.userAgent in a dozen places because karma also runs FirefoxHeadless. Not a new problem.

---

Reviewer recommendation was request changes on the strength of Finding 1. Under this gate's rule the verdict is computed from severity — the highest open finding is Medium, so the status is green — but Finding 1 is a genuine defect reachable by this PR's own test and is worth fixing before merge.

Verdict: PASS — core fix and its test coverage are solid; one Medium and one Low remain open, neither gating.