Skip to content

[view-transitions] Implement matching for view transition classes #32332

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
Aug 17, 2024
Merged

[view-transitions] Implement matching for view transition classes #32332

merged 1 commit into from
Aug 17, 2024

Conversation

@nt1m
Copy link
Member

@nt1m nt1m commented Aug 16, 2024 •
edited by webkit-early-warning-system
Loading

28a8cb7

[view-transitions] Implement matching for view transition classes
https://bugs.webkit.org/show_bug.cgi?id=278223
rdar://134020027

Reviewed by Darin Adler.

Implements matching as described in:
https://drafts.csswg.org/css-view-transitions-2/#shared-style-with-vt-classes

* LayoutTests/TestExpectations:

Unskip all tests.

* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:

Enable by default, since all WPT pass.

* Source/WebCore/css/CSSSelector.cpp:
(WebCore::operator<<):
* Source/WebCore/css/CSSSelector.h:

Add PossiblyQuotedIdentifier TextStream for easy debugging.

* Source/WebCore/css/SelectorChecker.cpp:
(WebCore::SelectorChecker::checkOne const):
* Source/WebCore/css/SelectorChecker.h:

All classes in need to match in order for the styles to apply.

* Source/WebCore/dom/ViewTransition.cpp:
(WebCore::effectiveViewTransitionClassList):
(WebCore::ViewTransition::captureOldState):
(WebCore::ViewTransition::captureNewState):

Captures the classList as described in https://drafts.csswg.org/css-view-transitions-2/#vt-class-algorithms

* Source/WebCore/dom/ViewTransition.h: Add classList to CapturedElement.

* Source/WebCore/rendering/updating/RenderTreeUpdater.cpp:
(WebCore::pseudoStyleCacheIsInvalid):

Fix a bug in the cache where were passing a null name argument string.

* Source/WebCore/style/ElementRuleCollector.cpp:
(WebCore::Style::classListForNamedViewTransitionPseudoElement):
(WebCore::Style::ElementRuleCollector::ruleMatches):

Pass the classList to the SelectorChecker when checking the selector.

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

3f4800c

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac   🛠 wpe   🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2   🧪 wincairo-tests
✅ 🧪 webkitperl   🧪 ios-wk2 ⏳ 🧪 api-mac   🧪 api-wpe
  🧪 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 ✅ 🛠 jsc-armv7
  🛠 tv ⏳ 🧪 jsc-armv7-tests
  🛠 tv-sim
  🛠 watch
  🛠 watch-sim

@nt1m nt1m requested review from cdumez and rniwa as code owners August 16, 2024 23:13
@nt1m nt1m self-assigned this Aug 16, 2024
@nt1m nt1m added the CSS Cascading Style Sheets implementation label Aug 16, 2024
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Aug 16, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 907f466)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ⏳ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl   🧪 ios-wk2 ⏳ 🧪 api-mac ⏳ 🧪 api-wpe
⏳ 🧪 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 ✅ 🛠 jsc-armv7
  🛠 tv ✅ 🧪 jsc-armv7-tests
  🛠 tv-sim
  🛠 watch
  🛠 watch-sim

darinadler
darinadler approved these changes Aug 16, 2024
View reviewed changes
Copy link
Member

@darinadler darinadler left a comment

Choose a reason for hiding this comment

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

Does this extra data have measurable performance cost? I suppose we’ll find out from our benchmarks.

@nt1m nt1m force-pushed the eng/view-transitions-Implement-matching-for-view-transition-classes branch from 907f466 to 6b7922b Compare August 17, 2024 00:13
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Aug 17, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 6b7922b)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ⏳ 🧪 wpe-wk2   🧪 wincairo-tests
✅ 🧪 webkitperl ⏳ 🧪 ios-wk2 ⏳ 🧪 api-mac ⏳ 🧪 api-wpe
⏳ 🧪 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 ✅ 🛠 jsc-armv7
  🛠 tv   🧪 jsc-armv7-tests
  🛠 tv-sim
  🛠 watch
  🛠 watch-sim

@nt1m nt1m force-pushed the eng/view-transitions-Implement-matching-for-view-transition-classes branch from 6b7922b to 03e724a Compare August 17, 2024 00:17
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Aug 17, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 03e724a)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ⏳ 🧪 wpe-wk2   🧪 wincairo-tests
✅ 🧪 webkitperl ⏳ 🧪 ios-wk2 ⏳ 🧪 api-mac   🧪 api-wpe
⏳ 🧪 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 ✅ 🛠 jsc-armv7
  🛠 tv ✅ 🧪 jsc-armv7-tests
  🛠 tv-sim
  🛠 watch
  🛠 watch-sim

@nt1m nt1m force-pushed the eng/view-transitions-Implement-matching-for-view-transition-classes branch from 03e724a to 5ed54ae Compare August 17, 2024 00:35
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Aug 17, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 5ed54ae)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac   🛠 wpe   🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2   🧪 wincairo-tests
✅ 🧪 webkitperl   🧪 ios-wk2 ⏳ 🧪 api-mac   🧪 api-wpe
⏳ 🧪 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 ✅ 🛠 jsc-armv7
  🛠 tv ⏳ 🧪 jsc-armv7-tests
  🛠 tv-sim
  🛠 watch
  🛠 watch-sim

@nt1m
Copy link
Member Author

nt1m commented Aug 17, 2024

Does this extra data have measurable performance cost? I suppose we’ll find out from our benchmarks.

I would not expect so, this extra data is only populated during a View Transition, and the class list is expected to be small.

@nt1m nt1m force-pushed the eng/view-transitions-Implement-matching-for-view-transition-classes branch from 5ed54ae to 4d780ec Compare August 17, 2024 00:40
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Aug 17, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 4d780ec)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac   🛠 wpe   🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2   🧪 wincairo-tests
✅ 🧪 webkitperl ⏳ 🧪 ios-wk2 ⏳ 🧪 api-mac   🧪 api-wpe
⏳ 🧪 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 ✅ 🛠 jsc-armv7
  🛠 tv ⏳ 🧪 jsc-armv7-tests
  🛠 tv-sim
  🛠 watch
  🛠 watch-sim

darinadler
darinadler approved these changes Aug 17, 2024
View reviewed changes
@nt1m nt1m force-pushed the eng/view-transitions-Implement-matching-for-view-transition-classes branch from 4d780ec to 4c9127e Compare August 17, 2024 00:47
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Aug 17, 2024 •
edited
Loading

EWS run on previous version of this PR (hash 4c9127e)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac   🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2   🧪 wincairo-tests
✅ 🧪 webkitperl   🧪 ios-wk2 ⏳ 🧪 api-mac   🧪 api-wpe
⏳ 🧪 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
  🛠 🧪 merge   🧪 vision-wk2 ✅ 🛠 jsc-armv7
  🛠 tv ⏳ 🧪 jsc-armv7-tests
  🛠 tv-sim
  🛠 watch
  🛠 watch-sim

@nt1m nt1m added merge-queue Applied to send a pull request to merge-queue and removed merge-queue Applied to send a pull request to merge-queue labels Aug 17, 2024
@nt1m nt1m force-pushed the eng/view-transitions-Implement-matching-for-view-transition-classes branch from 4c9127e to bcde8ce Compare August 17, 2024 01:06
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Aug 17, 2024 •
edited
Loading

EWS run on previous version of this PR (hash bcde8ce)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2   🧪 wincairo-tests
✅ 🧪 webkitperl   🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
⏳ 🧪 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
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🛠 jsc-armv7
  🛠 tv ✅ 🧪 jsc-armv7-tests
  🛠 tv-sim
  🛠 watch
loading 🛠 watch-sim

@nt1m nt1m added merge-queue Applied to send a pull request to merge-queue and removed merge-queue Applied to send a pull request to merge-queue labels Aug 17, 2024
@nt1m nt1m changed the title [view-transitions] Implement matching for view transition classes Use interleave() in CSSSelector Aug 17, 2024
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Aug 17, 2024 •
edited
Loading

EWS run on current version of this PR (hash 3f4800c)

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac   🛠 wpe   🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug   🧪 wpe-wk2   🧪 wincairo-tests
✅ 🧪 webkitperl   🧪 ios-wk2 ⏳ 🧪 api-mac   🧪 api-wpe
  🧪 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 ✅ 🛠 jsc-armv7
  🛠 tv ⏳ 🧪 jsc-armv7-tests
  🛠 tv-sim
  🛠 watch
  🛠 watch-sim

@nt1m nt1m changed the title Use interleave() in CSSSelector [view-transitions] Implement matching for view transition classes Aug 17, 2024
@nt1m nt1m force-pushed the eng/view-transitions-Implement-matching-for-view-transition-classes branch from 3f4800c to bcde8ce Compare August 17, 2024 01:29
@nt1m nt1m added the merge-queue Applied to send a pull request to merge-queue label Aug 17, 2024
@nt1m
[view-transitions] Implement matching for view transition classes
28a8cb7 
https://bugs.webkit.org/show_bug.cgi?id=278223
rdar://134020027

Reviewed by Darin Adler.

Implements matching as described in:
https://drafts.csswg.org/css-view-transitions-2/#shared-style-with-vt-classes

* LayoutTests/TestExpectations:

Unskip all tests.

* Source/WTF/Scripts/Preferences/UnifiedWebPreferences.yaml:

Enable by default, since all WPT pass.

* Source/WebCore/css/CSSSelector.cpp:
(WebCore::operator<<):
* Source/WebCore/css/CSSSelector.h:

Add PossiblyQuotedIdentifier TextStream for easy debugging.

* Source/WebCore/css/SelectorChecker.cpp:
(WebCore::SelectorChecker::checkOne const):
* Source/WebCore/css/SelectorChecker.h:

All classes in need to match in order for the styles to apply.

* Source/WebCore/dom/ViewTransition.cpp:
(WebCore::effectiveViewTransitionClassList):
(WebCore::ViewTransition::captureOldState):
(WebCore::ViewTransition::captureNewState):

Captures the classList as described in https://drafts.csswg.org/css-view-transitions-2/#vt-class-algorithms

* Source/WebCore/dom/ViewTransition.h: Add classList to CapturedElement.

* Source/WebCore/rendering/updating/RenderTreeUpdater.cpp:
(WebCore::pseudoStyleCacheIsInvalid):

Fix a bug in the cache where were passing a null name argument string.

* Source/WebCore/style/ElementRuleCollector.cpp:
(WebCore::Style::classListForNamedViewTransitionPseudoElement):
(WebCore::Style::ElementRuleCollector::ruleMatches):

Pass the classList to the SelectorChecker when checking the selector.

Canonical link: https://commits.webkit.org/282383@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/view-transitions-Implement-matching-for-view-transition-classes branch from bcde8ce to 28a8cb7 Compare August 17, 2024 02:21
@webkit-commit-queue
Copy link
Collaborator

webkit-commit-queue commented Aug 17, 2024

Committed 282383@main (28a8cb7): https://commits.webkit.org/282383@main

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

@webkit-commit-queue webkit-commit-queue merged commit 28a8cb7 into WebKit:main Aug 17, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Aug 17, 2024
@nt1m nt1m deleted the eng/view-transitions-Implement-matching-for-view-transition-classes branch August 17, 2024 02:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@darinadler darinadler darinadler approved these changes

@rniwa rniwa Awaiting requested review from rniwa rniwa is a code owner

@cdumez cdumez Awaiting requested review from cdumez cdumez is a code owner

@smfr smfr Awaiting requested review from smfr

@heycam heycam Awaiting requested review from heycam

@anttijk anttijk Awaiting requested review from anttijk

@mattwoodrow mattwoodrow Awaiting requested review from mattwoodrow

Assignees

@nt1m nt1m

Labels

CSS Cascading Style Sheets implementation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nt1m @webkit-early-warning-system @webkit-commit-queue @darinadler