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] web-animations/interfaces/Animatable/animate-no-browsing-context.html is a unique failure #9872

Conversation

graouts
Copy link
Contributor

@graouts graouts commented Feb 9, 2023

87f47a6

[web-animations] web-animations/interfaces/Animatable/animate-no-browsing-context.html is a unique failure
https://bugs.webkit.org/show_bug.cgi?id=186494
rdar://problem/41000163

Reviewed by Dean Jackson.

The Web Animations spec defines several factors for an animation to be considered "ready" [0], one being
that the "user agent has completed any setup required to begin the playback of the animation's associated
effect including rendering the first frame of any keyframe effect".

While it does not specifically calls out the lack of a browsing context for the associated effect's target's
document, there certainly won't be any frames rendered in that specific case. So we do not run pending tasks,
the condition for the "ready" promise to resolve, if the associated effect is associated with a document
lacking a browsing context.

I filed a spec issue [1] to clarify this in the spec, but I believe the spirit of the spec already goes in
the direction of this test.

[0] https://drafts.csswg.org/web-animations-1/#ready
[1] w3c/csswg-drafts#8439

* LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animatable/animate-no-browsing-context-expected.txt:
* Source/WebCore/animation/AnimationEffect.h:
(WebCore::AnimationEffect::preventsAnimationReadiness const):
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::preventsAnimationReadiness const):
* Source/WebCore/animation/KeyframeEffect.h:
* Source/WebCore/animation/WebAnimation.cpp:
(WebCore::WebAnimation::tick):

Canonical link: https://commits.webkit.org/260101@main

80e03dc

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  wincairo
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ›  gtk
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk1 βœ… πŸ§ͺ api-gtk
βœ… πŸ›  tv βœ… πŸ§ͺ mac-wk2
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-AS-debug-wk2
βœ… πŸ›  watch βœ… πŸ§ͺ mac-wk2-stress
βœ… πŸ›  watch-sim
βœ… πŸ›  πŸ§ͺ unsafe-merge

@graouts graouts self-assigned this Feb 9, 2023
@graouts graouts added the Animations Bugs related to CSS + SVG animations and transitions label Feb 9, 2023
@graouts graouts force-pushed the animation-readiness-no-browsing-context branch from a368fff to 88dcbcd Compare February 9, 2023 17:33
@graouts graouts force-pushed the animation-readiness-no-browsing-context branch from 88dcbcd to 80e03dc Compare February 9, 2023 17:36
@graouts graouts added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Feb 10, 2023
…sing-context.html is a unique failure

https://bugs.webkit.org/show_bug.cgi?id=186494
rdar://problem/41000163

Reviewed by Dean Jackson.

The Web Animations spec defines several factors for an animation to be considered "ready" [0], one being
that the "user agent has completed any setup required to begin the playback of the animation's associated
effect including rendering the first frame of any keyframe effect".

While it does not specifically calls out the lack of a browsing context for the associated effect's target's
document, there certainly won't be any frames rendered in that specific case. So we do not run pending tasks,
the condition for the "ready" promise to resolve, if the associated effect is associated with a document
lacking a browsing context.

I filed a spec issue [1] to clarify this in the spec, but I believe the spirit of the spec already goes in
the direction of this test.

[0] https://drafts.csswg.org/web-animations-1/#ready
[1] w3c/csswg-drafts#8439

* LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animatable/animate-no-browsing-context-expected.txt:
* Source/WebCore/animation/AnimationEffect.h:
(WebCore::AnimationEffect::preventsAnimationReadiness const):
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::preventsAnimationReadiness const):
* Source/WebCore/animation/KeyframeEffect.h:
* Source/WebCore/animation/WebAnimation.cpp:
(WebCore::WebAnimation::tick):

Canonical link: https://commits.webkit.org/260101@main
@webkit-commit-queue
Copy link
Collaborator

Committed 260101@main (87f47a6): https://commits.webkit.org/260101@main

Reviewed commits have been landed. Closing PR #9872 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system merged commit 87f47a6 into WebKit:main Feb 10, 2023
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Feb 10, 2023
@graouts graouts deleted the animation-readiness-no-browsing-context branch February 10, 2023 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Animations Bugs related to CSS + SVG animations and transitions
Projects
None yet
4 participants