[css-animations] a @Keyframes rule using an "inherit" value does not …

…update the resolved value when the parent style changes https://bugs.webkit.org/show_bug.cgi?id=251433 Reviewed by Antti Koivisto. In the case where a @Keyframes rule has one of its properties set to "inherit", we need to update the computed keyframes in case an ancestor changes in a way that the inherited value changes. We already have a mechanism to deal with a similar scenario when the keyframes are provided using the Web Animations API where the m_inheritedProperties instance variable keeps track of all CSS properties set to "inherit" in keyframes. We now pass m_inheritedProperties to Style::Resolver::keyframeStylesForAnimation() to populate it when a CSS Animation's keyframes are computed. * LayoutTests/imported/w3c/web-platform-tests/css/css-animations/responsive/line-height-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-animations/responsive/line-height.html: Added. * Source/WebCore/animation/KeyframeEffect.cpp: (WebCore::KeyframeEffect::computeCSSAnimationBlendingKeyframes): * Source/WebCore/style/StyleResolver.cpp: (WebCore::Style::Resolver::keyframeStylesForAnimation): * Source/WebCore/style/StyleResolver.h: Canonical link: https://commits.webkit.org/259631@main
WebKit · Jan 31, 2023 · 9f095ff · 9f095ff
1 parent 9de2e80
commit 9f095ff
Show file tree

Hide file tree

Showing 5 changed files with 59 additions and 3 deletions.
diff --git a/...ts/imported/w3c/web-platform-tests/css/css-animations/responsive/line-height-expected.txt b/...ts/imported/w3c/web-platform-tests/css/css-animations/responsive/line-height-expected.txt
@@ -0,0 +1,3 @@
+
+PASS line-height responds to inherited changes
+
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/responsive/line-height.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/responsive/line-height.html
@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<title>CSS Animations: line-height animations respond to style changes</title>
+<link rel="help" href="https://drafts.csswg.org/css-inline/#line-height-property">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<style>
+  #target {
+    animation-name: line-height-animation;
+    animation-duration: 4s;
+    animation-timing-function: linear;
+    animation-delay: -2s;
+    animation-play-state: paused;
+  }
+  @keyframes line-height-animation {
+    from { line-height: inherit; }
+    to { line-height: 20px; }
+  }
+</style>
+</head>
+<body>
+<div id="container">
+  <div id="target"></div>
+</div>
+<script>
+'use strict';
+const container = document.getElementById('container');
+const target = document.getElementById('target');
+
+test(() => {
+  container.style.lineHeight = '100px';
+  assert_equals(getComputedStyle(target).lineHeight, '60px');
+
+  container.style.lineHeight = '50px';
+  assert_equals(getComputedStyle(target).lineHeight, '35px');
+
+  container.style.lineHeight = '100px';
+  assert_equals(getComputedStyle(target).lineHeight, '60px');
+}, 'line-height responds to inherited changes');
+
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/animation/KeyframeEffect.cpp b/Source/WebCore/animation/KeyframeEffect.cpp
@@ -1051,7 +1051,7 @@ void KeyframeEffect::computeCSSAnimationBlendingKeyframes(const RenderStyle& una
 
     KeyframeList keyframeList(AtomString { backingAnimation.name().string });
     if (auto* styleScope = Style::Scope::forOrdinal(*m_target, backingAnimation.nameStyleScopeOrdinal()))
-        styleScope->resolver().keyframeStylesForAnimation(*m_target, unanimatedStyle, resolutionContext, keyframeList, m_containsCSSVariableReferences);
+        styleScope->resolver().keyframeStylesForAnimation(*m_target, unanimatedStyle, resolutionContext, keyframeList, m_containsCSSVariableReferences, m_inheritedProperties);
 
     // Ensure resource loads for all the frames.
     for (auto& keyframe : keyframeList) {

diff --git a/Source/WebCore/style/StyleResolver.cpp b/Source/WebCore/style/StyleResolver.cpp
@@ -430,8 +430,10 @@ 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)
+void Resolver::keyframeStylesForAnimation(const Element& element, const RenderStyle& elementStyle, const ResolutionContext& context, KeyframeList& list, bool& containsCSSVariableReferences, HashSet<CSSPropertyID>& inheritedProperties)
 {
+    inheritedProperties.clear();
+
     list.clear();
 
     auto keyframeRules = keyframeRulesForName(list.animationName());
@@ -454,6 +456,12 @@ void Resolver::keyframeStylesForAnimation(const Element& element, const RenderSt
                 if (auto compositeOperation = toCompositeOperation(*compositeOperationCSSValue))
                     keyframeValue.setCompositeOperation(*compositeOperation);
             }
+            for (auto property : keyframeRule->properties()) {
+                if (auto* cssValue = property.value()) {
+                    if (cssValue->isPrimitiveValue() && downcast<CSSPrimitiveValue>(cssValue)->valueID() == CSSValueInherit)
+                        inheritedProperties.add(property.id());
+                }
+            }
             list.insert(WTFMove(keyframeValue));
         }
     }

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);
+    void keyframeStylesForAnimation(const Element&, const RenderStyle& elementStyle, const ResolutionContext&, KeyframeList&, bool& containsCSSVariableReferences, HashSet<CSSPropertyID>& inheritedProperties);
 
     WEBCORE_EXPORT std::optional<ResolvedStyle> styleForPseudoElement(const Element&, const PseudoElementRequest&, const ResolutionContext&);