Title: [236809] trunk
Revision
236809
Author
grao...@webkit.org
Date
2018-10-03 13:54:17 -0700 (Wed, 03 Oct 2018)

Log Message

[Web Animations] REGRESSION: setting 'animation-name: none' after a 'fill: forwards' animation has completed does not revert to the unanimated style
https://bugs.webkit.org/show_bug.cgi?id=190257
<rdar://problem/41341473>

Reviewed by Dean Jackson.

Source/WebCore:

Test: animations/animation-fill-forwards-removal.html

While we removed a declarative animation that was no longer targetting its element, we were not removing it from the declarative animation maps
on the timeline, which means that the animation would still be picked up when resolving styles. We now notify the timeline that the animation
was detached from the element. This preserves the DeclarativeAnimation relationship returning the element as its effect's target and the document
timeline as its timeline, but the document timeline will no longer see this animation as targeting this element.

* animation/AnimationTimeline.cpp:
(WebCore::AnimationTimeline::cancelOrRemoveDeclarativeAnimation):
* animation/DeclarativeAnimation.h:
(WebCore::DeclarativeAnimation::target const):

LayoutTests:

Add a test that checks that an animation with fill: forwards no longer applies to an element once it's been removed.

* animations/animation-fill-forwards-removal-expected.txt: Added.
* animations/animation-fill-forwards-removal.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (236808 => 236809)


--- trunk/LayoutTests/ChangeLog	2018-10-03 20:19:37 UTC (rev 236808)
+++ trunk/LayoutTests/ChangeLog	2018-10-03 20:54:17 UTC (rev 236809)
@@ -1,3 +1,16 @@
+2018-10-03  Antoine Quint  <grao...@apple.com>
+
+        [Web Animations] REGRESSION: setting 'animation-name: none' after a 'fill: forwards' animation has completed does not revert to the unanimated style
+        https://bugs.webkit.org/show_bug.cgi?id=190257
+        <rdar://problem/41341473>
+
+        Reviewed by Dean Jackson.
+
+        Add a test that checks that an animation with fill: forwards no longer applies to an element once it's been removed.
+
+        * animations/animation-fill-forwards-removal-expected.txt: Added.
+        * animations/animation-fill-forwards-removal.html: Added.
+
 2018-10-03  Chris Dumez  <cdu...@apple.com>
 
         Regression(r236779): Crash when changing the input element type from inside an 'input' event listener

Added: trunk/LayoutTests/animations/animation-fill-forwards-removal-expected.txt (0 => 236809)


--- trunk/LayoutTests/animations/animation-fill-forwards-removal-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/animations/animation-fill-forwards-removal-expected.txt	2018-10-03 20:54:17 UTC (rev 236809)
@@ -0,0 +1,3 @@
+
+PASS Setting 'animation-name: none' after a 'fill: forwards' animation has completed reverts to the unanimated style. 
+

Added: trunk/LayoutTests/animations/animation-fill-forwards-removal.html (0 => 236809)


--- trunk/LayoutTests/animations/animation-fill-forwards-removal.html	                        (rev 0)
+++ trunk/LayoutTests/animations/animation-fill-forwards-removal.html	2018-10-03 20:54:17 UTC (rev 236809)
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<meta charset=utf-8>
+<style>
+    @keyframes animation {
+        to { margin-left: 100px }
+    }
+</style>
+<body>
+<script src=""
+<script src=""
+<script>
+
+'use strict';
+
+async_test(t => {
+    const target = document.body.appendChild(document.createElement("div"));
+    target.style.animation = "animation 10ms forwards";
+
+    target.addEventListener("animationend", () => {
+        assert_equals(getComputedStyle(target).marginLeft, "100px", "The target element has style values from the final keyframe of its animation.");
+        target.style.animation = "none";
+        assert_equals(getComputedStyle(target).marginLeft, "0px", "The target element has no animation after setting 'animation-name: none'.");
+        t.done();
+    });
+}, "Setting 'animation-name: none' after a 'fill: forwards' animation has completed reverts to the unanimated style.");
+
+</script>
+</body>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (236808 => 236809)


--- trunk/Source/WebCore/ChangeLog	2018-10-03 20:19:37 UTC (rev 236808)
+++ trunk/Source/WebCore/ChangeLog	2018-10-03 20:54:17 UTC (rev 236809)
@@ -1,3 +1,23 @@
+2018-10-03  Antoine Quint  <grao...@apple.com>
+
+        [Web Animations] REGRESSION: setting 'animation-name: none' after a 'fill: forwards' animation has completed does not revert to the unanimated style
+        https://bugs.webkit.org/show_bug.cgi?id=190257
+        <rdar://problem/41341473>
+
+        Reviewed by Dean Jackson.
+
+        Test: animations/animation-fill-forwards-removal.html
+
+        While we removed a declarative animation that was no longer targetting its element, we were not removing it from the declarative animation maps
+        on the timeline, which means that the animation would still be picked up when resolving styles. We now notify the timeline that the animation
+        was detached from the element. This preserves the DeclarativeAnimation relationship returning the element as its effect's target and the document
+        timeline as its timeline, but the document timeline will no longer see this animation as targeting this element.
+
+        * animation/AnimationTimeline.cpp:
+        (WebCore::AnimationTimeline::cancelOrRemoveDeclarativeAnimation):
+        * animation/DeclarativeAnimation.h:
+        (WebCore::DeclarativeAnimation::target const):
+
 2018-10-03  Jer Noble  <jer.no...@apple.com>
 
         CRASH in CVPixelBufferGetBytePointerCallback()

Modified: trunk/Source/WebCore/animation/AnimationTimeline.cpp (236808 => 236809)


--- trunk/Source/WebCore/animation/AnimationTimeline.cpp	2018-10-03 20:19:37 UTC (rev 236808)
+++ trunk/Source/WebCore/animation/AnimationTimeline.cpp	2018-10-03 20:54:17 UTC (rev 236809)
@@ -477,14 +477,8 @@
 void AnimationTimeline::cancelOrRemoveDeclarativeAnimation(RefPtr<DeclarativeAnimation> animation)
 {
     ASSERT(animation);
-    if (auto* effect = animation->effect()) {
-        auto phase = effect->phase();
-        if (phase != AnimationEffectReadOnly::Phase::Idle && phase != AnimationEffectReadOnly::Phase::After) {
-            animation->cancel();
-            return;
-        }
-    }
-
+    animation->cancel();
+    animationWasRemovedFromElement(*animation, animation->target());
     removeAnimation(animation.releaseNonNull());
 }
 

Modified: trunk/Source/WebCore/animation/DeclarativeAnimation.h (236808 => 236809)


--- trunk/Source/WebCore/animation/DeclarativeAnimation.h	2018-10-03 20:19:37 UTC (rev 236808)
+++ trunk/Source/WebCore/animation/DeclarativeAnimation.h	2018-10-03 20:54:17 UTC (rev 236809)
@@ -42,6 +42,7 @@
 
     bool isDeclarativeAnimation() const final { return true; }
 
+    Element& target() const { return m_target; }
     const Animation& backingAnimation() const { return m_backingAnimation; }
     void setBackingAnimation(const Animation&);
     void invalidateDOMEvents(Seconds elapsedTime = 0_s);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to