From 115e4fee30b0d696a9991b3ee906c5dabfcdb68d Mon Sep 17 00:00:00 2001
From: Shinsuke Sugaya <shinsuke@apache.org>
Date: Sat, 5 Sep 2026 09:14:44 +0900
Subject: [PATCH] Stop zero-value minification from producing invalid CSS

Two long-standing rules in CssCompressor rewrite a zero into a form the CSS
grammar does not accept in that position. A browser drops the whole
declaration, so the rule costs the declaration to save a byte, and it does so
silently - the build reports no warning.

Zero-percentage stripping inside a group

  hsla(0,0%,100%,.5)          -> hsla(0,0,100%,.5)
  rgb(0%,50%,100%)            -> rgb(0,50%,100%)
  color-mix(in srgb,red 0%,b) -> color-mix(in srgb,red 0,b)
  min(0%,10px)                -> min(0,10px)

The rule "a zero <length> may drop its unit" is true, but it was applied to
<percentage> as well, and it fired on anything inside a "(". The colour
functions' comma-separated legacy form takes <percentage> arguments, and the
math functions type-check their arguments against each other, so none of the
outputs above is a valid value.

The "%" case now consults the enclosing function name. The name is captured
rather than looked behind, so "min" does not also match the tail of "minmax()",
where a zero really may lose its unit. Length units inside groups, gradient
colour stops and minmax() are unchanged.

Collapsing a run of zeroes

  box-shadow:0 0              -> box-shadow:0
  text-shadow:0 0 0           -> text-shadow:0
  perspective-origin:0 0      -> perspective-origin:0

"margin:0 0 0 0" is the box-model shorthand and collapses exactly, but a
<shadow> needs both of its offsets, so "box-shadow:0" is invalid - and taking
the declaration down takes every other shadow in the same comma-separated list
with it. "perspective-origin:0" means "0 center", not "0 0". The collapse now
matches on the property name (vendor prefix removed) and skips those three;
"flex" keeps the exclusion it already had.

Verification

- 143 real-world stylesheets (Bootstrap, Font Awesome, Swiper, Vuetify and
  Fess/CodeLibs themes) were compressed and every declaration checked against
  the CSS grammar with css-tree's lexer. Declarations valid in the source but
  invalid after compression: 26 files before, 0 after - the 16 files still
  flagged are var() fallbacks css-tree cannot resolve, and are flagged
  identically in the source.
- All 143 stay byte-identical when compressed a second time.
- CssColorFunctionTest pins each corrupted value plus the neighbouring cases
  that must keep compressing: minmax(), gradient stops, translate(0px),
  margin/padding/border-radius/gap, background-position, transform-origin.
- The three percentage entries and the min() entry in KnownCssLimitationsTest
  recorded these defects with instructions to delete them once fixed; they are
  removed. The <time>-inside-a-group case there is untouched and still fails.
---
 .../yui/compressor/CssCompressor.java         |  82 +++++++++-
 .../yuicompressor/CssColorFunctionTest.java   | 154 ++++++++++++++++++
 .../KnownCssLimitationsTest.java              |  58 +------
 3 files changed, 234 insertions(+), 60 deletions(-)
 create mode 100644 src/test/java/org/codelibs/yuicompressor/CssColorFunctionTest.java

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 <percentage>} is written, so the "a zero value may drop its unit"
+     * rule must not fire inside them.
+     *
+     * <p>The colour functions take {@code <percentage>} 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 <number>} with a {@code <length>}.
+     * In every one of those cases a browser drops the whole declaration, which
+     * costs the declaration to save one byte.
+     *
+     * <p>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<String> PERCENTAGE_REQUIRED_FUNCTIONS = Collections.unmodifiableSet(
+            new HashSet<String>(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}.
+     *
+     * <p>{@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 <shadow>} 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.
+     *
+     * <p>Names are matched with their vendor prefix removed.
+     */
+    private static final Set<String> ZERO_RUN_NOT_COLLAPSIBLE = Collections.unmodifiableSet(
+            new HashSet<String>(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
+        // <percentage> 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("(?<!flex):0 0(;|})", ":0$1");
+        // Replace ":0 0 0 0" / ":0 0 0" / ":0 0" with ":0", for the properties where
+        // that is the box-model shorthand rather than a value with a fixed arity -
+        // see ZERO_RUN_NOT_COLLAPSIBLE.
+        p = Pattern.compile("(?i)([-a-z0-9_]+):0(?: 0){1,3}(;|})");
+        m = p.matcher(css);
+        sb = new StringBuffer();
+        while (m.find()) {
+            String property = m.group(1).toLowerCase().replaceFirst("^-(?:webkit|moz|ms|o)-", "");
+            String replacement = ZERO_RUN_NOT_COLLAPSIBLE.contains(property)
+                    ? m.group(0)
+                    : m.group(1) + ":0" + m.group(2);
+            m.appendReplacement(sb, Matcher.quoteReplacement(replacement));
+        }
+        m.appendTail(sb);
+        css = sb.toString();
 
 
         // Replace background-position:0; with background-position:0 0;
diff --git a/src/test/java/org/codelibs/yuicompressor/CssColorFunctionTest.java b/src/test/java/org/codelibs/yuicompressor/CssColorFunctionTest.java
new file mode 100644
index 0000000..c069e36
--- /dev/null
+++ b/src/test/java/org/codelibs/yuicompressor/CssColorFunctionTest.java
@@ -0,0 +1,154 @@
+package org.codelibs.yuicompressor;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.io.StringReader;
+import java.io.StringWriter;
+
+import org.junit.jupiter.api.Test;
+
+import com.yahoo.platform.yui.compressor.CssCompressor;
+
+/**
+ * The zero-value unit stripping ("0px" -&gt; "0") must not touch a "%" that its
+ * colour function requires.
+ *
+ * <p>hsl(), hsla(), rgb(), rgba() and color-mix() take {@code <percentage>}
+ * 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 <length> may drop its unit", which is true, and it is
-    // applied to "%" as well - but a <percentage> 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 <percentage> 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 <time> case below.
     // ------------------------------------------------------------------
 
-    /**
-     * {@code color-mix()} takes {@code <percentage [0,100]>}. A bare {@code 0}
-     * is not a percentage, so the whole declaration is dropped by a browser -
-     * the element loses its colour entirely. The cleanest case of the family:
-     * no legacy/modern grammar ambiguity to argue about.
-     */
-    @Test
-    void colorMixPercentageLosesItsUnit_knownDefect() {
-        assertDoesNotRoundTrip("a{color:color-mix(in srgb, red 0%, blue)}",
-                "a{color:color-mix(in srgb,red 0,blue)}");
-    }
-
-    /**
-     * Legacy comma-separated {@code rgb()} is
-     * {@code rgb(<percentage>#{3})} or {@code rgb(<number>#{3})} - the two may
-     * not be mixed. Stripping one unit produces exactly that mixture.
-     */
-    @Test
-    void rgbPercentageChannelLosesItsUnitAndMixesTypes_knownDefect() {
-        assertDoesNotRoundTrip("a{color:rgb(0%,50%,100%)}", "a{color:rgb(0,50%,100%)}");
-    }
-
-    /**
-     * Legacy comma-separated {@code hsl()}/{@code hsla()} require
-     * {@code <percentage>} for saturation and lightness.
-     */
-    @ParameterizedTest
-    @CsvSource(delimiter = '|', value = {
-            "a{color:hsl(27,0%,50%)}|a{color:hsl(27,0,50%)}",
-            "a{color:hsl(27, 0%, 50%)}|a{color:hsl(27,0,50%)}",
-            "a{color:hsla(27,0%,50%,0.5)}|a{color:hsla(27,0,50%,0.5)}" })
-    void hslSaturationLosesItsUnit_knownDefect(String source, String wrongOutput) throws Exception {
-        assertEquals(wrongOutput, css(source), "if this now keeps the unit, the defect is fixed - delete this row");
-    }
-
-    /**
-     * CSS math functions are type-checked: {@code min()} may not compare a
-     * {@code <number>} with a {@code <length>}. Stripping the "%" turns a legal
-     * comparison into an invalid one.
-     */
-    @Test
-    void aMathFunctionArgumentLosesItsPercentageAndBreaksTypeChecking_knownDefect() {
-        assertDoesNotRoundTrip("a{width:min(0%, 10px)}", "a{width:min(0,10px)}");
-    }
-
     /**
      * The same rule strips {@code <time>} units, but only inside a
      * parenthesised group. {@link ModernCssTest#zeroTimeValuesKeepTheirUnit}