Changing color scheme does not update gradients with system colors or…

… `light-dark()` https://bugs.webkit.org/show_bug.cgi?id=267790 rdar://121285450 Reviewed by Matthieu Dubet. `CSSGradientValue`s cache their generated `StyleImage` when possible. Currently, caching is disallowed when the gradient contains one of the following color keywords as a color stop: "-internal-document-text", "-webkit-link", "-webkit-active-link", and "currentcolor". However, that denylist is not comprehensive with respect to dynamic colors. Notably, it excludes almost all system color keywords, and `light-dark()`. Consequently, the same cached `StyleImage` is used even when the used color scheme of an element changes. Fix by updating `BuilderState::isColorFromPrimitiveValueDerivedFromElement` to account for other scenarios where colors change dynamically. * LayoutTests/imported/w3c/web-platform-tests/css/css-images/gradient/color-scheme-dependent-color-stops-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-images/gradient/color-scheme-dependent-color-stops-ref.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-images/gradient/color-scheme-dependent-color-stops.html: Added. * Source/WebCore/css/StyleColor.cpp: (WebCore::StyleColor::colorFromKeyword): Drive-by style fix. (WebCore::StyleColor::containsColorSchemeDependentColor): System colors, `light-dark()`, and any other unresolved colors containing either of those, are color-scheme dependent colors. * Source/WebCore/css/StyleColor.h: * Source/WebCore/css/color/CSSUnresolvedColor.cpp: (WebCore::CSSUnresolvedColor::containsColorSchemeDependentColor const): * Source/WebCore/css/color/CSSUnresolvedColor.h: * Source/WebCore/style/StyleBuilderState.cpp: (WebCore::Style::BuilderState::isColorFromPrimitiveValueDerivedFromElement): Update to cover colors that are color-scheme dependent. Canonical link: https://commits.webkit.org/275645@main
WebKit · Mar 4, 2024 · 5596316 · 5596316
1 parent 052dae3
commit 5596316
Show file tree

Hide file tree

Showing 8 changed files with 183 additions and 10 deletions.
diff --git a/...b-platform-tests/css/css-images/gradient/color-scheme-dependent-color-stops-expected.html b/...b-platform-tests/css/css-images/gradient/color-scheme-dependent-color-stops-expected.html
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="author" title="Aditya Keerthi" href="https://github.com/pxlcoder">
+<link rel="help" href="https://www.w3.org/TR/css-color-adjust-1/#color-scheme-prop">
+<link rel="help" href="https://www.w3.org/TR/css-color-4/#css-system-colors">
+<link rel="help" href="https://www.w3.org/TR/css-color-5/#light-dark">
+<link rel="help" href="https://www.w3.org/TR/css-color-5/#color-mix">
+<title>Reference: Test changing used color-scheme updates gradient with color-scheme dependent color stops.</title>
+<style>
+
+.box {
+    color-scheme: dark;
+
+    width: 100px;
+    height: 100px;
+}
+
+#system-color {
+    background-image: linear-gradient(CanvasText, CanvasText);
+}
+
+#system-color-in-color-mix {
+    background-image: linear-gradient(color-mix(in lch, Canvas, pink), color-mix(in lch, Canvas, pink));
+}
+
+#light-dark {
+    background-image: linear-gradient(light-dark(red, green), light-dark(red, green));
+}
+
+#light-dark-in-color-mix {
+    background-image: linear-gradient(color-mix(in lch, light-dark(red, green), pink), color-mix(in lch, light-dark(red, green), pink));
+}
+
+</style>
+</head>
+<body>
+<p>Test system color</p>
+<div id="system-color" class="box"></div>
+<p>Test system color in color-mix()</p>
+<div id="system-color-in-color-mix" class="box"></div>
+<p>Test light-dark()</p>
+<div id="light-dark" class="box"></div>
+<p>Test light-dark() in color-mix()</p>
+<div id="light-dark-in-color-mix" class="box"></div>
+</body>
+</html>
diff --git a/...3c/web-platform-tests/css/css-images/gradient/color-scheme-dependent-color-stops-ref.html b/...3c/web-platform-tests/css/css-images/gradient/color-scheme-dependent-color-stops-ref.html
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<html>
+<head>
+<link rel="author" title="Aditya Keerthi" href="https://github.com/pxlcoder">
+<link rel="help" href="https://www.w3.org/TR/css-color-adjust-1/#color-scheme-prop">
+<link rel="help" href="https://www.w3.org/TR/css-color-4/#css-system-colors">
+<link rel="help" href="https://www.w3.org/TR/css-color-5/#light-dark">
+<link rel="help" href="https://www.w3.org/TR/css-color-5/#color-mix">
+<title>Reference: Test changing used color-scheme updates gradient with color-scheme dependent color stops.</title>
+<style>
+
+.box {
+    color-scheme: dark;
+
+    width: 100px;
+    height: 100px;
+}
+
+#system-color {
+    background-image: linear-gradient(CanvasText, CanvasText);
+}
+
+#system-color-in-color-mix {
+    background-image: linear-gradient(color-mix(in lch, Canvas, pink), color-mix(in lch, Canvas, pink));
+}
+
+#light-dark {
+    background-image: linear-gradient(light-dark(red, green), light-dark(red, green));
+}
+
+#light-dark-in-color-mix {
+    background-image: linear-gradient(color-mix(in lch, light-dark(red, green), pink), color-mix(in lch, light-dark(red, green), pink));
+}
+
+</style>
+</head>
+<body>
+<p>Test system color</p>
+<div id="system-color" class="box"></div>
+<p>Test system color in color-mix()</p>
+<div id="system-color-in-color-mix" class="box"></div>
+<p>Test light-dark()</p>
+<div id="light-dark" class="box"></div>
+<p>Test light-dark() in color-mix()</p>
+<div id="light-dark-in-color-mix" class="box"></div>
+</body>
+</html>
diff --git a/...ed/w3c/web-platform-tests/css/css-images/gradient/color-scheme-dependent-color-stops.html b/...ed/w3c/web-platform-tests/css/css-images/gradient/color-scheme-dependent-color-stops.html
@@ -0,0 +1,61 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+<head>
+<link rel="author" title="Aditya Keerthi" href="https://github.com/pxlcoder">
+<link rel="help" href="https://www.w3.org/TR/css-color-adjust-1/#color-scheme-prop">
+<link rel="help" href="https://www.w3.org/TR/css-color-4/#css-system-colors">
+<link rel="help" href="https://www.w3.org/TR/css-color-5/#light-dark">
+<link rel="help" href="https://www.w3.org/TR/css-color-5/#color-mix">
+<title>Test changing used color-scheme updates gradient with color-scheme dependent color stops.</title>
+<link rel="match" href="color-scheme-dependent-color-stops-ref.html">
+<style>
+
+.dark {
+    color-scheme: dark;
+}
+
+.box {
+    width: 100px;
+    height: 100px;
+}
+
+#system-color {
+    background-image: linear-gradient(CanvasText, CanvasText);
+}
+
+#system-color-in-color-mix {
+    background-image: linear-gradient(color-mix(in lch, Canvas, pink), color-mix(in lch, Canvas, pink));
+}
+
+#light-dark {
+    background-image: linear-gradient(light-dark(red, green), light-dark(red, green));
+}
+
+#light-dark-in-color-mix {
+    background-image: linear-gradient(color-mix(in lch, light-dark(red, green), pink), color-mix(in lch, light-dark(red, green), pink));
+}
+
+</style>
+</head>
+<body>
+<p>Test system color</p>
+<div id="system-color" class="box"></div>
+<p>Test system color in color-mix()</p>
+<div id="system-color-in-color-mix" class="box"></div>
+<p>Test light-dark()</p>
+<div id="light-dark" class="box"></div>
+<p>Test light-dark() in color-mix()</p>
+<div id="light-dark-in-color-mix" class="box"></div>
+<script>
+
+setTimeout(() => {
+    document.querySelectorAll(".box").forEach((box) => {
+        box.classList.add("dark");
+    });
+
+    document.documentElement.className = '';
+}, 0);
+
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/css/StyleColor.cpp b/Source/WebCore/css/StyleColor.cpp
@@ -75,7 +75,7 @@ Color StyleColor::colorFromAbsoluteKeyword(CSSValueID keyword)
     return { };
 }
 
-Color StyleColor::colorFromKeyword(CSSValueID keyword , OptionSet<StyleColorOptions> options)
+Color StyleColor::colorFromKeyword(CSSValueID keyword, OptionSet<StyleColorOptions> options)
 {
     if (isAbsoluteColorKeyword(keyword))
         return colorFromAbsoluteKeyword(keyword);
@@ -132,6 +132,17 @@ bool StyleColor::containsCurrentColor(const CSSPrimitiveValue& value)
     return false;
 }
 
+bool StyleColor::containsColorSchemeDependentColor(const CSSPrimitiveValue& value)
+{
+    if (StyleColor::isSystemColorKeyword(value.valueID()))
+        return true;
+
+    if (value.isUnresolvedColor())
+        return value.unresolvedColor().containsColorSchemeDependentColor();
+
+    return false;
+}
+
 String StyleColor::debugDescription() const
 {
     TextStream ts;

diff --git a/Source/WebCore/css/StyleColor.h b/Source/WebCore/css/StyleColor.h
@@ -105,6 +105,8 @@ class StyleColor {
     WEBCORE_EXPORT static bool isSystemColorKeyword(CSSValueID);
     static bool isDeprecatedSystemColorKeyword(CSSValueID);
 
+    static bool containsColorSchemeDependentColor(const CSSPrimitiveValue&);
+
     enum class CSSColorType : uint8_t {
         Absolute = 1 << 0,
         Current = 1 << 1,

diff --git a/Source/WebCore/css/color/CSSUnresolvedColor.cpp b/Source/WebCore/css/color/CSSUnresolvedColor.cpp
@@ -45,6 +45,18 @@ bool CSSUnresolvedColor::containsCurrentColor() const
     );
 }
 
+bool CSSUnresolvedColor::containsColorSchemeDependentColor() const
+{
+    return WTF::switchOn(m_value,
+        [] (const CSSUnresolvedColorMix& unresolved) {
+            return StyleColor::containsColorSchemeDependentColor(unresolved.mixComponents1.color) || StyleColor::containsColorSchemeDependentColor(unresolved.mixComponents2.color);
+        },
+        [] (const CSSUnresolvedLightDark&) {
+            return true;
+        }
+    );
+}
+
 void CSSUnresolvedColor::serializationForCSS(StringBuilder& builder) const
 {
     return WTF::switchOn(m_value, [&] (auto& unresolved) { WebCore::serializationForCSS(builder, unresolved); });

diff --git a/Source/WebCore/css/color/CSSUnresolvedColor.h b/Source/WebCore/css/color/CSSUnresolvedColor.h
@@ -50,6 +50,7 @@ class CSSUnresolvedColor {
     ~CSSUnresolvedColor();
 
     bool containsCurrentColor() const;
+    bool containsColorSchemeDependentColor() const;
 
     void serializationForCSS(StringBuilder&) const;
     String serializationForCSS() const;

diff --git a/Source/WebCore/style/StyleBuilderState.cpp b/Source/WebCore/style/StyleBuilderState.cpp
@@ -142,15 +142,7 @@ std::optional<FilterOperations> BuilderState::createFilterOperations(const CSSVa
 
 bool BuilderState::isColorFromPrimitiveValueDerivedFromElement(const CSSPrimitiveValue& value)
 {
-    switch (value.valueID()) {
-    case CSSValueInternalDocumentTextColor:
-    case CSSValueWebkitLink:
-    case CSSValueWebkitActivelink:
-    case CSSValueCurrentcolor:
-        return true;
-    default:
-        return false;
-    }
+    return StyleColor::containsCurrentColor(value) || StyleColor::containsColorSchemeDependentColor(value);
 }
 
 StyleColor BuilderState::colorFromPrimitiveValue(const CSSPrimitiveValue& value, ForVisitedLink forVisitedLink) const