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

[css-view-transitions-1] Should view transition names be tree scoped? #10145

Closed
khushalsagar opened this issue Mar 26, 2024 · 3 comments
Closed

Comments

@khushalsagar
Copy link
Member

The spec currently traverses all DOM elements to look for view transition names, see text here. This should probably be tree scoped similar to other CSS naming conventions like scroll timeline and I'm guessing anchor positioning (@tabatkins ?).

@nt1m fyi.

@khushalsagar khushalsagar added Agenda+ css-view-transitions-1 View Transitions; Bugs only labels Mar 26, 2024
@noamr
Copy link
Collaborator

noamr commented Mar 26, 2024

+1 to view transition names being tree-scoped. The implication of the current spec/implementation is that custom elements that are supposed to be encapsulated can animate each other's internals if they know the internal vt-names.

@nt1m
Copy link
Member

nt1m commented Mar 26, 2024

More work in the implementation but makes sense to me.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-view-transitions-1] Should view transition names be tree scoped?, and agreed to the following:

  • RESOLVED: view-transition-name lookup is tree-scoped
The full IRC log of that discussion <fantasai> khush: Spec currently traverses all DOM elements, including those in shadow DOM
<fantasai> khush: Suggestion is to limit to tree-scoped
<TabAtkins> +1 to this
<fantasai> khush: so this would exclude things in shadow DOM
<fantasai> astearns: I've written some tests for tree-scoping, btw, and interop is terrible
<fantasai> khush: Are you talking about other features that are similar?
<fantasai> astearns: Yeah, I did @Property rules and @xxx rules, which are supposed to be tree-scoped, and they're not handled correctly
<astearns> zakim, open queue
<Zakim> ok, astearns, the speaker queue is open
<fantasai> astearns: so probably some other things are also broken
<emilio> fantasai: two things, khush does this mean that for v-t that you'll never be doing transitions on anything in the shadow dom?
<emilio> khush: yeah this means that you couldn't use something inside the shadow dom as an independent thing
<kizu> q+
<emilio> ... scoped transitions would allow you to do that
<emilio> ... but only as a unit inside the DOM
<emilio> fantasai: to alan, there's two different kinds of scoping, at-rules and name defining
<emilio> ... those are quite different mechanism
<astearns> ack kizu
<fantasai> s/defining/defining on elements/
<emilio> ... I'd expect less issues with things like timeline scopes and so on
<fantasai> kizu: Wrt other scoping things, we also have timeline-scope and anchor-name
<fantasai> kizu: Might want to have a way to expose these names, since might have cases where you want to expose
<khush> q+
<fantasai> kizu: maybe this is a more general issue that we need to handle in CSS, and have it work consistently
<astearns> ack khush
<TabAtkins> +1, we need a general mechanic for weakening shadow encapsulation
<TabAtkins> But we should be consistent for now
<fantasai> khush: kizu's point reminds me of ::part(), could use CSS to expose names from within shadow DOM outside it
<fantasai> khush: maybe if you expose the element could allow it to match
<fantasai> khush: could make it more complicated to implement
<fantasai> khush: we already have things in the platform that allow such information, in those cases do you want naming in CSS to also be exposed?
<fantasai> astearns: That would be a separate feature, though. This is for default behavior for view transitions
<fantasai> khush: Does it make sense to make this resolution cover other cases also? Have a general principle
<fantasai> astearns: I think a separate issue on general mechanism, because we do want to consider them all
<fantasai> astearns: don't want to have a specific escape route just for VT
<TabAtkins> Agree, separate issues
<fantasai> RESOLVED: view-transition-name lookup is tree-scoped

nt1m added a commit to nt1m/WebKit that referenced this issue May 13, 2024
https://bugs.webkit.org/show_bug.cgi?id=273883
rdar://127995859

Reviewed by NOBODY (OOPS!).

Corresponding spec issue: w3c/csswg-drafts#10145
Corresponding spec PR: w3c/csswg-drafts#10306

This ignores view-transition names from shadow DOM, given the pseudo-elements are linked to the document element,
exposing shadow DOM names to the view transition pseudo-elements would be a violation of shadow DOM principles.

We don't check for the tree scope directly in `forEachRendererInPaintOrder` because a shadow DOM element might have
flat tree descendants that have an outer tree scope.

We also use the `Styleable`'s element instead of `RenderElement`'s element because for pseudo-elements we want to consider
the associated element's tree scope.

* LayoutTests/TestExpectations:
* Source/WebCore/dom/ViewTransition.cpp:
(WebCore::ViewTransition::captureOldState):
(WebCore::ViewTransition::captureNewState):
nt1m added a commit to nt1m/WebKit that referenced this issue May 13, 2024
https://bugs.webkit.org/show_bug.cgi?id=273883
rdar://127995859

Reviewed by NOBODY (OOPS!).

Corresponding spec issue: w3c/csswg-drafts#10145
Corresponding spec PR: w3c/csswg-drafts#10306

This ignores view-transition names from shadow DOM, given the pseudo-elements are linked to the document element,
exposing shadow DOM names to the view transition pseudo-elements would be a violation of shadow DOM principles.

We don't check for the tree scope directly in `forEachRendererInPaintOrder` because a shadow DOM element might have
flat tree descendants that have an outer tree scope.

We also use the `Styleable`'s element instead of `RenderElement`'s element because for pseudo-elements we want to consider
the associated element's tree scope.

* LayoutTests/TestExpectations:
* LayoutTests/platform/glib/TestExpectations:
* Source/WebCore/dom/ViewTransition.cpp:
(WebCore::ViewTransition::captureOldState):
(WebCore::ViewTransition::captureNewState):
webkit-commit-queue pushed a commit to nt1m/WebKit that referenced this issue May 13, 2024
https://bugs.webkit.org/show_bug.cgi?id=273883
rdar://127995859

Reviewed by Antti Koivisto.

Corresponding spec issue: w3c/csswg-drafts#10145
Corresponding spec PR: w3c/csswg-drafts#10306

This ignores view-transition names from shadow DOM, given the pseudo-elements are linked to the document element,
exposing shadow DOM names to the view transition pseudo-elements would be a violation of shadow DOM principles.

We don't check for the tree scope directly in `forEachRendererInPaintOrder` because a shadow DOM element might have
flat tree descendants that have an outer tree scope.

We also use the `Styleable`'s element instead of `RenderElement`'s element because for pseudo-elements we want to consider
the associated element's tree scope.

* LayoutTests/TestExpectations:
* LayoutTests/platform/glib/TestExpectations:
* Source/WebCore/dom/ViewTransition.cpp:
(WebCore::ViewTransition::captureOldState):
(WebCore::ViewTransition::captureNewState):

Canonical link: https://commits.webkit.org/278704@main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants