Fix three CSS compressions that break valid stylesheets - #16
Merged
Conversation
A descendant combinator was deleted before a pseudo-class @media screen{p :link{...}} -> @media screen{p:link{...}} .a{& :hover{...}} -> .a{&:hover{...}} "p :link" is links descended from a p; "p:link" is a p that is itself a link. They select disjoint sets of elements, so the rule applies to the wrong ones. The colon-protection pattern was anchored at the start of the file or after a "}", so a selector that is the first thing inside a block had no anchor and the space in front of its pseudo-class was eaten. That covers the first rule in every @media/@supports/@layer/@container - and, with CSS nesting, every nested rule, which is by definition the first thing after a "{" or a ";". A rule preceded by a sibling's "}" was protected, which is why the top-level golden fixtures never showed it. "{" and ";" are now anchors too, and the anchor is a lookbehind rather than part of the match: an at-rule's own prelude ends with the very "{" that opens the block, and consuming it left the first nested rule with no anchor of its own. ";" and "}" are excluded from the runs on either side of the colon so a match cannot start in a declaration block and reach into the next selector - that is what keeps "color:red" out of the protection, where it would only cost bytes. rgb() with CSS Color 4 channels aborted the build a{color:rgb(0 0 0)} -> [ERROR] For input string: "0 0 0" exit 1, no output The character class admitted whitespace, so split(",") yielded one token and Integer.parseInt threw NumberFormatException - unchecked, and not declared by compress(Writer,int) - on input every browser accepts. The pattern now matches only the legacy comma form, which is the one that can be shortened to hex; the modern form is left alone, which costs bytes rather than the build. Channel digits are bounded so a nonsense value cannot overflow parseInt either; rgb(1000,500,300) still clamps to #fff as before. Operator spacing was only repaired inside calc() a{width:min(10px + 5px,5px)} -> a{width:min(10px+5px,5px)} CSS Values 4 requires whitespace on both sides of + and - in a <calc-sum>, which is what every math function takes - so the browser drops the declaration. respaceCalcOperators searched for the literal "calc(" and never saw min, max, clamp, round, mod or rem. It now matches any of them, on the whole function name so that minmax() is not caught by the min entry, and still on a vendor prefix so -webkit-calc() keeps working. Verification - 143 real-world stylesheets: all still compress, all still byte-identical on a second pass, and the set of declarations that the CSS grammar rejects after compression is unchanged from before this branch - no new corruption. - CssStructureTest pins each case together with what must not change: a declaration's own colon still loses its spaces, a pseudo-class with no space stays tight, comma-separated rgb() still shortens, minmax() is left alone.
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.
Three
CssCompressordefects that break valid stylesheets: one changes which elements a rule selects, one aborts the build, one makes the browser drop a declaration.1. A descendant combinator was deleted before a pseudo-class
p :linkis links descended from a p;p:linkis a p that is itself a link. They select disjoint sets of elements — the rule ends up applying to the wrong ones.The colon-protection pattern was anchored at
^or after a}, so a selector that is the first thing inside a block had no anchor and the space in front of its pseudo-class was eaten. That covers the first rule in every@media/@supports/@layer/@container— and, with CSS nesting, every nested rule, which is by definition the first thing after a{or a;. A rule preceded by a sibling's}was protected, which is why the top-level golden fixtures never showed it.{and;are now anchors too, and the anchor is a lookbehind rather than part of the match: an at-rule's own prelude ends with the very{that opens the block, and consuming it left the first nested rule with no anchor of its own.;and}are excluded from the runs on either side of the colon so a match cannot start in a declaration block and reach into the next selector — that is what keepscolor:redout of the protection, where it would only cost bytes.2.
rgb()with CSS Color 4 channels aborted the buildThe character class
[0-9,\s]admitted whitespace, sosplit(",")yielded the single token"0 0 0"andInteger.parseIntthrewNumberFormatException— unchecked, and not declared bycompress(Writer,int)— on input every browser accepts. A Maven build fails on it.The pattern now matches only the legacy comma form, which is the one that can be shortened to hex. The modern form is left alone: that costs bytes, not the build. Channel digits are bounded so a nonsense value cannot overflow
parseInteither;rgb(1000,500,300)still clamps to#fffas the golden expects.3. Operator spacing was only repaired inside
calc()CSS Values 4 requires whitespace on both sides of
+and-in a<calc-sum>, which is what every math function takes — so the browser drops the declaration.respaceCalcOperatorssearched for the literal"calc("and never sawmin,max,clamp,round,modorrem.It now matches any of them, on the whole function name so
minmax()is not caught by theminentry, and still on a vendor prefix so-webkit-calc()keeps working. The nested-function and custom-property hardening from #9 and #11 comes along unchanged.Verification
css-tree's lexer) is identical to before this branch — no new corruption.CssStructureTest(new, 12 cases) pins each defect together with what must not change: a declaration's own colon still loses its spaces, a pseudo-class with no space stays tight, comma-separatedrgb()still shortens,minmax()is left alone,margin: 0 -5pxis untouched.Full suite: 777 tests, 0 failures, 3 skipped.
Independent of #13, #14 and #15; all branch from
main. #13 also touchesCssCompressorbut a different region.