[web-animations] keyframes should be recomputed when the "currentcolo…

…r" value is used on a custom property https://bugs.webkit.org/show_bug.cgi?id=251591 Reviewed by Antti Koivisto. We keep a set of CSS properties set to "currentcolor" in a KeyframeEffect but until now only considered "standard" CSS properties. We now also consider custom properties by changing the set's type from HashSet<CSSPropertyID> to HashSet<AnimatableProperty>. * LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/at-property-animation-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/at-property-animation.html: * Source/WebCore/animation/KeyframeEffect.cpp: (WebCore::KeyframeEffect::updateBlendingKeyframes): * Source/WebCore/animation/KeyframeEffect.h: * Source/WebCore/css/CSSCustomPropertyValue.cpp: (WebCore::CSSCustomPropertyValue::isCurrentColor const): * Source/WebCore/css/CSSCustomPropertyValue.h: * Source/WebCore/style/StyleResolver.cpp: (WebCore::Style::Resolver::keyframeStylesForAnimation): * Source/WebCore/style/StyleResolver.h: Canonical link: https://commits.webkit.org/259808@main
WebKit · Feb 3, 2023 · b1a5811 · b1a5811
1 parent ffe6d89
commit b1a5811
Show file tree

Hide file tree

Showing 8 changed files with 77 additions and 3 deletions.
diff --git a/...d/w3c/web-platform-tests/css/css-properties-values-api/at-property-animation-expected.txt b/...d/w3c/web-platform-tests/css/css-properties-values-api/at-property-animation-expected.txt
@@ -13,4 +13,6 @@ PASS No transition when changing types
 PASS No transition when removing @property rule
 PASS Unregistered properties referencing animated properties update correctly.
 PASS Registered properties referencing animated properties update correctly.
+PASS CSS animation setting "currentColor" for a custom property on a keyframe is responsive to changing "color" on the parent.
+PASS JS-originated animation setting "currentColor" for a custom property on a keyframe is responsive to changing "color" on the parent.
 
diff --git a/.../imported/w3c/web-platform-tests/css/css-properties-values-api/at-property-animation.html b/.../imported/w3c/web-platform-tests/css/css-properties-values-api/at-property-animation.html
@@ -338,4 +338,59 @@
   });
 }, 'Registered properties referencing animated properties update correctly.');
 
+test_with_at_property({
+  syntax: '"<color>"',
+  inherits: false,
+  initialValue: 'black'
+}, (name) => {
+  with_style_node(`
+    @keyframes test {
+      from { ${name}: currentcolor; }
+      to { ${name}: rgb(200, 200, 200); }
+    }
+    #outer {
+      color: rgb(100, 100, 100);
+    }
+    #div {
+      animation: test 100s -50s linear paused;
+    }
+  `, (style) => {
+    assert_equals(getComputedStyle(div).getPropertyValue(name), 'rgb(150, 150, 150)');
+
+    outer.style.color = 'rgb(50, 50, 50)';
+    assert_equals(getComputedStyle(div).getPropertyValue(name), 'rgb(125, 125, 125)');
+
+    outer.style.color = 'rgb(150, 150, 150)';
+    assert_equals(getComputedStyle(div).getPropertyValue(name), 'rgb(175, 175, 175)');
+
+    outer.style.removeProperty("color");
+  });
+}, 'CSS animation setting "currentColor" for a custom property on a keyframe is responsive to changing "color" on the parent.');
+
+test_with_at_property({
+  syntax: '"<color>"',
+  inherits: false,
+  initialValue: 'black'
+}, (name) => {
+  with_style_node(`
+    #outer {
+      color: rgb(100, 100, 100);
+    }
+  `, () => {
+    const animation = div.animate({ [name]: ["currentcolor", "rgb(200, 200, 200)"]}, 1000);
+    animation.currentTime = 500;
+    animation.pause();
+
+    assert_equals(getComputedStyle(div).getPropertyValue(name), 'rgb(150, 150, 150)');
+
+    outer.style.color = 'rgb(50, 50, 50)';
+    assert_equals(getComputedStyle(div).getPropertyValue(name), 'rgb(125, 125, 125)');
+
+    outer.style.color = 'rgb(150, 150, 150)';
+    assert_equals(getComputedStyle(div).getPropertyValue(name), 'rgb(175, 175, 175)');
+
+    outer.style.removeProperty("color");
+  });
+}, 'JS-originated animation setting "currentColor" for a custom property on a keyframe is responsive to changing "color" on the parent.');
+
 </script>
diff --git a/Source/WebCore/animation/KeyframeEffect.cpp b/Source/WebCore/animation/KeyframeEffect.cpp
@@ -28,6 +28,7 @@
 
 #include "Animation.h"
 #include "CSSAnimation.h"
+#include "CSSCustomPropertyValue.h"
 #include "CSSKeyframeRule.h"
 #include "CSSPrimitiveValue.h"
 #include "CSSPropertyAnimation.h"
@@ -930,6 +931,9 @@ void KeyframeEffect::updateBlendingKeyframes(RenderStyle& elementStyle, const St
                         m_currentColorProperties.add(property.id());
                     else if (property.id() == CSSPropertyFontWeight && (valueId == CSSValueBolder || valueId == CSSValueLighter))
                         m_hasRelativeFontWeight = true;
+                } else if (auto* customPropertyValue = dynamicDowncast<CSSCustomPropertyValue>(cssValue)) {
+                    if (customPropertyValue->isCurrentColor())
+                        m_currentColorProperties.add(customPropertyValue->name());
                 }
             }
         }

diff --git a/Source/WebCore/animation/KeyframeEffect.h b/Source/WebCore/animation/KeyframeEffect.h
@@ -247,7 +247,7 @@ class KeyframeEffect : public AnimationEffect, public CSSPropertyBlendingClient
     KeyframeList m_blendingKeyframes { emptyAtom() };
     HashSet<AnimatableProperty> m_animatedProperties;
     HashSet<CSSPropertyID> m_inheritedProperties;
-    HashSet<CSSPropertyID> m_currentColorProperties;
+    HashSet<AnimatableProperty> m_currentColorProperties;
     Vector<ParsedKeyframe> m_parsedKeyframes;
     Vector<AcceleratedAction> m_pendingAcceleratedActions;
     RefPtr<Element> m_target;

diff --git a/Source/WebCore/css/CSSCustomPropertyValue.cpp b/Source/WebCore/css/CSSCustomPropertyValue.cpp
@@ -152,4 +152,12 @@ Ref<const CSSVariableData> CSSCustomPropertyValue::asVariableData() const
     });
 }
 
+bool CSSCustomPropertyValue::isCurrentColor() const
+{
+    // FIXME: it's surprising that setting a custom property to "currentcolor"
+    // results in m_value being a CSSVariableReferenceValue rather than a
+    // CSSValueID set to CSSValueCurrentcolor or a StyleValue.
+    return std::holds_alternative<Ref<CSSVariableReferenceValue>>(m_value) && std::get<Ref<CSSVariableReferenceValue>>(m_value)->customCSSText() == "currentcolor"_s;
+}
+
 }
diff --git a/Source/WebCore/css/CSSCustomPropertyValue.h b/Source/WebCore/css/CSSCustomPropertyValue.h
@@ -107,6 +107,7 @@ class CSSCustomPropertyValue final : public CSSValue {
     bool isResolved() const { return !std::holds_alternative<Ref<CSSVariableReferenceValue>>(m_value); }
     bool isUnset() const { return std::holds_alternative<CSSValueID>(m_value) && std::get<CSSValueID>(m_value) == CSSValueUnset; }
     bool isInvalid() const { return std::holds_alternative<CSSValueID>(m_value) && std::get<CSSValueID>(m_value) == CSSValueInvalid; }
+    bool isCurrentColor() const;
     bool containsCSSWideKeyword() const;
 
     const VariantValue& value() const { return m_value; }

diff --git a/Source/WebCore/style/StyleResolver.cpp b/Source/WebCore/style/StyleResolver.cpp
@@ -30,6 +30,7 @@
 #include "config.h"
 #include "StyleResolver.h"
 
+#include "CSSCustomPropertyValue.h"
 #include "CSSFontSelector.h"
 #include "CSSKeyframeRule.h"
 #include "CSSKeyframesRule.h"
@@ -434,7 +435,7 @@ Vector<Ref<StyleRuleKeyframe>> Resolver::keyframeRulesForName(const AtomString&
     return deduplicatedKeyframes;
 }
 
-void Resolver::keyframeStylesForAnimation(const Element& element, const RenderStyle& elementStyle, const ResolutionContext& context, KeyframeList& list, bool& containsCSSVariableReferences, bool& hasRelativeFontWeight, HashSet<CSSPropertyID>& inheritedProperties, HashSet<CSSPropertyID>& currentColorProperties)
+void Resolver::keyframeStylesForAnimation(const Element& element, const RenderStyle& elementStyle, const ResolutionContext& context, KeyframeList& list, bool& containsCSSVariableReferences, bool& hasRelativeFontWeight, HashSet<CSSPropertyID>& inheritedProperties, HashSet<AnimatableProperty>& currentColorProperties)
 {
     inheritedProperties.clear();
     currentColorProperties.clear();
@@ -472,6 +473,9 @@ void Resolver::keyframeStylesForAnimation(const Element& element, const RenderSt
                             currentColorProperties.add(property.id());
                         else if (property.id() == CSSPropertyFontWeight && (valueId == CSSValueBolder || valueId == CSSValueLighter))
                             hasRelativeFontWeight = true;
+                    } else if (auto* customPropertyValue = dynamicDowncast<CSSCustomPropertyValue>(cssValue)) {
+                        if (customPropertyValue->isCurrentColor())
+                            currentColorProperties.add(customPropertyValue->name());
                     }
                 }
             }

diff --git a/Source/WebCore/style/StyleResolver.h b/Source/WebCore/style/StyleResolver.h
@@ -95,7 +95,7 @@ class Resolver : public RefCounted<Resolver> {
 
     ResolvedStyle styleForElement(const Element&, const ResolutionContext&, RuleMatchingBehavior = RuleMatchingBehavior::MatchAllRules);
 
-    void keyframeStylesForAnimation(const Element&, const RenderStyle& elementStyle, const ResolutionContext&, KeyframeList&, bool& containsCSSVariableReferences, bool& hasRelativeFontWeight, HashSet<CSSPropertyID>& inheritedProperties, HashSet<CSSPropertyID>& currentColorProperties);
+    void keyframeStylesForAnimation(const Element&, const RenderStyle& elementStyle, const ResolutionContext&, KeyframeList&, bool& containsCSSVariableReferences, bool& hasRelativeFontWeight, HashSet<CSSPropertyID>& inheritedProperties, HashSet<AnimatableProperty>& currentColorProperties);
 
     WEBCORE_EXPORT std::optional<ResolvedStyle> styleForPseudoElement(const Element&, const PseudoElementRequest&, const ResolutionContext&);