Address review comments.

flackr · Dec 20, 2023 · 1db7fe1 · 1db7fe1
1 parent 00ae9df
commit 1db7fe1
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 14 deletions.
diff --git a/src/proxy-animation.js b/src/proxy-animation.js
@@ -37,7 +37,7 @@ function createReadyPromise(details) {
   details.readyPromise = new PromiseWrapper();
   // Trigger the pending task on the next animation frame.
   requestAnimationFrame(() => {
-    const timelineTime = details.timeline.currentTime;
+    const timelineTime = details.timeline ? details.timeline.currentTime : null;
     if (timelineTime !== null)
       notifyReady(details);
   });
@@ -612,6 +612,11 @@ function renormalizeTiming() {
 }
 
 function notifyReady(details) {
+  // If the timeline has been changed, resolve the previous ready promise.
+  if (!details.timeline) {
+    details.readyPromise.resolve(details.proxy);
+    return;
+  }
   if (details.pendingTask == 'pause') {
     commitPendingPause(details);
   } else if (details.pendingTask == 'play') {

diff --git a/src/scroll-timeline-css.js b/src/scroll-timeline-css.js
@@ -143,15 +143,17 @@ export function initCSSPolyfill() {
   window.addEventListener('animationstart', (evt) => {
     evt.target.getAnimations().filter(anim => anim.animationName === evt.animationName).forEach(anim => {
       const result = createScrollTimeline(anim, anim.animationName, evt.target);
-      // If the CSS Animation refers to a scroll or view timeline we need to proxy the animation instance.
-      if (result.timeline && !(anim instanceof ProxyAnimation)) {
-        const proxyAnimation = new ProxyAnimation(anim, result.timeline, result.animOptions);
-        anim.pause();
-        proxyAnimation.play();
-      } else {
-        // If the timeline was removed or the animation was already an instance of a proxy animation,
-        // invoke the set the timeline procedure on the existing animation.
-        anim.timeline = result.timeline;
+      if (result) {
+        // If the CSS Animation refers to a scroll or view timeline we need to proxy the animation instance.
+        if (result.timeline && !(anim instanceof ProxyAnimation)) {
+          const proxyAnimation = new ProxyAnimation(anim, result.timeline, result.animOptions);
+          anim.pause();
+          proxyAnimation.play();
+        } else {
+          // If the timeline was removed or the animation was already an instance of a proxy animation,
+          // invoke the set the timeline procedure on the existing animation.
+          anim.timeline = result.timeline;
+        }
       }
     });
   });

diff --git a/test/expected.txt b/test/expected.txt
@@ -79,10 +79,10 @@ FAIL	/scroll-animations/css/animation-timeline-deferred.html	Animation.timeline
 FAIL	/scroll-animations/css/animation-timeline-deferred.html	Animation.timeline returns null for inactive deferred timeline
 FAIL	/scroll-animations/css/animation-timeline-deferred.html	Animation.timeline returns null for inactive (overattached) deferred timeline
 FAIL	/scroll-animations/css/animation-timeline-ignored.tentative.html	Changing animation-timeline changes the timeline (sanity check)
-FAIL	/scroll-animations/css/animation-timeline-ignored.tentative.html	animation-timeline ignored after setting timeline with JS (ScrollTimeline from JS)
+PASS	/scroll-animations/css/animation-timeline-ignored.tentative.html	animation-timeline ignored after setting timeline with JS (ScrollTimeline from JS)
 FAIL	/scroll-animations/css/animation-timeline-ignored.tentative.html	animation-timeline ignored after setting timeline with JS (ScrollTimeline from CSS)
-PASS	/scroll-animations/css/animation-timeline-ignored.tentative.html	animation-timeline ignored after setting timeline with JS (document timeline)
-PASS	/scroll-animations/css/animation-timeline-ignored.tentative.html	animation-timeline ignored after setting timeline with JS (null)
+FAIL	/scroll-animations/css/animation-timeline-ignored.tentative.html	animation-timeline ignored after setting timeline with JS (document timeline)
+FAIL	/scroll-animations/css/animation-timeline-ignored.tentative.html	animation-timeline ignored after setting timeline with JS (null)
 FAIL	/scroll-animations/css/animation-timeline-in-keyframe.html	The animation-timeline property may not be used in keyframes
 PASS	/scroll-animations/css/animation-timeline-multiple.html	animation-timeline works with multiple timelines
 FAIL	/scroll-animations/css/animation-timeline-none.html	Animation with animation-timeline:none holds current time at zero
@@ -957,4 +957,4 @@ FAIL	/scroll-animations/view-timelines/view-timeline-sticky-block.html	View time
 FAIL	/scroll-animations/view-timelines/view-timeline-sticky-inline.html	View timeline with sticky target, block axis.
 FAIL	/scroll-animations/view-timelines/view-timeline-subject-size-changes.html	View timeline with subject size change after the creation of the animation
 FAIL	/scroll-animations/view-timelines/zero-intrinsic-iteration-duration.tentative.html	Intrinsic iteration duration is non-negative
-Passed 356 of 959 tests.
+Passed 355 of 959 tests.