diff --git a/src/main/java/com/yahoo/platform/yui/compressor/CssCompressor.java b/src/main/java/com/yahoo/platform/yui/compressor/CssCompressor.java index dfe9437..a93c658 100644 --- a/src/main/java/com/yahoo/platform/yui/compressor/CssCompressor.java +++ b/src/main/java/com/yahoo/platform/yui/compressor/CssCompressor.java @@ -17,9 +17,48 @@ import java.util.regex.Pattern; import java.util.regex.Matcher; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; public class CssCompressor { + /** + * Functions whose grammar rejects a bare {@code 0} where a + * {@code } is written, so the "a zero value may drop its unit" + * rule must not fire inside them. + * + *

The colour functions take {@code } arguments in their + * comma-separated legacy form ({@code hsl(27,0%,50%)}), and the math + * functions type-check their arguments against each other, so + * {@code min(0,10px)} compares a {@code } with a {@code }. + * In every one of those cases a browser drops the whole declaration, which + * costs the declaration to save one byte. + * + *

Matching is on the whole function name, so {@code minmax()} - where a + * zero really may lose its unit - is not caught by the {@code min} entry. + */ + private static final Set PERCENTAGE_REQUIRED_FUNCTIONS = Collections.unmodifiableSet( + new HashSet(Arrays.asList("hsl", "hsla", "rgb", "rgba", "color-mix", "min", "max", "clamp"))); + + /** + * Properties whose value must not be collapsed from a run of zeroes to a + * single {@code 0}. + * + *

{@code margin:0 0 0 0} is the box-model shorthand, where the collapse is + * exact. {@code box-shadow} and {@code text-shadow} are not shorthands at all: + * a {@code } needs both of its offsets, so {@code box-shadow:0} is + * invalid and the browser drops the declaration - and with it every other + * shadow in the same comma-separated list. {@code perspective-origin:0} means + * {@code 0 center}, not {@code 0 0}. {@code flex} was already excluded here + * for the same class of reason. + * + *

Names are matched with their vendor prefix removed. + */ + private static final Set ZERO_RUN_NOT_COLLAPSIBLE = Collections.unmodifiableSet( + new HashSet(Arrays.asList("box-shadow", "text-shadow", "perspective-origin", "flex"))); + private StringBuffer srcsb = new StringBuffer(); public CssCompressor(Reader in) throws IOException { @@ -1100,21 +1139,52 @@ public void compress(Writer out, int linebreakpos) css = m.replaceAll("$1to{"); } while (!(css.equals(oldCss))); - // Replace 0(px,em,%) with 0 inside groups (e.g. -MOZ-RADIAL-GRADIENT(CENTER 45DEG, CIRCLE CLOSEST-SIDE, ORANGE 0%, RED 100%)) - p = Pattern.compile("(?i)\\( ?((?:[0-9a-z-.]+[ ,])*)?(?:0?\\.)?0(?:px|em|%|in|cm|mm|pc|pt|ex|deg|g?rad|m?s|k?hz)"); + // Replace 0(px,em) with 0 inside groups (e.g. -MOZ-RADIAL-GRADIENT(CENTER 45DEG, CIRCLE CLOSEST-SIDE, ORANGE 0PX, RED 100%)) + p = Pattern.compile("(?i)\\( ?((?:[0-9a-z-.]+[ ,])*)?(?:0?\\.)?0(?:px|em|in|cm|mm|pc|pt|ex|deg|g?rad|m?s|k?hz)"); do { oldCss = css; m = p.matcher(css); css = m.replaceAll("($10"); } while (!(css.equals(oldCss))); + // The same for "%", except inside a function that requires a real + // there - see PERCENTAGE_REQUIRED_FUNCTIONS. The function + // name has to be captured rather than looked behind, so that "min" does + // not also match the tail of "minmax". + p = Pattern.compile("(?i)([-a-z0-9_]*)\\( ?((?:[0-9a-z-.]+[ ,])*)?(?:0?\\.)?0%"); + do { + oldCss = css; + m = p.matcher(css); + sb = new StringBuffer(); + while (m.find()) { + String function = m.group(1).toLowerCase(); + String replacement = PERCENTAGE_REQUIRED_FUNCTIONS.contains(function) + ? m.group(0) + : m.group(1) + "(" + (m.group(2) == null ? "" : m.group(2)) + "0"; + m.appendReplacement(sb, Matcher.quoteReplacement(replacement)); + } + m.appendTail(sb); + css = sb.toString(); + } while (!(css.equals(oldCss))); + // Replace x.0(px,em,%) with x(px,em,%). css = css.replaceAll("([0-9])\\.0(px|em|%|in|cm|mm|pc|pt|ex|deg|m?s|g?rad|k?hz| |;)", "$1$2"); - // Replace 0 0 0 0; with 0. - css = css.replaceAll(":0 0 0 0(;|})", ":0$1"); - css = css.replaceAll(":0 0 0(;|})", ":0$1"); - css = css.replaceAll("(?hsl(), hsla(), rgb(), rgba() and color-mix() take {@code } + * arguments in their comma-separated legacy form; a bare "0" there is not a value + * any browser accepts, so the declaration is dropped and the colour silently + * disappears from the page. + */ +class CssColorFunctionTest { + + private String compress(String source) throws Exception { + StringWriter out = new StringWriter(); + new CssCompressor(new StringReader(source)).compress(out, -1); + return out.toString().trim(); + } + + @Test + void hslaKeepsZeroPercentSaturation() throws Exception { + assertEquals("a{color:hsla(0,0%,100%,0.5)}", compress("a{color:hsla(0,0%,100%,0.5)}")); + } + + @Test + void hslKeepsZeroPercentSaturation() throws Exception { + assertEquals("a{color:hsl(0,0%,100%)}", compress("a{color:hsl(0,0%,100%)}")); + } + + @Test + void hslaKeepsBothZeroPercentArguments() throws Exception { + assertEquals("a{color:hsla(120,0%,0%,1)}", compress("a{color:hsla(120,0%,0%,1)}")); + } + + @Test + void rgbKeepsZeroPercentChannel() throws Exception { + assertEquals("a{color:rgb(0%,50%,0%)}", compress("a{color:rgb(0%,50%,0%)}")); + } + + @Test + void rgbaKeepsZeroPercentChannel() throws Exception { + assertEquals("a{color:rgba(0%,50%,50%,0.4)}", compress("a{color:rgba(0%,50%,50%,0.4)}")); + } + + @Test + void colorMixKeepsZeroPercent() throws Exception { + assertEquals("a{color:color-mix(in srgb,red 0%,blue)}", + compress("a{color:color-mix(in srgb, red 0%, blue)}")); + } + + /** + * A nested colour function keeps its percentages. The stop that follows it + * keeps its own "%" too - the argument-prefix part of the pattern cannot + * step over the nested parentheses to reach it - which is a missed byte, + * not a correctness problem. + */ + @Test + void nestedColorFunctionKeepsItsPercentages() throws Exception { + assertEquals("a{background:linear-gradient(hsla(0,0%,50%,1) 0%,red 100%)}", + compress("a{background:linear-gradient(hsla(0,0%,50%,1) 0%, red 100%)}")); + } + + @Test + void gradientColorStopStillLosesItsPercent() throws Exception { + assertEquals("a{background:linear-gradient(red 0,blue 100%)}", + compress("a{background:linear-gradient(red 0%, blue 100%)}")); + } + + @Test + void minKeepsZeroPercentSoItsArgumentsStayTypeCompatible() throws Exception { + assertEquals("a{width:min(0%,10px)}", compress("a{width:min(0%, 10px)}")); + } + + @Test + void maxKeepsZeroPercent() throws Exception { + assertEquals("a{width:max(0%,10px)}", compress("a{width:max(0%, 10px)}")); + } + + @Test + void clampKeepsZeroPercent() throws Exception { + assertEquals("a{width:clamp(0%,50%,100%)}", compress("a{width:clamp(0%, 50%, 100%)}")); + } + + /** "minmax" must not be caught by the "min" entry: a zero there may lose its unit. */ + @Test + void nonColorFunctionStillLosesItsPercent() throws Exception { + assertEquals("a{grid-template-columns:minmax(0,1fr)}", + compress("a{grid-template-columns:minmax(0%,1fr)}")); + } + + @Test + void lengthUnitsInsideGroupsAreStillStripped() throws Exception { + assertEquals("a{transform:translate(0,10px)}", compress("a{transform:translate(0px,10px)}")); + } + + // ------------------------------------------------------------------ + // A run of zeroes may only collapse to one where the property is a + // box-model shorthand. + // ------------------------------------------------------------------ + + @Test + void boxShadowKeepsBothOffsets() throws Exception { + assertEquals("a{box-shadow:0 0}", compress("a{box-shadow:0 0}")); + } + + @Test + void boxShadowWithSpreadIsNotCollapsed() throws Exception { + assertEquals("a{box-shadow:0 0 0 0}", compress("a{box-shadow:0 0 0 0}")); + } + + @Test + void vendorPrefixedBoxShadowIsNotCollapsed() throws Exception { + assertEquals("a{-webkit-box-shadow:0 0 0 0}", compress("a{-webkit-box-shadow:0 0 0 0}")); + } + + @Test + void textShadowKeepsBothOffsets() throws Exception { + assertEquals("a{text-shadow:0 0 0}", compress("a{text-shadow:0 0 0}")); + } + + /** "perspective-origin:0" means "0 center", which is not "0 0". */ + @Test + void perspectiveOriginKeepsBothAxes() throws Exception { + assertEquals("a{perspective-origin:0 0}", compress("a{perspective-origin:0 0}")); + } + + @Test + void flexIsStillNotCollapsed() throws Exception { + assertEquals("a{flex:0 0}", compress("a{flex:0 0}")); + } + + @Test + void boxModelShorthandsStillCollapse() throws Exception { + assertEquals("a{margin:0}", compress("a{margin:0 0 0 0}")); + assertEquals("a{padding:0}", compress("a{padding:0 0}")); + assertEquals("a{border-radius:0}", compress("a{border-radius:0 0}")); + assertEquals("a{gap:0}", compress("a{gap:0 0}")); + } + + @Test + void backgroundPositionIsStillNormalisedToTwoAxes() throws Exception { + assertEquals("a{background-position:0 0}", compress("a{background-position:0 0}")); + assertEquals("a{transform-origin:0 0}", compress("a{transform-origin:0 0}")); + } +} diff --git a/src/test/java/org/codelibs/yuicompressor/KnownCssLimitationsTest.java b/src/test/java/org/codelibs/yuicompressor/KnownCssLimitationsTest.java index ba1d59d..0f5abe8 100644 --- a/src/test/java/org/codelibs/yuicompressor/KnownCssLimitationsTest.java +++ b/src/test/java/org/codelibs/yuicompressor/KnownCssLimitationsTest.java @@ -35,63 +35,13 @@ private static String css(String source) throws IOException { } // ------------------------------------------------------------------ - // A. Zero-percentage stripping produces invalid CSS. + // A. Zero-value unit stripping inside a group. // - // CssCompressor.java (identical on main): - // Pattern.compile("(?i)\\( ?((?:[0-9a-z-.]+[ ,])*)?(?:0?\\.)?0(?:px|em|%|in|cm|mm|pc|pt|ex|deg|g?rad|m?s|k?hz)") - // - // The rule is "a zero may drop its unit", which is true, and it is - // applied to "%" as well - but a is a distinct type, not a - // length, and several grammars accept ONLY a percentage in the position - // this rewrites. The rule fires on anything inside a "(", so it reaches - // colour functions and math functions it was never meant for. + // The half of this family is fixed: CssCompressor now skips + // the functions whose grammar requires a real percentage, and + // CssColorFunctionTest pins that. What is left is the