- Revision
- 287835
- Author
- [email protected]
- Date
- 2022-01-10 07:09:14 -0800 (Mon, 10 Jan 2022)
Log 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):
Modified Paths
Diff
Modified: trunk/LayoutTests/imported/w3c/ChangeLog (287834 => 287835)
--- trunk/LayoutTests/imported/w3c/ChangeLog 2022-01-10 13:10:42 UTC (rev 287834)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2022-01-10 15:09:14 UTC (rev 287835)
@@ -1,3 +1,14 @@
+2022-01-10 Antoine Quint <[email protected]>
+
+ [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 <[email protected]>
Unreviewed WPT update after https://github.com/web-platform-tests/wpt/pull/32222.
Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt (287834 => 287835)
--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt 2022-01-10 13:10:42 UTC (rev 287834)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative-expected.txt 2022-01-10 15:09:14 UTC (rev 287835)
@@ -17,8 +17,8 @@
PASS KeyframeEffect.getKeyframes() returns expected frames for overlapping keyframes
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
PASS KeyframeEffect.getKeyframes() returns expected values for animations with a CSS variable which is overriden by the value in keyframe
Modified: trunk/Source/WebCore/ChangeLog (287834 => 287835)
--- trunk/Source/WebCore/ChangeLog 2022-01-10 13:10:42 UTC (rev 287834)
+++ trunk/Source/WebCore/ChangeLog 2022-01-10 15:09:14 UTC (rev 287835)
@@ -1,3 +1,21 @@
+2022-01-10 Antoine Quint <[email protected]>
+
+ [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 <[email protected]>
[LBSE] Begin layer-aware RenderSVGRect implementation
Modified: trunk/Source/WebCore/animation/KeyframeEffect.cpp (287834 => 287835)
--- trunk/Source/WebCore/animation/KeyframeEffect.cpp 2022-01-10 13:10:42 UTC (rev 287834)
+++ trunk/Source/WebCore/animation/KeyframeEffect.cpp 2022-01-10 15:09:14 UTC (rev 287835)
@@ -665,6 +665,17 @@
return nullptr;
};
+ 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();
HashSet<CSSPropertyID> _oneKeyframeProperties_ = computedKeyframes.properties();
@@ -692,6 +703,8 @@
// };
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,
// and keyframe-specific timing function of keyframe.
@@ -703,10 +716,27 @@
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.
+ String idlValue = "";
+ if (keyframeRule) {
+ if (auto cssValue = keyframeRule->properties().getPropertyCSSValue(cssPropertyId)) {
+ if (!cssValue->hasVariableReferences())
+ idlValue = keyframeRule->properties().getPropertyValue(cssPropertyId);
+ }
+ }
+ 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();
+ }
// 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,
@@ -715,38 +745,21 @@
};
// 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) {
- if (auto cssValue = keyframeRule->properties().getPropertyCSSValue(cssPropertyId)) {
- if (!cssValue->hasVariableReferences())
- idlValue = keyframeRule->properties().getPropertyValue(cssPropertyId);
- }
- }
- if (idlValue.isEmpty()) {
- if (auto cssValue = computedStyleExtractor.valueForPropertyInStyle(style, cssPropertyId, renderer))
- idlValue = cssValue->cssText();
- }
- addPropertyToKeyframe(cssPropertyId, idlValue);
+ addPropertyToKeyframe(cssPropertyId);
}
// Now add the implicit properties in case there are any and we're dealing with a 0% or 100% keyframe.
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();
}
}
