[web-animations] keyframes should be recomputed when a parent element…

… changes value for a custom property set to "inherit" https://bugs.webkit.org/show_bug.cgi?id=251596 Reviewed by Antti Koivisto. We keep a set of CSS properties set to "inherit" 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: (WebCore::KeyframeEffect::inheritedProperties 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/259812@main
WebKit · Feb 3, 2023 · a2b572c · a2b572c
1 parent 148e18a
commit a2b572c
Show file tree

Hide file tree

Showing 7 changed files with 66 additions and 4 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,6 +13,8 @@ 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 "inherit" for a custom property on a keyframe is responsive to changing that custom property on the parent.
+PASS JS-originated animation setting "inherit" for a custom property on a keyframe is responsive to changing that custom property on the parent.
 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,6 +338,61 @@
   });
 }, 'Registered properties referencing animated properties update correctly.');
 
+test_with_at_property({
+  syntax: '"<length>"',
+  inherits: false,
+  initialValue: '0px'
+}, (name) => {
+  with_style_node(`
+    @keyframes test {
+      from { ${name}: inherit; }
+      to { ${name}: 300px; }
+    }
+    #outer {
+      ${name}: 100px;
+    }
+    #div {
+      animation: test 100s -50s linear paused;
+    }
+  `, () => {
+    assert_equals(getComputedStyle(div).getPropertyValue(name), '200px');
+
+    outer.style.setProperty(name, '200px');
+    assert_equals(getComputedStyle(div).getPropertyValue(name), '250px');
+
+    outer.style.setProperty(name, '0px');
+    assert_equals(getComputedStyle(div).getPropertyValue(name), '150px');
+
+    outer.style.removeProperty(name);
+  });
+}, 'CSS animation setting "inherit" for a custom property on a keyframe is responsive to changing that custom property on the parent.');
+
+test_with_at_property({
+  syntax: '"<length>"',
+  inherits: false,
+  initialValue: '0px'
+}, (name) => {
+  with_style_node(`
+    #outer {
+      ${name}: 100px;
+    }
+  `, () => {
+    const animation = div.animate({ [name]: ["inherit", "300px"]}, 1000);
+    animation.currentTime = 500;
+    animation.pause();
+
+    assert_equals(getComputedStyle(div).getPropertyValue(name), '200px');
+
+    outer.style.setProperty(name, '200px');
+    assert_equals(getComputedStyle(div).getPropertyValue(name), '250px');
+
+    outer.style.setProperty(name, '0px');
+    assert_equals(getComputedStyle(div).getPropertyValue(name), '150px');
+
+    outer.style.removeProperty(name);
+  });
+}, 'JS-originated animation setting "inherit" for a custom property on a keyframe is responsive to changing that custom property on the parent.');
+
 test_with_at_property({
   syntax: '"<color>"',
   inherits: false,

diff --git a/Source/WebCore/animation/KeyframeEffect.cpp b/Source/WebCore/animation/KeyframeEffect.cpp
@@ -934,6 +934,8 @@ void KeyframeEffect::updateBlendingKeyframes(RenderStyle& elementStyle, const St
                 } else if (auto* customPropertyValue = dynamicDowncast<CSSCustomPropertyValue>(cssValue)) {
                     if (customPropertyValue->isCurrentColor())
                         m_currentColorProperties.add(customPropertyValue->name());
+                    else if (customPropertyValue->isInherit())
+                        m_inheritedProperties.add(customPropertyValue->name());
                 }
             }
         }

diff --git a/Source/WebCore/animation/KeyframeEffect.h b/Source/WebCore/animation/KeyframeEffect.h
@@ -150,7 +150,7 @@ class KeyframeEffect : public AnimationEffect, public CSSPropertyBlendingClient
     void computeDeclarativeAnimationBlendingKeyframes(const RenderStyle* oldStyle, const RenderStyle& newStyle, const Style::ResolutionContext&);
     const KeyframeList& blendingKeyframes() const { return m_blendingKeyframes; }
     const HashSet<AnimatableProperty>& animatedProperties();
-    const HashSet<CSSPropertyID>& inheritedProperties() const { return m_inheritedProperties; }
+    const HashSet<AnimatableProperty>& inheritedProperties() const { return m_inheritedProperties; }
     bool animatesProperty(AnimatableProperty) const;
     bool animatesDirectionAwareProperty() const;
 
@@ -246,7 +246,7 @@ class KeyframeEffect : public AnimationEffect, public CSSPropertyBlendingClient
     AtomString m_keyframesName;
     KeyframeList m_blendingKeyframes { emptyAtom() };
     HashSet<AnimatableProperty> m_animatedProperties;
-    HashSet<CSSPropertyID> m_inheritedProperties;
+    HashSet<AnimatableProperty> m_inheritedProperties;
     HashSet<AnimatableProperty> m_currentColorProperties;
     Vector<ParsedKeyframe> m_parsedKeyframes;
     Vector<AcceleratedAction> m_pendingAcceleratedActions;

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 isInherit() const { return std::holds_alternative<CSSValueID>(m_value) && std::get<CSSValueID>(m_value) == CSSValueInherit; }
     bool isCurrentColor() const;
     bool containsCSSWideKeyword() const;
 

diff --git a/Source/WebCore/style/StyleResolver.cpp b/Source/WebCore/style/StyleResolver.cpp
@@ -435,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<AnimatableProperty>& currentColorProperties)
+void Resolver::keyframeStylesForAnimation(const Element& element, const RenderStyle& elementStyle, const ResolutionContext& context, KeyframeList& list, bool& containsCSSVariableReferences, bool& hasRelativeFontWeight, HashSet<AnimatableProperty>& inheritedProperties, HashSet<AnimatableProperty>& currentColorProperties)
 {
     inheritedProperties.clear();
     currentColorProperties.clear();
@@ -476,6 +476,8 @@ void Resolver::keyframeStylesForAnimation(const Element& element, const RenderSt
                     } else if (auto* customPropertyValue = dynamicDowncast<CSSCustomPropertyValue>(cssValue)) {
                         if (customPropertyValue->isCurrentColor())
                             currentColorProperties.add(customPropertyValue->name());
+                        else if (customPropertyValue->isInherit())
+                            inheritedProperties.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<AnimatableProperty>& currentColorProperties);
+    void keyframeStylesForAnimation(const Element&, const RenderStyle& elementStyle, const ResolutionContext&, KeyframeList&, bool& containsCSSVariableReferences, bool& hasRelativeFontWeight, HashSet<AnimatableProperty>& inheritedProperties, HashSet<AnimatableProperty>& currentColorProperties);
 
     WEBCORE_EXPORT std::optional<ResolvedStyle> styleForPseudoElement(const Element&, const PseudoElementRequest&, const ResolutionContext&);