Skip to content

Animation not updated when Container Query units are used #52296

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 15, 2025
Merged

Animation not updated when Container Query units are used #52296

merged 1 commit into from
Oct 15, 2025
+94 −40

Conversation

@graouts
Copy link
Contributor

@graouts graouts commented Oct 14, 2025 •
edited by webkit-early-warning-system
Loading

1fab171

Animation not updated when Container Query units are used
https://bugs.webkit.org/show_bug.cgi?id=299952
rdar://162194744

Reviewed by Antti Koivisto and Anne van Kesteren.

Recompute keyframes when container query units are used for all types of animations,
not just CSS Animations.

* LayoutTests/imported/w3c/web-platform-tests/css/css-conditional/container-queries/container-units-animation-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-conditional/container-queries/container-units-animation.html:
* Source/WebCore/animation/CSSAnimation.cpp:
(WebCore::CSSAnimation::keyframesRuleDidChange):
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::recomputeKeyframesAtNextOpportunity):
(WebCore::KeyframeEffect::keyframesRuleDidChange): Deleted.
* Source/WebCore/animation/KeyframeEffect.h:
* Source/WebCore/style/Styleable.cpp:
(WebCore::Styleable::queryContainerDidChange const):

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

142e675

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win loading 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2   🧪 win-tests ✅ 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ❌ 🧪 api-wpe ✅ 🛠 vision-apple
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
  🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 loading-orange 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@graouts graouts self-assigned this Oct 14, 2025
@graouts graouts added the Animations Bugs related to CSS + SVG animations and transitions label Oct 14, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Oct 14, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 07ab8b8)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style   🛠 ios   🛠 mac ⏳ 🛠 wpe   🛠 win ❌ 🛠 ios-apple
✅ 🧪 bindings   🛠 ios-sim   🛠 mac-AS-debug ⏳ 🧪 wpe-wk2   🧪 win-tests ⏳ 🛠 mac-apple
✅ 🧪 webkitperl   🧪 ios-wk2   🧪 api-mac ⏳ 🧪 api-wpe ❌ 🛠 vision-apple
  🧪 ios-wk2-wpt   🧪 mac-wk1 ⏳ 🛠 wpe-cairo
⏳ 🛠 🧪 jsc   🧪 api-ios   🧪 mac-wk2 ⏳ 🛠 gtk
⏳ 🛠 🧪 jsc-arm64   🛠 vision   🧪 mac-AS-debug-wk2 ⏳ 🧪 gtk-wk2
  🛠 vision-sim   🧪 mac-wk2-stress ⏳ 🧪 api-gtk
⏳ 🧪 vision-wk2   🧪 mac-intel-wk2   🛠 playstation
  🛠 tv   🛠 mac-safer-cpp
  🛠 tv-sim
  🛠 watch
  🛠 watch-sim

@graouts graouts force-pushed the eng/Animation-not-updated-when-Container-Query-units-are-used branch from 07ab8b8 to 76497b2 Compare October 14, 2025 14:22
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Oct 14, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 76497b2)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win ✅ 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ❌ 🧪 wpe-wk2 ✅ 🧪 win-tests ✅ 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe ✅ 🛠 vision-apple
❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ❌ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ❌ 🧪 mac-AS-debug-wk2 ❌ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ❌ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 14, 2025
@KingSora KingSora mentioned this pull request Oct 14, 2025
Open
@graouts graouts removed the merging-blocked Applied to prevent a change from being merged label Oct 15, 2025
@graouts graouts force-pushed the eng/Animation-not-updated-when-Container-Query-units-are-used branch from 76497b2 to 142e675 Compare October 15, 2025 15:38
@graouts graouts requested review from annevk and anttijk and removed request for anttijk October 15, 2025 15:38
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Oct 15, 2025 •
edited
Loading

EWS run on current version of this PR (hash 142e675)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows Apple Internal
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win loading 🛠 ios-apple
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2   🧪 win-tests ✅ 🛠 mac-apple
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ❌ 🧪 api-wpe ✅ 🛠 vision-apple
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
  🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 loading-orange 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@graouts graouts requested review from anttijk, heycam and nt1m October 15, 2025 15:38
@graouts graouts marked this pull request as ready for review October 15, 2025 15:39
annevk
annevk approved these changes Oct 15, 2025
View reviewed changes
Source/WebCore/style/Styleable.cpp
if (keyframeEffect && keyframeEffect->blendingKeyframes().usesContainerUnits())
cssAnimation->keyframesRuleDidChange();
for (auto& animation : *animations) {
RefPtr keyframeEffect = dynamicDowncast<KeyframeEffect>(animation->effect());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be sure, effect() here can return a variety of types? (We always seem to dynamicDowncast to KeyframeEffect.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The effect() method returns an AnimationEffect, current subclasses are KeyframeEffect and CustomEffect (experimental at this stage).

@graouts graouts added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Oct 15, 2025
@graouts
Animation not updated when Container Query units are used
1fab171 
https://bugs.webkit.org/show_bug.cgi?id=299952
rdar://162194744

Reviewed by Antti Koivisto and Anne van Kesteren.

Recompute keyframes when container query units are used for all types of animations,
not just CSS Animations.

* LayoutTests/imported/w3c/web-platform-tests/css/css-conditional/container-queries/container-units-animation-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-conditional/container-queries/container-units-animation.html:
* Source/WebCore/animation/CSSAnimation.cpp:
(WebCore::CSSAnimation::keyframesRuleDidChange):
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::recomputeKeyframesAtNextOpportunity):
(WebCore::KeyframeEffect::keyframesRuleDidChange): Deleted.
* Source/WebCore/animation/KeyframeEffect.h:
* Source/WebCore/style/Styleable.cpp:
(WebCore::Styleable::queryContainerDidChange const):

Canonical link: https://commits.webkit.org/301584@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Animation-not-updated-when-Container-Query-units-are-used branch from 142e675 to 1fab171 Compare October 15, 2025 22:10
@webkit-commit-queue
Copy link
Collaborator

Committed 301584@main (1fab171): https://commits.webkit.org/301584@main

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

@webkit-commit-queue webkit-commit-queue merged commit 1fab171 into WebKit:main Oct 15, 2025
@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 Oct 15, 2025
@graouts graouts deleted the eng/Animation-not-updated-when-Container-Query-units-are-used branch October 16, 2025 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@annevk annevk annevk approved these changes

@anttijk anttijk anttijk approved these changes

@nt1m nt1m Awaiting requested review from nt1m

@heycam heycam Awaiting requested review from heycam

Assignees

@graouts graouts

Labels

Animations Bugs related to CSS + SVG animations and transitions

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@graouts @webkit-early-warning-system @webkit-commit-queue @annevk @anttijk @webkit-ews-buildbot