fix(dom): scope nested interactive-state selectors to their parent (PER-9775) by Sriram567 · Pull Request #2324 · percy/cli

Sriram567 · 2026-06-29T03:46:53Z

Fixes PER-9775.

Symptom

From @percy/cli 1.31.14 onward, snapshots of pages built with CSS-in-JS (emotion) render with a blue page background, black backgrounds behind SVG icons, and wrong button colors — across all four browsers, including Safari whose engine version did not change. The triggering customer commit only bumped dependencies (@percy/cli 1.31.13 → 1.31.14).

Root cause

The interactive-states feature (#2177) walks every CSS rule, rewrites interactive pseudos (:hover/:focus/:focus-within/…) to [data-percy-*] attribute selectors, and injects them into a <style data-percy-interactive-states> block in the cloned <head>.

walkCSSRules recursed into nested rules but only preserved at-rule preludes (@media/@layer/@supports). For native CSS nesting — a style rule with nested style-rule children, which emotion emits — it dropped the parent selector and emitted the child with its &-relative selector intact:

result.push(inner); // inner.selectorText is still "&:hover"

So .cta-card { &:hover { background: var(--blue) } } was flattened to a bare &:hover, rewritten to &[data-percy-hover], and injected at document <head>. On every current engine that supports CSS nesting (Chrome/Edge/Firefox and Safari 17.3+), a top-level & resolves to :root, so the component-scoped rule applied to the whole document. Combined with :focus-within markers being stamped up the ancestor chain onto <html>/<body>, :root-matching rules paint page-wide.

This explains the puzzle that pointed people away from the CLI: the regression is baked into the captured DOM, so it's browser-agnostic — Safari breaks too even though its version was unchanged.

Fix

Thread a parentSelector through walkCSSRules and resolve each nested selector's & against it via :is(<parent>):

.cta-card { &:hover {…} } → :is(.cta-card):hover → :is(.cta-card)[data-percy-hover] (component-scoped, no :root leak).

The CSSOM serializes nested selectors with the implicit nesting selector made explicit, so every nested selector — including each item of a selector list — carries its own &; a global &→:is(parent) replace therefore scopes lists correctly too. At-rule prelude handling (@media/@layer/@supports) is unchanged.

Testing

4 new regression tests in serialize-pseudo-classes.test.js: &:hover scoping, &-less descendant (& a:hover), nested selector lists (&:hover, &:focus), and a complex :is()+attribute selector. Verified red→green (they fail on master, pass with this change).
Full @percy/dom suite passes (902/902); lint clean; coverage of changed code is 100%.

🤖 Generated with Claude Code

…ER-9775) walkCSSRules recursed into native CSS nesting but only preserved at-rule preludes (@media/@layer/@supports), dropping the parent style-rule selector. A nested interactive-pseudo rule like `.card { &:hover { … } }` was therefore flattened to a bare `&:hover`, rewritten to `&[data-percy-hover]`, and injected into the document <head> interactive-states block. On engines that support CSS nesting (Chrome/Edge/Firefox and Safari 17.3+), a top-level `&` resolves to `:root`, so component-scoped styles applied to the whole page — blue page background, black SVG backgrounds, wrong button colors. Thread a parentSelector through walkCSSRules and resolve each nested selector's `&` against it via :is(<parent>) (the CSSOM serializes nested selectors with an explicit `&`, so this scopes selector lists too). At-rule prelude handling is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

amandeepsingh333 · 2026-06-29T10:32:07Z

+  // `&[data-percy-hover]`. A top-level `&` resolves to :root on engines that
+  // support nesting, leaking component styles to the whole page (blue
+  // background, black SVG backgrounds, wrong button colors).
+  describe('native CSS nesting — parent selector resolution (PER-9775)', () => {


remove the ticket id

Done in ba16b31 — removed the ticket id from the describe name and the leading comment.

amandeepsingh333 · 2026-06-29T10:34:18Z

+      const childParent = atRulePrelude
+        ? parentSelector
+        : (rule.selectorText
+            ? resolveNestedSelector(rule.selectorText, parentSelector)
+            : parentSelector);


nested ternary operator : difficult to understand
please make this better either by having a helper fxn or changing code

Done in ba16b31 — replaced the nested ternary with a default assignment + a single guarded if (let childParent = parentSelector; if (!atRulePrelude && rule.selectorText) childParent = resolveNestedSelector(...)). Same behavior, and the branch stays 100% covered.

… from test - replace the nested ternary in walkCSSRules with a default + guarded assignment (clearer, same behavior, fully covered) - remove the PER ticket id from the regression describe name and comment Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Sriram567 · 2026-06-29T11:35:39Z

Claude Code PR Review

PR: #2324 • Head: ba16b31 • Reviewers: stack:code-reviewer

Summary

Fixes PER-9775: native CSS nesting (emotion / CSS-in-JS) emits child rules with &-relative selectors that walkCSSRules previously stripped, so an interactive-pseudo child like &:hover leaked as a bare &[data-percy-hover] — and a top-level & resolves to :root, leaking component styles page-wide. The fix threads a parentSelector through walkCSSRules and resolves & to :is(parent) via a new resolveNestedSelector helper, scoping nested interactive-state rules to their parent. Adds 4 regression tests covering nested &:hover, &-less descendants, top-level selector lists, and a complex :is()/attribute selector.

Review Table

Priority	Category	Check	Status	Notes
High	Security	No hardcoded secrets or credentials	Pass	None introduced
High	Security	Authentication/authorization checks present	N/A	DOM-serialization code, no auth surface
High	Security	Input validation and sanitization	Pass	Operates on CSSOM-parsed `selectorText` (already syntactically valid); `nosemgrep`-annotated `innerHTML`/`textContent` writes are pre-existing test/clone patterns
High	Security	No IDOR — resource ownership validated	N/A	No resources
High	Security	No SQL injection (parameterized queries)	N/A	No DB
High	Correctness	Logic is correct, handles edge cases	Pass	`&`→`:is(parent)` substitution is correct; at-rules keep parent scope, style rules resolve their own selector first; multi-level nesting composes via recursion; selector lists each carry their own `&`
High	Correctness	Error handling is explicit, no swallowed exceptions	Pass	Existing cross-origin/`try` guards unchanged
High	Correctness	No race conditions or concurrency issues	N/A	Synchronous walk
Medium	Testing	New code has corresponding tests	Pass	4 new tests exercise the nesting path; `nestingSupported()` guards that the engine actually parses nesting
Medium	Testing	Error paths and edge cases tested	Pass	Selector-list, `&`-less, and complex-selector cases covered; 3-level nesting not pinned (Low #4)
Medium	Testing	Existing tests still pass (no regressions)	Pass	Change is additive; top-level rules (`parentSelector=null`) return unchanged
Medium	Performance	No N+1 queries or unbounded data fetching	Pass	One `String.replace` per rule
Medium	Performance	Long-running tasks use background jobs	N/A
Medium	Quality	Follows existing codebase patterns	Pass	Matches the existing `walkCSSRules`/wrapper style; well-commented
Medium	Quality	Changes are focused (single concern)	Pass	Scoped to the nesting-leak fix
Low	Quality	Meaningful names, no dead code	Pass	`resolveNestedSelector` / `childParent` read clearly; nested ternary removed per review
Low	Quality	Comments explain why, not what	Pass	The `why` (bare `&`→`:root` leak) is documented on both the helper and the recursion
Low	Quality	No unnecessary dependencies added	Pass	None

Findings

No blocking issues. All findings are Low / informational.

File: packages/dom/src/serialize-pseudo-classes.js:341
Severity: Low
Reviewer: stack:code-reviewer
Issue: resolveNestedSelector does a global /&/g replace. This is safe because CSSOM-serialized selectorText only ever contains & as the nesting-selector token (never inside attribute values or string literals), but that invariant isn't documented.
Suggestion: Add a one-line comment noting & only appears as the nesting token in CSSOM selectorText.
File: packages/dom/src/serialize-pseudo-classes.js:341 (the if (hasNested) branch)
Severity: Low (pre-existing; not worsened by this PR)
Reviewer: stack:code-reviewer
Issue: walkCSSRules only recurses into rule.cssRules for a nested rule and never emits the parent rule's own declarations. A rule that simultaneously has its own interactive-pseudo declarations and native-nested children (e.g. .card:hover { color: red; &.active { … } }) would drop the .card:hover declarations. Unusual in practice and structurally unchanged from the original code.
Suggestion: Add a comment near the hasNested branch documenting that a nesting rule's own declarations are not emitted.
File: packages/dom/test/serialize-pseudo-classes.test.js:1825
Severity: Low
Reviewer: stack:code-reviewer
Issue: nestingSupported() scans all of document.styleSheets, so a prior test's still-attached sheet with nested rules could return true even if the current test's own sheet failed to parse — a potential false-positive guard. Benign in the current Jasmine/headless-Chrome runner.
Suggestion: Scope the check to the sheet added by the current withExample() call.
File: packages/dom/test/serialize-pseudo-classes.test.js:1804
Severity: Low
Reviewer: stack:code-reviewer
Issue: No regression test for 3-level nesting (.a { .b { &:hover {} } }). The recursion handles it (producing :is(:is(.a) .b)[data-percy-hover]), but there's no guard pinning that the double-:is() wrapping stays correct.
Suggestion: Add a 3-level nesting test asserting no bare & leaks and the resolved selector matches the target element.

Raised by other reviewers (not independently confirmed)

Both human review threads from @amandeepsingh333 are resolved — confirmed against current HEAD (ba16b31):

~~packages/dom/src/serialize-pseudo-classes.js — "nested ternary operator: difficult to understand"~~ Resolved — the nested ternary is replaced by a default assignment + single guarded if (let childParent = parentSelector; if (!atRulePrelude && rule.selectorText) childParent = resolveNestedSelector(...)). Confirmed in HEAD.
~~packages/dom/test/serialize-pseudo-classes.test.js — "remove the ticket id"~~ Resolved — no ticket id remains in the describe name or leading comment. Confirmed in HEAD.

Verdict: PASS — correct, focused fix for the nesting leak with solid regression coverage; only Low/informational nits remain, and prior human review comments are addressed.

Sriram567 · 2026-06-29T11:55:02Z

@AkashBrowserStack — the percy/Percy-CLI-regression check here is flagging the 9 new Config - snapshots from your PER-8250 E2E config-coverage work; they have no baseline in the project yet (build 51328641), so this PR's merge with master is among the first builds to include them and Percy marks them "needs review." This PR itself is a DOM-serialization fix (PER-9775) and doesn't touch the regression fixtures. Could you review/approve the visual build to baseline them? 🙏

…PER-10588) The interactive-state auto-detect path has shipped four regressions in three months, all of the same shape: it copies customer CSS, rewrites a pseudo-class to a data-percy-* attribute, and re-injects the copy, which reorders the cascade for rules nobody asked it to touch. PER-9775 (#2324) nested selectors leaked page-wide via a bare `&` PER-9836 pseudoClassEnabledElements froze layout/transitions PER-10077 (#2342) :checked/:disabled copies recolored Angular Material buttons across 70+ snapshots PER-10588 one :hover member of an 82-selector rule repainted every colored-card heading on an AEM site Adoption does not justify that risk. On the PER-10588 customer's captured DOM the injected block was 289KB with 629 [data-percy-hover] selectors and ZERO data-percy-* attributes stamped on any element — nothing it emitted could match. The feature contributed no correct behavior and all of the breakage. Gate both paths behind a new snapshot option, enablePseudoClassSerialization (default false; snake_case enable_pseudo_class_serialization also accepted). Configuring pseudoClassEnabledElements is itself an opt-in and turns the feature on without the flag, so no existing opt-in user changes behavior. The predicate is computed once in serializeDOM and carried as ctx.pseudoClassSerialization — one auditable switch, not a check per call site. No pseudo-class logic is removed. Deliberately NOT gated, both verified against percy-renderer: - Open-popover stamping. src/script/popover-element-helper.js reopens [popover][data-percy-popover-open]; without the stamp an open popover renders hidden behind the UA [popover]:not(:popover-open) rule. - Custom element :state() rewriting. serialize-custom-states rewrites <style> text in place, so it cannot reorder the cascade. The renderer consumes only shadow-host, injected, popover-open, dialog-modal, scrolltop/left and freeze-animation-logs attributes — none of the gated focus/hover/active stamps — so nothing downstream regresses. Verified end to end on the PER-10588 customer's real page and CSS: master 714,774 bytes, injected block, headings rgb(0,75,111) gated (default) 429,635 bytes, no block, headings white gated + flag 629,882 bytes, injected block, headings white @percy/dom is regression-clean against baseline (466 pass vs 459, same 40 pre-existing focus failures on a headless browser without window focus), plus 7 new specs covering default-off, both opt-in routes, the snake_case alias, and the two carve-outs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Sriram567 requested a review from a team as a code owner June 29, 2026 03:46

Sriram567 requested review from amandeepsingh333 and ninadbstack June 29, 2026 03:46

amandeepsingh333 reviewed Jun 29, 2026

View reviewed changes

Sriram567 enabled auto-merge (squash) June 29, 2026 14:05

rishigupta1599 approved these changes Jun 29, 2026

View reviewed changes

Sriram567 merged commit fe24a65 into master Jun 29, 2026
81 of 83 checks passed

Sriram567 deleted the fix/PER-9775-nested-pseudo-selector-leak branch June 29, 2026 14:13

pranavz28 mentioned this pull request Aug 25, 2026

fix(dom): make interactive-state serialization opt-in, default off (PER-10588) #2399

Merged

Navigation Menu

Uh oh!

fix(dom): scope nested interactive-state selectors to their parent (PER-9775) - #2324

fix(dom): scope nested interactive-state selectors to their parent (PER-9775)#2324
Sriram567 merged 2 commits into
masterfrom
fix/PER-9775-nested-pseudo-selector-leak

Sriram567 commented Jun 29, 2026

Uh oh!

amandeepsingh333 Jun 29, 2026

Uh oh!

Sriram567 Jun 29, 2026

Uh oh!

amandeepsingh333 Jun 29, 2026

Uh oh!

Sriram567 Jun 29, 2026

Uh oh!

Sriram567 commented Jun 29, 2026

Uh oh!

Sriram567 commented Jun 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

Sriram567 commented Jun 29, 2026

Symptom

Root cause

Fix

Testing

Uh oh!

amandeepsingh333 Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

Sriram567 Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

amandeepsingh333 Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

Sriram567 Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

Sriram567 commented Jun 29, 2026

Claude Code PR Review

Summary

Review Table

Findings

Raised by other reviewers (not independently confirmed)

Uh oh!

Sriram567 commented Jun 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants