fix(restructure): keep universal-subject selectors from reordering across same-specificity typed selectors#494
Open
spokodev wants to merge 1 commit into
Conversation
…ross same-specificity typed selectors The restructurer treats two equal-specificity selectors with different subject element types as non-conflicting, using the `,tagName` suffix of compareMarker as a fast-path. That assumption breaks when a selector has a universal subject (no type selector on its subject), because such a selector matches an element of any type and can collide with a same-specificity typed-subject selector. As a result the restructurer could reorder/merge two rules across an intervening rule that targets the same element at equal specificity, flipping the cascade winner. hasSimilarSelectors now also reports a collision when either subject is universal and the specificity (plus scope) matches, so the restructurer stops at such a rule instead of skipping past it.
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
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.
Problem
The restructurer can change the meaning of valid CSS by reordering two rules across an intervening rule that targets the same element at equal specificity, flipping the cascade winner.
That output is correct (the merged
div .xstays after.y span, so red still wins). But onv5.0.5/ currentmainthe same input drops the third rule entirely:For
<div class="y"><span class="x"></span></div>, the span matches bothdiv .xand.y span, each with specificity(0,1,1). Per the CSS Cascade, Order of Appearance (§6.4.4, https://www.w3.org/TR/css-cascade-4/#cascade-order), the later rule wins, so the element computescolor: red. The dropped-rule output computescolor: green. Browser-verified red -> green.minify(css, { restructure: false })keeps all three rules, which isolates the issue to the restructurer.Root cause
processSelectorbuildscompareMarker = specificity + (",<tagName>" if the subject has a type selector). ACombinatorresetstagNameto*, so:div .x-> subject.x, no type -> marker0,1,1.y span-> subjectspan-> marker0,1,1,spanhasSimilarSelectorscompares markers for exact equality. The differing,tagNamesuffix is a fast-path that assumes two equal-specificity selectors with different subject element types can never match the same element. That assumption does not hold for a universal subject:.x(no type) can match a<span>, sodiv .xand.y spancan match the same element and must be treated as conflicting. Because they were not, the restructurer skipped past.y spanand merged the twodiv .xrules across it.Fix
processSelectornow also records, per simple selector, the specificity-only key (marker without the,tagNamesuffix) and whether the subject is universal.hasSimilarSelectorsreports a collision when either subject is universal and the specificity (plus scope) matches, in addition to the existing exact-marker equality.compareMarkeritself is unchanged, so the merge-eligibility logic in7-mergeRulesetand8-restructRulesetis untouched; only the skip/reorder gate gains the universal-subject awareness. Legitimate merges are preserved: two equal-specificity selectors with distinct concrete subject types (e.g..y spanvs.z em) still group, and the bug-family inputs now reorder into a cascade-preserving form instead of dropping a rule.@keyframesselectors keep their by-value comparison (the fast-path is held inert there).Tests
fixtures/similarSelectors.json(div .xvs.y span, both orderings), which drivetest/hasSimilarSelectors.js. They fail on currentmain(false, expectedtrue) and pass with the fix.539 passing, 4 pending, lint clean.Related
Issue #457 reports another restructurer cascade flip, but with a different mechanism (universal-subject attribute selectors merged at the declaration level). This fix targets the universal-subject marker collision in the skip/reorder gate and does not change the #457 output, so the two are separate.