Changeset 287835 in webkit

Timestamp:

Jan 10, 2022 7:09:14 AM (6 months ago)

Author:

graouts@webkit.org

Message:

[Web Animations] getKeyframes() for a CSS Animation should not use computed style for keyframes (part 2)
​https://bugs.webkit.org/show_bug.cgi?id=235028

Reviewed by Antti Koivisto.

LayoutTests/imported/w3c:

Mark WPT progressions.

• web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt:

Source/WebCore:

In bug 235008, we already improved the situtation by reading from the StyleProperties
associated with the StyleRuleKeyframe for CSS Animations. We're now taking the next
step by reading from the matching rules for the provided element.

There is one remaining case where we use the computed style: when the CSSValue uses
a CSS variable. To fix the css/css-animations/KeyframeEffect-getKeyframes.tentative.html
WPT entirely, we'll need to be able to substitute those in the output.

• animation/KeyframeEffect.cpp:

(WebCore::KeyframeEffect::getKeyframes):

Location:

trunk

Files:

• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/animation/KeyframeEffect.cpp (modified) (5 diffs)

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+2022-01-10  Antoine Quint  <graouts@webkit.org>
+
+        [Web Animations] getKeyframes() for a CSS Animation should not use computed style for keyframes (part 2)
+        https://bugs.webkit.org/show_bug.cgi?id=235028
+
+        Reviewed by Antti Koivisto.
+
+        Mark WPT progressions.
+
+        * web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt:
+
 2022-01-09  Antoine Quint  <graouts@webkit.org>
 

trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt

@@ -18 +18 @@
 PASS KeyframeEffect.getKeyframes() returns expected values for animations with filter properties and missing keyframes
 PASS KeyframeEffect.getKeyframes() returns expected values for animation with drop-shadow of filter property
-FAIL KeyframeEffect.getKeyframes() returns expected values for animations with text-shadow properties and missing keyframes assert_equals: value for 'textShadow' on Keyframe #0 should match expected "rgb(0, 0, 0) 1px 1px 2px, rgb(0, 0, 255) 0px 0px 16px, rgb(0, 0, 255) 0px 0px 3.2px" but got "rgb(0, 0, 0) 1px 1px 2px, rgb(0, 0, 255) 0px 0px 16px, rgb(0, 0, 255) 0px 0px 3.200000047683716px"
-FAIL KeyframeEffect.getKeyframes() returns expected values for animations with background-size properties and missing keyframes assert_equals: value for 'backgroundSize' on ComputedKeyframe #0 after updating current style should match expected "30px, 40%, auto" but got "30px"
+PASS KeyframeEffect.getKeyframes() returns expected values for animations with text-shadow properties and missing keyframes
+PASS KeyframeEffect.getKeyframes() returns expected values for animations with background-size properties and missing keyframes
 FAIL KeyframeEffect.getKeyframes() returns expected values for animations with CSS variables as keyframe values assert_equals: value for 'transform' on Keyframe #1 should match expected "translate(100px)" but got "matrix(1, 0, 0, 1, 100, 0)"
 PASS KeyframeEffect.getKeyframes() returns expected values for animations with CSS variables as keyframe values in a shorthand property

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2022-01-10  Antoine Quint  <graouts@webkit.org>
+
+        [Web Animations] getKeyframes() for a CSS Animation should not use computed style for keyframes (part 2)
+        https://bugs.webkit.org/show_bug.cgi?id=235028
+
+        Reviewed by Antti Koivisto.
+
+        In bug 235008, we already improved the situtation by reading from the StyleProperties
+        associated with the StyleRuleKeyframe for CSS Animations. We're now taking the next
+        step by reading from the matching rules for the provided element.
+
+        There is one remaining case where we use the computed style: when the CSSValue uses
+        a CSS variable. To fix the css/css-animations/KeyframeEffect-getKeyframes.tentative.html
+        WPT entirely, we'll need to be able to substitute those in the output.
+
+        * animation/KeyframeEffect.cpp:
+        (WebCore::KeyframeEffect::getKeyframes):
+
 2022-01-10  Nikolas Zimmermann  <nzimmermann@igalia.com>
 

trunk/Source/WebCore/animation/KeyframeEffect.cpp

@@ -666 +666 @@
         };
 
+        auto styleProperties = MutableStyleProperties::create();
+        if (m_blendingKeyframesSource == BlendingKeyframesSource::CSSAnimation) {
+            auto matchingRules = m_target->styleResolver().pseudoStyleRulesForElement(target, m_pseudoId, Style::Resolver::AllCSSRules);
+            for (auto& matchedRule : matchingRules)
+                styleProperties->mergeAndOverrideOnConflict(matchedRule->properties());
+            if (is<StyledElement>(m_target) && m_pseudoId == PseudoId::None) {
+                if (auto* inlineProperties = downcast<StyledElement>(*m_target).inlineStyle())
+                    styleProperties->mergeAndOverrideOnConflict(*inlineProperties);
+            }
+        }
+
         // We need to establish which properties are implicit for 0% and 100%.
         HashSet<CSSPropertyID> zeroKeyframeProperties = computedKeyframes.properties();
@@ -693 +704 @@
 
             auto& keyframe = computedKeyframes[i];
+            auto& style = *keyframe.style();
+            auto* keyframeRule = keyframeRuleForKey(keyframe.key());
 
             // 2. Set offset, computedOffset, easing members of output keyframe to the respective values keyframe offset, computed keyframe offset,
@@ -704 +717 @@
             auto outputKeyframe = convertDictionaryToJS(lexicalGlobalObject, *jsCast<JSDOMGlobalObject*>(&lexicalGlobalObject), computedKeyframe);
 
-            auto addPropertyToKeyframe = [&](CSSPropertyID cssPropertyId, String idlValue) {
+            auto addPropertyToKeyframe = [&](CSSPropertyID cssPropertyId) {
                 // 1. Let property name be the result of applying the animation property name to IDL attribute name algorithm to the property name of declaration.
                 auto propertyName = CSSPropertyIDToIDLAttributeName(cssPropertyId);
                 // 2. Let IDL value be the result of serializing the property value of declaration by passing declaration to the algorithm to serialize a CSS value.
-                // 3. Let value be the result of converting IDL value to an ECMAScript String value.
-                auto value = toJS<IDLDOMString>(lexicalGlobalObject, idlValue);
-                // 4. Call the [[DefineOwnProperty]] internal method on output keyframe with property name property name,
-                //    Property Descriptor { [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: true, [[Value]]: value } and Boolean flag false.
-                JSObject::defineOwnProperty(outputKeyframe, &lexicalGlobalObject, AtomString(propertyName).impl(), PropertyDescriptor(value, 0), false);
-            };
-
-            // 3. For each animation property-value pair specified on keyframe, declaration, perform the following steps:
-            auto& style = *keyframe.style();
-            auto* keyframeRule = keyframeRuleForKey(keyframe.key());
-            for (auto cssPropertyId : keyframe.properties()) {
-                if (cssPropertyId == CSSPropertyCustom)
-                    continue;
                 String idlValue = "";
                 if (keyframeRule) {
@@ -729 +729 @@
                 }
                 if (idlValue.isEmpty()) {
+                    if (auto cssValue = styleProperties->getPropertyCSSValue(cssPropertyId)) {
+                        if (!cssValue->hasVariableReferences())
+                            idlValue = styleProperties->getPropertyValue(cssPropertyId);
+                    }
+                }
+                if (idlValue.isEmpty()) {
                     if (auto cssValue = computedStyleExtractor.valueForPropertyInStyle(style, cssPropertyId, renderer))
                         idlValue = cssValue->cssText();
                 }
-                addPropertyToKeyframe(cssPropertyId, idlValue);
+                // 3. Let value be the result of converting IDL value to an ECMAScript String value.
+                auto value = toJS<IDLDOMString>(lexicalGlobalObject, idlValue);
+                // 4. Call the [[DefineOwnProperty]] internal method on output keyframe with property name property name,
+                //    Property Descriptor { [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: true, [[Value]]: value } and Boolean flag false.
+                JSObject::defineOwnProperty(outputKeyframe, &lexicalGlobalObject, AtomString(propertyName).impl(), PropertyDescriptor(value, 0), false);
+            };
+
+            // 3. For each animation property-value pair specified on keyframe, declaration, perform the following steps:
+            for (auto cssPropertyId : keyframe.properties()) {
+                if (cssPropertyId == CSSPropertyCustom)
+                    continue;
+                addPropertyToKeyframe(cssPropertyId);
             }
 
@@ -738 +755 @@
             if (lastStyleChangeEventStyle) {
                 if (!keyframe.key()) {
-                    for (auto cssPropertyId : zeroKeyframeProperties) {
-                        if (auto cssValue = computedStyleExtractor.valueForPropertyInStyle(*lastStyleChangeEventStyle, cssPropertyId, renderer))
-                            addPropertyToKeyframe(cssPropertyId, cssValue->cssText());
-                    }
+                    for (auto cssPropertyId : zeroKeyframeProperties)
+                        addPropertyToKeyframe(cssPropertyId);
                     zeroKeyframeProperties.clear();
                 } else if (keyframe.key() == 1) {
-                    for (auto cssPropertyId : oneKeyframeProperties) {
-                        if (auto cssValue = computedStyleExtractor.valueForPropertyInStyle(*lastStyleChangeEventStyle, cssPropertyId, renderer))
-                            addPropertyToKeyframe(cssPropertyId, cssValue->cssText());
-                    }
+                    for (auto cssPropertyId : oneKeyframeProperties)
+                        addPropertyToKeyframe(cssPropertyId);
                     oneKeyframeProperties.clear();
                 }