[css-view-transitions-1] Fix tree-scoping from element-based to name-based. by noamr · Pull Request #10528 · w3c/csswg-drafts

noamr · 2024-07-04T11:34:46Z

The previous fix for using the tree context made it inconsistent with how shadow DOM styling works for things like anchor positioning, and made it so that e.g. ::part cannot set a view-transition-name.

Refactored to make the view-transition-name a tree-scoped name, rather than check the tree context of the element.

This aligns better with the existing resolution and should not require a new resolution.

Closes #10145

… being name-based. The previous fix for using the tree context made it inconsistent with how shadow DOM styling works for things like anchor positioning, and made it so that e.g. `::part` cannot set a `view-transition-name`. Refactored to make the `view-transition-name` a tree-scoped name, rather than check the tree context of the element. Closes w3c#10145

noamr · 2024-07-04T11:35:50Z

@nt1m can you check if this sits well with you, and if it requires changes to the WebKit implementation?
If you or @fantasai thinks this needs an additional CSSWG resolution I'm happy to add it to the agenda.

khushalsagar

Add a line to the appendix for changes below. LGTM otherwise. Thanks!

noamr · 2024-07-04T15:58:08Z

Add a line to the appendix for changes below. LGTM otherwise. Thanks!

Done

See spec PR: w3c/csswg-drafts#10528 Using a new runtime flag so that it can be backported to M127. Bug: 349653208 Change-Id: Ifce6ee159ca44c5d8a54739ef050354eed5acf22

noamr · 2024-07-04T18:52:15Z

@nt1m can you check if this sits well with you, and if it requires changes to the WebKit implementation? If you or @fantasai thinks this needs an additional CSSWG resolution I'm happy to add it to the agenda.

WPT: web-platform-tests/wpt#47006

nt1m · 2024-07-04T22:41:40Z

Not sure the spec change is worded quite right, if you have:

<style>
::part(party) {
  view-transition-name: document-scoped;
}
</style>
#shadow-root
    <style>
      div {
        width: 100px;
        height: 100px;
        view-transition-name: shadow-scoped !important;
      }
    </style>
    <div part="party"></div>

I expect the effective view transition name of the <div> to be shadow-scoped, and document-scoped being ignored.

Since shadow-scoped is scoped to the shadow root and document-scoped ignored, the <div> wouldn't participate in the view transition.

The spec change suggests otherwise, although I think the Chromium change is correct.

Anyway it would be good to add coverage for that case + capturing ::before/::backdrop pseudo-elements (where we should use the originating element's scope) in those situations as well.

nt1m · 2024-07-05T04:17:59Z

Fwiw, the change is fairly easy to adopt on WebKit's side: WebKit/WebKit#30501

noamr · 2024-07-05T08:28:38Z

Not sure the spec change is worded quite right, if you have:
<style>
::part(party) {
  view-transition-name: document-scoped;
}
</style>
#shadow-root
    <style>
      div {
        width: 100px;
        height: 100px;
        view-transition-name: shadow-scoped !important;
      }
    </style>
    <div part="party"></div>
I expect the effective view transition name of the <div> to be shadow-scoped, and document-scoped being ignored.

Since shadow-scoped is scoped to the shadow root and document-scoped ignored, the <div> wouldn't participate in the view transition.

Hmm right, so the wording should be something along the line of the implementation, where we get the computed value for view-transition-name and check its associated context. Will revise and add a WPT for this (and for the other cases). Thanks!

nt1m

New version looks good, thanks!

See spec PR: w3c/csswg-drafts#10528 Using a new runtime flag so that it can be backported to M127. Bug: 349653208 Change-Id: Ifce6ee159ca44c5d8a54739ef050354eed5acf22

See spec PR: w3c/csswg-drafts#10528 Using a new runtime flag so that it can be backported to M127. Bug: 349653208 Change-Id: Ifce6ee159ca44c5d8a54739ef050354eed5acf22 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5678884 Commit-Queue: Noam Rosenthal <nrosenthal@chromium.org> Reviewed-by: Khushal Sagar <khushalsagar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1323734}

https://bugs.webkit.org/show_bug.cgi?id=276236 rdar://131139528 Reviewed by Matt Woodrow. Follow w3c/csswg-drafts#10528 * LayoutTests/TestExpectations: * Source/WebCore/dom/ViewTransition.cpp: (WebCore::effectiveViewTransitionName): (WebCore::ViewTransition::captureOldState): (WebCore::ViewTransition::captureNewState): (WebCore::ViewTransition::documentElementIsCaptured const): Canonical link: https://commits.webkit.org/280701@main

See spec PR: w3c/csswg-drafts#10528 Using a new runtime flag so that it can be backported to M127. (cherry picked from commit db489af) Bug: 349653208 Change-Id: Ifce6ee159ca44c5d8a54739ef050354eed5acf22 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5678884 Commit-Queue: Noam Rosenthal <nrosenthal@chromium.org> Reviewed-by: Khushal Sagar <khushalsagar@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1323734} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5679292 Auto-Submit: Noam Rosenthal <nrosenthal@chromium.org> Commit-Queue: Khushal Sagar <khushalsagar@chromium.org> Cr-Commit-Position: refs/branch-heads/6533@{#1067} Cr-Branched-From: 7e0b87e-refs/heads/main@{#1313161}

…coped rather then nodes, a=testonly Automatic update from web-platform-tests View transitions: names should be tree-scoped rather then nodes See spec PR: w3c/csswg-drafts#10528 Using a new runtime flag so that it can be backported to M127. Bug: 349653208 Change-Id: Ifce6ee159ca44c5d8a54739ef050354eed5acf22 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5678884 Commit-Queue: Noam Rosenthal <nrosenthal@chromium.org> Reviewed-by: Khushal Sagar <khushalsagar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1323734} -- wpt-commits: bf29ed53ac66890035b47a75d7d84df15245a411 wpt-pr: 47006

See spec PR: w3c/csswg-drafts#10528 Using a new runtime flag so that it can be backported to M127. Bug: 349653208 Change-Id: Ifce6ee159ca44c5d8a54739ef050354eed5acf22 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5678884 Commit-Queue: Noam Rosenthal <nrosenthal@chromium.org> Reviewed-by: Khushal Sagar <khushalsagar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1323734}

…coped rather then nodes, a=testonly Automatic update from web-platform-tests View transitions: names should be tree-scoped rather then nodes See spec PR: w3c/csswg-drafts#10528 Using a new runtime flag so that it can be backported to M127. Bug: 349653208 Change-Id: Ifce6ee159ca44c5d8a54739ef050354eed5acf22 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5678884 Commit-Queue: Noam Rosenthal <nrosenthal@chromium.org> Reviewed-by: Khushal Sagar <khushalsagar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1323734} -- wpt-commits: bf29ed53ac66890035b47a75d7d84df15245a411 wpt-pr: 47006

noamr requested review from khushalsagar, nt1m and vmpstr July 4, 2024 11:34

khushalsagar approved these changes Jul 4, 2024

View reviewed changes

Add changelog entry

ce070b8

chromium-wpt-export-bot mentioned this pull request Jul 4, 2024

View transitions: names should be tree-scoped rather then nodes web-platform-tests/wpt#47006

Merged

nt1m mentioned this pull request Jul 4, 2024

[view-transitions] Check tree scopes against names instead of elements WebKit/WebKit#30501

Merged

Modifiy vt-name algorithm

6a0aeec

nt1m approved these changes Jul 5, 2024

View reviewed changes

noamr merged commit eaf34b1 into w3c:main Jul 5, 2024

noamr deleted the vt-shadow branch July 5, 2024 09:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[css-view-transitions-1] Fix tree-scoping from element-based to name-based.#10528

[css-view-transitions-1] Fix tree-scoping from element-based to name-based.#10528
noamr merged 3 commits intow3c:mainfrom
noamr:vt-shadow

noamr commented Jul 4, 2024 •

edited

Loading

Uh oh!

noamr commented Jul 4, 2024

Uh oh!

khushalsagar left a comment

Uh oh!

noamr commented Jul 4, 2024

Uh oh!

noamr commented Jul 4, 2024

Uh oh!

nt1m commented Jul 4, 2024

Uh oh!

nt1m commented Jul 5, 2024

Uh oh!

noamr commented Jul 5, 2024

Uh oh!

nt1m left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

noamr commented Jul 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

noamr commented Jul 4, 2024

Uh oh!

khushalsagar left a comment

Choose a reason for hiding this comment

Uh oh!

noamr commented Jul 4, 2024

Uh oh!

noamr commented Jul 4, 2024

Uh oh!

nt1m commented Jul 4, 2024

Uh oh!

nt1m commented Jul 5, 2024

Uh oh!

noamr commented Jul 5, 2024

Uh oh!

nt1m left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

noamr commented Jul 4, 2024 •

edited

Loading