Title: [287881] trunk
Revision
287881
Author
grao...@webkit.org
Date
2022-01-11 08:20:06 -0800 (Tue, 11 Jan 2022)

Log Message

css/css-transitions/KeyframeEffect-setKeyframes.tentative.html is a failure
https://bugs.webkit.org/show_bug.cgi?id=235062

Reviewed by Dean Jackson.

LayoutTests/imported/w3c:

Mark WPT progressions.

* web-platform-tests/css/css-transitions/KeyframeEffect-setKeyframes.tentative-expected.txt:

Source/WebCore:

The WPT at css/css-transitions/KeyframeEffect-setKeyframes.tentative.html, which checks
the behavior of programmatically changing keyframes for a CSS Transition, highlighted
three failures:

1. we did not flush pending style changes as setKeyframes() was called for an animation
   created fromm CSS (CSS Transition or CSS Animation),
2. we did not use the style from the last style update to computed the before-change style
   if available, but rather the computed style generated _after_ the last style update,
3. we did not apply _all_ animations as we compute the before-change style to consider
   new transitions to run, but only animations matching the current property.

We've corrected all of those errors and this test now passes save for one failure, which
seems to be a different type of problem and will be looked at in a future patch.

* animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::setBindingsKeyframes):
* style/Styleable.cpp:
(WebCore::updateCSSTransitionsForStyleableAndProperty):

Modified Paths

Diff

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (287880 => 287881)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2022-01-11 15:44:23 UTC (rev 287880)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2022-01-11 16:20:06 UTC (rev 287881)
@@ -1,3 +1,14 @@
+2022-01-11  Antoine Quint  <grao...@webkit.org>
+
+        css/css-transitions/KeyframeEffect-setKeyframes.tentative.html is a failure
+        https://bugs.webkit.org/show_bug.cgi?id=235062
+
+        Reviewed by Dean Jackson.
+
+        Mark WPT progressions.
+
+        * web-platform-tests/css/css-transitions/KeyframeEffect-setKeyframes.tentative-expected.txt:
+
 2022-01-11  Tim Nguyen  <n...@apple.com>
 
         ::backdrop pseudo element should react to associated element event listeners

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-transitions/KeyframeEffect-setKeyframes.tentative-expected.txt (287880 => 287881)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-transitions/KeyframeEffect-setKeyframes.tentative-expected.txt	2022-01-11 15:44:23 UTC (rev 287880)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-transitions/KeyframeEffect-setKeyframes.tentative-expected.txt	2022-01-11 16:20:06 UTC (rev 287881)
@@ -3,8 +3,8 @@
 PASS A transition with replaced keyframes still returns the original transitionProperty
 PASS A transition with no keyframes still returns the original transitionProperty
 PASS A transition with replaced keyframes animating the same property still exhibits normal reversing behavior.
-FAIL A transition with replaced keyframes animating a different property still exhibits normal reversing behavior  (reversing from the base value). undefined is not an object (evaluating 'reversedTransition.effect')
-FAIL A transition with replaced keyframes animating nothing still exhibits normal reversing behavior (reversing from the base value). undefined is not an object (evaluating 'reversedTransition.effect')
+PASS A transition with replaced keyframes animating a different property still exhibits normal reversing behavior  (reversing from the base value).
+PASS A transition with replaced keyframes animating nothing still exhibits normal reversing behavior (reversing from the base value).
 FAIL A transition with replaced keyframes animating nothing on a property being controlled by another modified transition exhibits normal reversing behavior and reverses from the other transition's current value. assert_equals: The reversed transition gets its start value from the other transition controlling left expected "280px" but got "200px"
-FAIL A transition with replaced kefyrames and composite 'add' exhibits normal reversing behavior, and the effect is not double counted when calculating the before change style assert_equals: The start value matches the 'before change' value expected "175px" but got "150px"
+PASS A transition with replaced kefyrames and composite 'add' exhibits normal reversing behavior, and the effect is not double counted when calculating the before change style
 

Modified: trunk/Source/WebCore/ChangeLog (287880 => 287881)


--- trunk/Source/WebCore/ChangeLog	2022-01-11 15:44:23 UTC (rev 287880)
+++ trunk/Source/WebCore/ChangeLog	2022-01-11 16:20:06 UTC (rev 287881)
@@ -1,3 +1,29 @@
+2022-01-11  Antoine Quint  <grao...@webkit.org>
+
+        css/css-transitions/KeyframeEffect-setKeyframes.tentative.html is a failure
+        https://bugs.webkit.org/show_bug.cgi?id=235062
+
+        Reviewed by Dean Jackson.
+
+        The WPT at css/css-transitions/KeyframeEffect-setKeyframes.tentative.html, which checks
+        the behavior of programmatically changing keyframes for a CSS Transition, highlighted
+        three failures:
+
+        1. we did not flush pending style changes as setKeyframes() was called for an animation
+           created fromm CSS (CSS Transition or CSS Animation),
+        2. we did not use the style from the last style update to computed the before-change style
+           if available, but rather the computed style generated _after_ the last style update,
+        3. we did not apply _all_ animations as we compute the before-change style to consider
+           new transitions to run, but only animations matching the current property.
+
+        We've corrected all of those errors and this test now passes save for one failure, which
+        seems to be a different type of problem and will be looked at in a future patch.
+
+        * animation/KeyframeEffect.cpp:
+        (WebCore::KeyframeEffect::setBindingsKeyframes):
+        * style/Styleable.cpp:
+        (WebCore::updateCSSTransitionsForStyleableAndProperty):
+
 2022-01-11  Andres Gonzalez  <andresg...@apple.com>
 
         Make [WebAccessibilityObjectWrapperBase axBackingObject] return the appropriate underlying object for the calling thread.

Modified: trunk/Source/WebCore/animation/KeyframeEffect.cpp (287880 => 287881)


--- trunk/Source/WebCore/animation/KeyframeEffect.cpp	2022-01-11 15:44:23 UTC (rev 287880)
+++ trunk/Source/WebCore/animation/KeyframeEffect.cpp	2022-01-11 16:20:06 UTC (rev 287881)
@@ -819,6 +819,8 @@
 
 ExceptionOr<void> KeyframeEffect::setBindingsKeyframes(JSGlobalObject& lexicalGlobalObject, Strong<JSObject>&& keyframesInput)
 {
+    if (is<DeclarativeAnimation>(animation()))
+        downcast<DeclarativeAnimation>(*animation()).flushPendingStyleChanges();
     auto retVal = setKeyframes(lexicalGlobalObject, WTFMove(keyframesInput));
     if (!retVal.hasException() && is<CSSAnimation>(animation()))
         downcast<CSSAnimation>(*animation()).effectKeyframesWereSetUsingBindings();

Modified: trunk/Source/WebCore/style/Styleable.cpp (287880 => 287881)


--- trunk/Source/WebCore/style/Styleable.cpp	2022-01-11 15:44:23 UTC (rev 287880)
+++ trunk/Source/WebCore/style/Styleable.cpp	2022-01-11 16:20:06 UTC (rev 287881)
@@ -410,20 +410,17 @@
     // Define the before-change style as the computed values of all properties on the element as of the previous style change event, except with
     // any styles derived from declarative animations such as CSS Transitions, CSS Animations, and SMIL Animations updated to the current time.
     auto beforeChangeStyle = [&]() -> const RenderStyle {
-        if (animation && animation->isRelevant()) {
-            auto animatedStyle = RenderStyle::clone(currentStyle);
-            // If a transition has not yet started or started when animations were last updated, use the timeline time at its creation
-            // as its start time to ensure that it will produce a style with progress > 0.
-            bool shouldUseTimelineTimeAtCreation = is<CSSTransition>(animation) && (!animation->startTime() || *animation->startTime() == styleable.element.document().timeline().currentTime());
-            animation->resolve(animatedStyle, { nullptr }, shouldUseTimelineTimeAtCreation ? downcast<CSSTransition>(*animation).timelineTimeAtCreation() : std::nullopt);
-            return animatedStyle;
+        if (auto* lastStyleChangeEventStyle = styleable.lastStyleChangeEventStyle()) {
+            auto style = RenderStyle::clone(*lastStyleChangeEventStyle);
+            if (auto* keyframeEffectStack = styleable.keyframeEffectStack()) {
+                for (const auto& effect : keyframeEffectStack->sortedEffects()) {
+                    auto* effectAnimation = effect->animation();
+                    bool shouldUseTimelineTimeAtCreation = is<CSSTransition>(effectAnimation) && (!effectAnimation->startTime() || *effectAnimation->startTime() == styleable.element.document().timeline().currentTime());
+                    effectAnimation->resolve(style, { nullptr }, shouldUseTimelineTimeAtCreation ? downcast<CSSTransition>(*effectAnimation).timelineTimeAtCreation() : std::nullopt);
+                }
+            }
+            return style;
         }
-
-        // If it exists, use the recorded RenderStyle for this element during a previous call to Style::TreeResolver::createAnimatedElementUpdate().
-        if (auto* lastStyleChangeEventStyle = styleable.lastStyleChangeEventStyle())
-            return RenderStyle::clone(*lastStyleChangeEventStyle);
-
-        // If we haven't computed styles from animations for this element, the before-change style is the previously resolved style for this element.
         return RenderStyle::clone(currentStyle);
     }();
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to