css: avoid overwriting manual fallback colors

evanw · Dec 8, 2023 · b837f21 · b837f21
1 parent 824ede6
commit b837f21
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 7 deletions.
diff --git a/internal/css_parser/css_decls.go b/internal/css_parser/css_decls.go
@@ -127,12 +127,10 @@ func (p *parser) processDeclarations(rules []css_ast.Rule, composesContext *comp
 			continue
 		}
 
-		// If the previous loop iteration would have clipped a color, we duplicate
-		// it and insert the clipped copy before the unclipped copy. But we only do
-		// this if there was no previous instance of that property so we avoid
-		// overwriting any manually-specified fallback values.
+		// If the previous loop iteration would have clipped a color, we will
+		// duplicate it and insert the clipped copy before the unclipped copy
 		var wouldClipColor *bool
-		if wouldClipColorFlag && p.options.unsupportedCSSFeatures.Has(compat.ColorFunction) {
+		if wouldClipColorFlag {
 			wouldClipColorFlag = false
 			clone := *decl
 			clone.Value = css_ast.CloneTokensWithoutImportRecords(clone.Value)
@@ -372,8 +370,28 @@ func (p *parser) processDeclarations(rules []css_ast.Rule, composesContext *comp
 			}
 		}
 
-		if wouldClipColorFlag && p.options.unsupportedCSSFeatures.Has(compat.ColorFunction) {
-			i -= 1
+		// If this loop iteration would have clipped a color, the out-of-gamut
+		// colors will not be clipped and this flag will be set. We then set up the
+		// next iteration of the loop to duplicate this rule and process it again
+		// with color clipping enabled.
+		if wouldClipColorFlag {
+			if p.options.unsupportedCSSFeatures.Has(compat.ColorFunction) {
+				// Only do this if there was no previous instance of that property so
+				// we avoid overwriting any manually-specified fallback values
+				for j := len(rewrittenRules) - 2; j >= 0; j-- {
+					if prev, ok := rewrittenRules[j].Data.(*css_ast.RDeclaration); ok && prev.Key == decl.Key {
+						wouldClipColorFlag = false
+						break
+					}
+				}
+				if wouldClipColorFlag {
+					// If the code above would have clipped a color outside of the sRGB gamut,
+					// process this rule again so we can generate the clipped version next time
+					i -= 1
+					continue
+				}
+			}
+			wouldClipColorFlag = false
 		}
 	}
 

diff --git a/internal/css_parser/css_parser_test.go b/internal/css_parser/css_parser_test.go
@@ -556,6 +556,12 @@ func TestColorFunction(t *testing.T) {
 	expectPrintedLowerMangle(t, "a { before: 0; box-shadow: 1px color(display-p3 1 0 0 / 0.5); after: 1 }",
 		"a {\n  before: 0;\n  box-shadow: 1px rgba(255, 15, 14, .5);\n  box-shadow: 1px color(display-p3 1 0 0 / .5);\n  after: 1;\n}\n", "")
 
+	// Don't insert a fallback after a previous instance of the same property
+	expectPrintedLower(t, "a { color: red; color: color(display-p3 1 0 0) }",
+		"a {\n  color: red;\n  color: color(display-p3 1 0 0);\n}\n", "")
+	expectPrintedLower(t, "a { color: color(display-p3 1 0 0); color: color(display-p3 0 1 0) }",
+		"a {\n  color: #ff0f0e;\n  color: color(display-p3 1 0 0);\n  color: color(display-p3 0 1 0);\n}\n", "")
+
 	// Check case sensitivity
 	expectPrintedLower(t, "a { color: color(srgb 0.87 0.98 0.807) }", "a {\n  color: #deface;\n}\n", "")
 	expectPrintedLower(t, "A { Color: Color(Srgb 0.87 0.98 0.807) }", "A {\n  Color: #deface;\n}\n", "")