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

:empty selector with animation not working properly #25583

Conversation

graouts
Copy link
Contributor

@graouts graouts commented Mar 7, 2024

319ecb9

:empty selector with animation not working properly
https://bugs.webkit.org/show_bug.cgi?id=269051
rdar://122838142

Reviewed by Antti Koivisto.

If an element is targeted by an animation, it may cause style updates with an `AnimationOnly` resolution type.
When `Style::TreeResolver::resolveElement()` is called for that element, `resetStyleRelations()` is called on
that element first under `Style::TreeResolver::resolveComposedTree()` while iterating through the composed tree.

If an `:empty` pseudo-class rule matches that element, that call to `resetStyleRelations()` will remove the
`StyleAffectedByEmpty` flag on that element, but it will not be recomputed under `resolveElement()` because when
`styleForStyleable()` is called the `AnimationOnly` resolution type will mean that we clone the cached last style
resolution style to return the `ResolvedStyle` value, clear of any relations.

We now avoid calling `resetStyleRelations()` if the resolution type is `AnimationOnly`, ensuring that relations
are preserved throughout animation updates.

* LayoutTests/imported/w3c/web-platform-tests/css/css-animations/empty-pseudo-class-with-animation-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-animations/empty-pseudo-class-with-animation.html: Added.
* Source/WebCore/style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::resolveComposedTree):

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

1923f33

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

@graouts graouts self-assigned this Mar 7, 2024
@graouts graouts added the CSS Cascading Style Sheets implementation label Mar 7, 2024
@graouts graouts requested a review from anttijk March 7, 2024 17:36
@graouts graouts added the Animations Bugs related to CSS + SVG animations and transitions label Mar 7, 2024
@graouts graouts added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Mar 8, 2024
https://bugs.webkit.org/show_bug.cgi?id=269051
rdar://122838142

Reviewed by Antti Koivisto.

If an element is targeted by an animation, it may cause style updates with an `AnimationOnly` resolution type.
When `Style::TreeResolver::resolveElement()` is called for that element, `resetStyleRelations()` is called on
that element first under `Style::TreeResolver::resolveComposedTree()` while iterating through the composed tree.

If an `:empty` pseudo-class rule matches that element, that call to `resetStyleRelations()` will remove the
`StyleAffectedByEmpty` flag on that element, but it will not be recomputed under `resolveElement()` because when
`styleForStyleable()` is called the `AnimationOnly` resolution type will mean that we clone the cached last style
resolution style to return the `ResolvedStyle` value, clear of any relations.

We now avoid calling `resetStyleRelations()` if the resolution type is `AnimationOnly`, ensuring that relations
are preserved throughout animation updates.

* LayoutTests/imported/w3c/web-platform-tests/css/css-animations/empty-pseudo-class-with-animation-expected.txt: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-animations/empty-pseudo-class-with-animation.html: Added.
* Source/WebCore/style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::resolveComposedTree):

Canonical link: https://commits.webkit.org/275832@main
@webkit-commit-queue webkit-commit-queue force-pushed the bug-269051-empty-pseudo-class-and-animations branch from 1923f33 to 319ecb9 Compare March 8, 2024 12:21
@webkit-commit-queue
Copy link
Collaborator

Committed 275832@main (319ecb9): https://commits.webkit.org/275832@main

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

@webkit-commit-queue webkit-commit-queue merged commit 319ecb9 into WebKit:main Mar 8, 2024
@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 Mar 8, 2024
@graouts graouts deleted the bug-269051-empty-pseudo-class-and-animations branch March 8, 2024 12:30
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 CSS Cascading Style Sheets implementation
Projects
None yet
4 participants