fix(dom): stop rewriting :checked/:disabled rules — cascade-order corruption recolors Angular Material buttons (PER-10077) - #2342
Open
prklm10 wants to merge 4 commits into
Open
Conversation
…ade order (PER-10077) The interactive-states auto-detect path (PER-7292) copied every CSS rule containing :checked/:disabled, rewrote the pseudo to a data-percy-* attribute selector, and appended the copy at the end of <head>. The copy flips cascade ties: any equal-specificity rule from a later stylesheet that was NOT copied (no pseudo-class in its selector) loses to the re-injected copy purely on source order. Angular Material apps hit this hard — its pervasive .mat-mdc-raised-button:not(:disabled) rules were re-injected last and overrode customer theme overrides, recoloring buttons across 70+ snapshots. Both states serialize natively: disabled is a reflected content attribute, and serialize-inputs syncs checked/selected properties to attributes on the clone — so :checked/:disabled match in the renderer without any rewriting. Drop them from the auto-detect stamp + rewrite; keep :focus/:focus-within (not serializable) and :hover/:active (opt-in via pseudoClassEnabledElements). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
Minimal repro + end-to-end verification (also saved in percy-ops 3-file page mirroring the customer's cascade: sheet 1 has Local A/B (serialize with a given @percy/dom bundle, reload serialized HTML with page JS disabled, measure computed style):
Real Percy builds — same page, same CLI, only the dom bundle swapped:
|
…aIP (PER-10077)
The @percy/core 100% coverage gate was failing on utils.js — the
canonicalHost() catch branch (return bare when new URL('http://[host]/')
throws for a colon-containing, non-IPv6 host) had no deterministic test,
so coverage of that line depended on a flaky spec and CI stayed red on
this PR even though all specs pass.
Add an isMetadataIP('gg::1') case: the colon routes it into the IPv6
normalization branch, the invalid literal makes new URL throw, and the
catch falls back to the bare host (not a metadata target -> null).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ascade position (PER-10077) The interactive-states auto-detect copies every CSS rule matching :focus/ :focus-within/:hover/:active into a <style> and injected it at the END of <head>. That moved the copy to the back of the cascade, so it won equal-specificity !important ties it should have lost — the root cause of PER-10077 (Angular Material :not(:disabled) rules recolored buttons). The prior fix only stopped copying :checked/:disabled (which serialize natively), sidestepping the customer's trigger but leaving the same end-of-head hazard for :focus/:hover/:active on any page. Complete fix: group rewritten rules PER SOURCE SHEET and insert each sheet's <style> immediately AFTER that sheet's clone element instead of at end-of-head, so the copy keeps the sheet's original source-order rank and later stylesheets still win their ties — exactly as the live page renders. - serialize-pseudo-classes.js: per-sheet grouping + injectAtSheetPosition() anchors the copy after the sheet's clone element (found via the ownerNode's data-percy-element-id); falls back to end-of-head when no anchor exists. - prepare-dom.js: stamp <link rel=stylesheet> with data-percy-element-id so external sheets (the customer's real structure) can be anchored too. - :checked/:disabled stay on native serialization (already complete). Tests: per-sheet injection; cascade-position (unit + end-to-end) asserting the copy sits after its sheet and before the later sheet; stamped-sheet fallback; and link-stamping (stylesheet stamped, preload/no-rel not). Full @percy/dom suite passes in Chrome; prepare-dom 100% coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes PER-10077 — AON customer, 70+ visual diffs.
Root cause
packages/dom/src/serialize-pseudo-classes.js(interactive-states auto-detect, shipped in the 1.31.8 → 1.31.14 window via PER-7292 / #2177):markInteractiveStatesstamped every live:disabled/:checkedelement withdata-percy-disabled/data-percy-checked.extractPseudoClassRulescopied every CSS rule whose selector contains:disabled/:checked, rewrote the pseudo to the attribute selector, and appended the copy in a<style data-percy-interactive-states>at the end of<head>.The copy flips cascade ties. In the customer's app:
styles-*.css):.mat-mdc-raised-button:not(:disabled) { background-color: var(--mat-button-protected-container-color, white) !important }→ resolves#36495aonmat-primarybuttonsrpcc-styles-*.css, later):.mat-mdc-raised-button.mat-primary { background-color: #1b98e0 !important }→ blueBoth are
!importantwith equal specificity (0,2,0), so later source order wins → blue (matches live Chrome and the 1.31.8 baseline). Percy copied rule 1 (contains:disabled) to the end of head — rule 2 has no pseudo-class so it was not copied — making the copy the last equal-specificity!importantdeclaration → buttons rendered#36495ainstead of#1b98e0.Reproduced on snapshot 2792517378 / 2792516808 of build 51680903: loading the serialized DOM in headless Chrome and deleting only the injected
<style data-percy-interactive-states>flips the Execute buttons fromrgb(54,73,90)(broken head screenshot) back torgb(27,152,224)(baseline screenshot).Fix
Drop
:checkedand:disabledfrom the auto-detect stamp + rewrite entirely. Both states serialize natively, so the rewrite added no coverage:disabledis a reflected content attribute — always present in the serialized HTML (including<fieldset disabled>ancestors), so:disabledmatches in the renderer as-is.checked/selectedproperties are synced to attributes on the clone byserialize-inputs.js, so:checkedmatches as-is.:focus/:focus-within(genuinely not serializable) and opt-in:hover/:activeare unchanged. Their rewritten copies only ever match explicitly stamped elements, but note the same end-of-head reordering hazard exists in principle for:not(:focus)-style rules — preserving cascade position for injected copies would be the complete fix and can be a follow-up.Testing
yarn workspace @percy/dom test— 900 tests passserialize-pseudo-classes.test.js: 'cascade safety for :checked / :disabled rules (PER-10077)' — two-sheet equal-specificity!importantscenario asserts no:not(:disabled)copy is injected; rewritePseudoSelector expectations flipped to assert:checked/:disabledpass through unchangedserialize-dom.test.js: end-to-end asserts nodata-percy-checked/data-percy-disabledstamping or rule rewriting:checkedas a vehicle (@media wrapper preservation, multi-sheet owner grouping, createElement/head fallbacks) switched to:hover, which still exercises the same paths🤖 Generated with Claude Code