[pranavz28]

Fixes PER-10567

Root cause

cloneNodeAndShadow's STYLE branch — added in #1884 to capture CSSOM-only (JS-inserted) <style> sheets — set clone.textContent = cssText for every <style>, including ones with their own authored text:

let cssText = node.textContent?.trim() || '';   // authored text
if (!cssText && node.sheet) { /* synthesize from cssRules */ }
if (cssText) {                                  // ran for authored styles too
  clone.textContent = cssText;
  clone.setAttribute('data-percy-cssom-serialized', 'true');
}

clone is a shallow clone (element.cloneNode()), and the generic walkTree(node.firstChild, clone) call further down re-clones the live text node onto it. So for any <style> with authored text, the CSS was written into the clone twice — once manually here, once by walkTree.

Three consequences:

1. Every inline <style> with authored text shipped twice in the serialized DOM of every snapshot, since March 2025.
2. data-percy-cssom-serialized="true" was set on every non-empty <style>, not only on sheets actually re-serialized from CSSOM — a false positive for essentially every ordinary <style> tag.
3. The doubled clone sheet parses to twice the rule count, which broke serializeCSSOM's identity guard styleSheetsMatch(liveSheet, styleSheetFromNode(cloneOwnerNode)). Before PER-7129: Fix the cssom serialization bug of not accounting for inherit:all css rule  #2228 that guard rejected on lenA !== lenB, so a doubled sheet always fell through to the lossy cssRules.map(r => r.cssText) path.

That lossy path is what produced the reported symptom. Chrome cannot serialize a var()-containing shorthand that a later longhand partially overrides, so it expands it into pending-substitution longhands that serialize empty:

/* live */
#global-translate-button .button__text { background: var(--nyc-ref-gradient-rainbow); background-clip: text; color: transparent }

/* re-serialized from cssRules */
#global-translate-button .button__text { background-image: ; background-position-x: ; …; background-color: ; background-clip: text; color: transparent }

color: transparent and background-clip: text survive, the gradient does not — the text renders invisible. One customer page produced 278 empty declarations in a single sheet this way. The loss is not recoverable downstream: rule.style.getPropertyValue('background') also returns "" for this shape, so the authored text is the only source of truth.

Fix

Only synthesize clone.textContent when the <style> has no text node of its own — i.e. CSSOM-only sheets, which is what #1884 was actually for. A <style> with authored text now gets it exactly once, from walkTree, and no longer carries the CSSOM marker.

Verification

Repro page (background: var(--gradient); background-clip: text; color: transparent, mirroring the customer rule), served locally, serialized in real Chrome via CDP with the @percy/dom bundle built from each branch:

	master (9352a73)	this PR
serialized DOM	2618 chars	1943 chars (−25.8%)
total <style> content	1664 chars	1024 chars (−38.5%)
authored <style> content	1284 chars (CSS present twice)	644 chars (present once)
data-percy-cssom-serialized markers	2 (1 false)	1 (correct)

Percy builds on the same page, fix build based on the master build:

• baseline (master bundle): https://percy.io/9560f98d/web/test-pranav-8a4f5725/builds/53173287
• fix (this branch): https://percy.io/9560f98d/web/test-pranav-8a4f5725/builds/53173297 — 0 diffs / 5 comparisons

So the change is render-neutral while removing ~26% of the serialized DOM on a page with two inline <style> tags.

Tests: yarn workspace @percy/dom test --karma.browsers ChromeHeadless — 496/496 passing. yarn workspace @percy/dom build — passes.

• New regression test: an authored <style> with a var() shorthand survives serializeDOM() with its text intact, exactly once, and unmarked.
• Two pre-existing serialize-cssom tests had the false-positive marker counts baked in as expectations (3 → 2, 2 → 1) and are corrected.
• No regression to Added patch to capture dynamic media queries #1884's intent: a CSSOM-only <style> (including @media rules inserted via insertRule) is still captured and still marked — covered by the existing preserves media queries inside CSSOM / maintains order of CSSOM serialization tests, and re-confirmed manually.

Scope — what this does not fix

#2228 later relaxed styleSheetsMatch to only reject when lenA > lenB, which tolerates a clean doubling. So on 1.32.0+ the duplication no longer forces the lossy path by itself; consequence 3 above only bites on 1.30.x–1.31.x (relevant to products bundling an older percyDom.js).

A sheet that has any genuine CSSOM delta — insertRule, deleteRule, or an in-place rule.style mutation — still fails the guard and still goes through the lossy re-serialization, losing var() shorthands for the whole sheet. Verified on both master and this branch: the repro page's second <style> (authored text + one insertRule) yields 8 empty declarations on both, i.e. the invisible-gradient symptom persists there.

That remaining defect is in serializeCSSOM, not clone-dom: it replaces the clone's authored text wholesale instead of applying only the delta, and the lost value is unrecoverable from CSSOM. It needs a separate fix and is deliberately out of scope here. This PR removes the duplication that made the guard unreliable in the first place, and is a prerequisite for any per-rule delta approach.

[pranavz28]

Claude Code PR Review

PR: #2393 • Head: 9b81343 • Reviewers: code-review (fallback inline checklist — no in-scope installed reviewers discovered)

Summary

Fixes PER-10567: cloneNodeAndShadow's STYLE handling in packages/dom/src/clone-dom.js was unconditionally writing clone.textContent/data-percy-cssom-serialized for every non-empty <style>, even ones with their own authored text — duplicating the authored CSS a second time via the later generic walkTree copy, and mismarking ordinary <style> tags as CSSOM-synthesized. The fix only synthesizes clone.textContent from sheet.cssRules when the node has no authored text of its own (true CSSOM-only sheets), matching the marker's original intent from #1884; two pre-existing tests with baked-in duplicate-count expectations are corrected (3→2, 2→1) and one new regression test is added.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	N/A	No credentials/secrets involved — pure DOM string handling
High	Security	Authentication/authorization checks present	N/A	Client-side DOM serialization library, no auth surface
High	Security	Input validation and sanitization	N/A	Operates on live browser CSSOM/DOM APIs, not external/user input
High	Security	No IDOR — resource ownership validated	N/A	Not applicable to this module
High	Security	No SQL injection (parameterized queries)	N/A	No data-layer code
High	Correctness	Logic is correct, handles edge cases	Pass	Verified ownText gating against empty, whitespace-only, authored, and CSSOM-only <style> text; matches the PR's documented root cause. One residual edge case (a whitespace-only-text <style> that also has a live sheet gets the synthesized CSS plus a trailing empty text node from walkTree) is pre-existing behavior, unchanged by this diff — confirmed by reading master's equivalent code path, cosmetically harmless
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	try/catch around sheet.cssRules access is preserved unchanged from master; same intentional "ignore and move on" behavior
High	Correctness	No race conditions or concurrency issues	N/A	Synchronous tree walk, no concurrency introduced
Medium	Testing	New code has corresponding tests	Pass	New regression test (does not duplicate authored <style> text, preserving background: var(--x) shorthands) directly targets the fixed defect
Medium	Testing	Error paths and edge cases tested	Pass	Existing coverage for empty stylesheets, media queries, CSSOM ordering, and enableJavaScript bypass all still pass unmodified
Medium	Testing	Existing tests still pass (no regressions)	Pass	Ran the full @percy/dom Karma suite (ChromeHeadless) on the PR branch directly: 496/496 passing, matching the PR's stated verification
Medium	Performance	No N+1 queries or unbounded data fetching	N/A	Not applicable — in-memory DOM walk
Medium	Performance	Long-running tasks use background jobs	N/A	Not applicable
Medium	Quality	Follows existing codebase patterns	Pass	Consistent with the surrounding try/catch and marker-attribute conventions
Medium	Quality	Changes are focused (single concern)	Pass	Diff is exactly 2 files, ~20 lines net; tightly scoped to the described bug
Low	Quality	Meaningful names, no dead code	Pass	cssText → ownText rename clarifies the authored-vs-synthesized distinction; the synthesized value is now scoped inside the if block
Low	Quality	Comments explain why, not what	Pass (diff itself)	See Finding below — an adjacent, untouched file (serialize-cssom.js) carries a rationale comment that references the bug this PR fixes, now partially stale
Low	Quality	No unnecessary dependencies added	Pass	None added

Findings

• File: packages/dom/src/serialize-cssom.js:36
• Severity: Low
• Reviewer: code-review (fallback)
• Issue: styleSheetsMatch's if (lenA > lenB) return false; tolerance (added to accommodate clone-dom's now-fixed <style> text duplication — see the comment at lines 28–30, "often duplicated by clone-dom's <style> textContent handling") is not revisited by this PR even though the PR removes the duplication that comment describes. The dual justification (duplication-tolerance and shorthand-preservation) still mostly holds because the shorthand-preservation half is independent of the duplication bug, so this is not a functional regression from this PR. But the comment is now partially inaccurate framing, and the tolerance still unconditionally trusts a clone with a rule-count surplus over the live sheet (lenA <= lenB) without verifying content — a latent gap unrelated to, and pre-existing before, this PR's change.
• Suggestion: File a fast-follow to reword the serialize-cssom.js comment (drop or qualify the "often duplicated by clone-dom" framing now that clone-dom no longer does this for authored <style> elements) and, if desired, re-examine whether the lenA > lenB-only tolerance is still the right guard now that its original motivating case is fixed. Not a blocker for this PR — the PR description itself explicitly scopes serializeCSSOM changes out (see "Scope — what this does not fix").

Note: This finding's file is not part of this PR's diff (packages/dom/src/serialize-cssom.js is untouched), so it is reported here only — not as an inline PR comment (would 422, no anchor line in the diff).

---

Verdict: PASS

[pranavz28]

Triage of the review finding

packages/dom/src/serialize-cssom.js:36 — styleSheetsMatch tolerance / stale comment — split verdict.

Accepted: the stale comment. Genuine, and this PR is what invalidates it — leaving it would be debt this change created. Fixed in df9b61e. Two places, not one:

• serialize-cssom.js:30 — dropped "(often duplicated by clone-dom's <style> textContent handling)". The tolerance no longer has that motivation, so the comment now states only what it is for.
• test/serialize-css.test.js:464 — the regression: cloned <style> text-node duplication block described the duplication as current clone-dom behaviour ("what clone-dom currently produces"), in a file this PR does touch. Trimmed to describe the tolerance itself. The test is unchanged and still valuable: it constructs the duplicated clone sheet synthetically, so it pins styleSheetsMatch's behaviour directly rather than through clone-dom.

Rejected: reconsidering the lenA > lenB tolerance itself. Out of scope, and I'd argue against it on merit:

1. The tolerance is still reachable — deleteRule on the live sheet gives lenA < lenB with authored text intact. Duplication was one input shape that hit it, not its only one.
2. Its shorthand-preservation rationale is independent of clone-dom and still valid. Tightening it back toward lenA !== lenB would push exactly those sheets into the cssRules.map(r => r.cssText) path — the path that destroys background: var(--x) and caused PER-10567. That would reintroduce the reported symptom, not reduce risk.
3. @percy/dom is bundled by other products at pinned older versions; some of those still have the duplicating clone step. The tolerance is what keeps them from regressing.

So the guard stays exactly as-is. Verified after the change: 496/496 dom tests passing, including both regression: cloned <style> text-node duplication cases.

Independently of the review, the PR description now also carries a "Scope — what this does not fix" section: a sheet with any genuine CSSOM delta still fails the guard and still loses var() shorthands for the whole sheet. Reproduced on both master and this branch (8 empty declarations either way), and the lost value is unrecoverable from CSSOM — rule.style.getPropertyValue('background') returns "" for that shape. That residual defect lives in serializeCSSOM's wholesale text replacement and needs its own fix; this PR is a prerequisite for any per-rule delta approach, not a substitute for it.

[pranavz28]

Claude Code PR Review

PR: #2393 • Head: df9b61e • Reviewers: code-review (fallback inline checklist — no in-scope installed reviewers discovered)

Continues the previous review — changes since 9b81343 (delta).

Summary

Follow-up commit df9b61e2 ("docs(dom): drop stale clone-dom duplication references from styleSheetsMatch") addresses the Low finding from the previous review. It is a pure comment/prose change across the same two files (packages/dom/src/serialize-cssom.js, packages/dom/test/serialize-css.test.js): it removes the now-inaccurate "(often duplicated by clone-dom's <style> textContent handling)" framing from the styleSheetsMatch rationale comment and trims the parallel stale wording from the regression: cloned <style> text-node duplication test block, which had described the duplication as clone-dom's current behavior. The if (lenA > lenB) return false; guard, its surrounding logic, and every test assertion/body are byte-for-byte unchanged — confirmed directly from the diff and independently by the dispatched reviewer.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	N/A	Comment-only change
High	Security	Authentication/authorization checks present	N/A	Not applicable
High	Security	Input validation and sanitization	N/A	Not applicable
High	Security	No IDOR — resource ownership validated	N/A	Not applicable
High	Security	No SQL injection (parameterized queries)	N/A	Not applicable
High	Correctness	Logic is correct, handles edge cases	Pass	Zero logic changed — verified the lenA > lenB guard and all surrounding code are identical to the prior head; only comment prose was edited
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	Unchanged from prior review
High	Correctness	No race conditions or concurrency issues	N/A	Unchanged from prior review
Medium	Testing	New code has corresponding tests	N/A	No new code in this delta; only comment wording
Medium	Testing	Error paths and edge cases tested	Pass	Test assertions/bodies untouched; still passing per prior full-suite run against unchanged logic
Medium	Testing	Existing tests still pass (no regressions)	Pass	No executable lines changed in this delta — no re-run needed to confirm; prior 496/496 result stands
Medium	Performance	No N+1 queries or unbounded data fetching	N/A	Not applicable
Medium	Performance	Long-running tasks use background jobs	N/A	Not applicable
Medium	Quality	Follows existing codebase patterns	Pass	Comment now accurately describes only the tolerance's live purpose (CSS shorthand/logical-longhand ordering), matching the "why" style used elsewhere
Medium	Quality	Changes are focused (single concern)	Pass	Exactly the two comment blocks flagged in the prior review, nothing else touched
Low	Quality	Meaningful names, no dead code	Pass	N/A to this delta
Low	Quality	Comments explain why, not what	Pass	The prior finding is resolved: the stale "often duplicated by clone-dom" claim is gone from both the source comment and the test's block comment; remaining "duplicate"/"cloned" mentions are accurate descriptions of what the regression tests check for, not claims about current clone-dom behavior
Low	Quality	No unnecessary dependencies added	Pass	None

Findings

packages/dom/src/serialize-cssom.js:36 Low — Stale rationale comment on styleSheetsMatch's lenA>lenB tolerance Resolved by df9b61e2 — the stale "often duplicated by clone-dom's <style> textContent handling" clause was removed from the comment (and the parallel stale wording trimmed from the test's block comment). Verified the diff touches comments only — the guard logic, its return values, and every test assertion are unchanged — both from direct diff inspection and independently by the dispatched code-review reviewer.

No new findings.

---

Verdict: PASS