Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[web-animations-2] [scroll-animations] setting an animation's timeline to a progress-based timeline should create a new ready promise #11465

Open
graouts opened this issue Jan 9, 2025 · 1 comment
Labels

Comments

@graouts
Copy link
Contributor

graouts commented Jan 9, 2025

Step 9 of the setting the timeline of an animation procedure deals with the case where the new timeline is progress-based is to set the auto align start time flag to true. When this flag is true, the animation's start time may be computed to a new value based on the associated timeline's current time.

Furthermore, this procedures schedules a pending play task if the animation had finished or was running.

Since the start time will be set when the timeline is updated next, it seems to me that we should also reset the ready promise in the case where we schedule a pending play task.

In fact, some existing scroll-animations tests actually expect this behavior:

Chrome passes those tests and seems to implement this behavior. I think this is an oversight in the spec.

@graouts graouts added web-animations-2 Current Work scroll-animations-1 Current Work labels Jan 9, 2025
@graouts
Copy link
Contributor Author

graouts commented Jan 9, 2025

Cc @kevers-google, @flackr, @andruud and @bramus.

graouts added a commit to graouts/WebKit that referenced this issue Jan 9, 2025
…tion should reset its `ready` promise

https://bugs.webkit.org/show_bug.cgi?id=285667
rdar://142608438

Reviewed by NOBODY (OOPS!).

We were failing some scroll-animations WPT tests related to switching timelines dynamically because these tests
relied on the animation's `ready` promise being reset upon setting a new progress-based timeline. While the spec
doesn't currently state that this should occur, it makes sense that it would if the animation was not already
pending since a new start time will be computed in the next animation frame.

A spec issue was filed at w3c/csswg-drafts#11465 to clear this up.

So when we schedule a pending play task, we also reset the `ready` promise in case the animation was not already
in the pending state.

* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/css/animation-timeline-ignored.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/scroll-timelines/setting-timeline.tentative-expected.txt:
* Source/WebCore/animation/WebAnimation.cpp:
(WebCore::WebAnimation::setTimeline):
graouts added a commit to graouts/WebKit that referenced this issue Jan 9, 2025
…tion should reset its `ready` promise

https://bugs.webkit.org/show_bug.cgi?id=285667
rdar://142608438

Reviewed by NOBODY (OOPS!).

We were failing some scroll-animations WPT tests related to switching timelines dynamically because these tests
relied on the animation's `ready` promise being reset upon setting a new progress-based timeline. While the spec
doesn't currently state that this should occur, it makes sense that it would if the animation was not already
pending since a new start time will be computed in the next animation frame.

A spec issue was filed at w3c/csswg-drafts#11465 to clear this up.

So when we schedule a pending play task, we also reset the `ready` promise in case the animation was not already
in the pending state.

* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/css/animation-timeline-ignored.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/scroll-timelines/setting-timeline.tentative-expected.txt:
* LayoutTests/platform/glib/TestExpectations:
* LayoutTests/platform/win/TestExpectations:
* Source/WebCore/animation/WebAnimation.cpp:
(WebCore::WebAnimation::setTimeline):
webkit-commit-queue pushed a commit to graouts/WebKit that referenced this issue Jan 9, 2025
…tion should reset its `ready` promise

https://bugs.webkit.org/show_bug.cgi?id=285667
rdar://142608438

Reviewed by Anne van Kesteren.

We were failing some scroll-animations WPT tests related to switching timelines dynamically because these tests
relied on the animation's `ready` promise being reset upon setting a new progress-based timeline. While the spec
doesn't currently state that this should occur, it makes sense that it would if the animation was not already
pending since a new start time will be computed in the next animation frame.

A spec issue was filed at w3c/csswg-drafts#11465 to clear this up.

So when we schedule a pending play task, we also reset the `ready` promise in case the animation was not already
in the pending state.

* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/css/animation-timeline-ignored.tentative-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/scroll-animations/scroll-timelines/setting-timeline.tentative-expected.txt:
* LayoutTests/platform/glib/TestExpectations:
* LayoutTests/platform/win/TestExpectations:
* Source/WebCore/animation/WebAnimation.cpp:
(WebCore::WebAnimation::setTimeline):

Canonical link: https://commits.webkit.org/288655@main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant