-
Notifications
You must be signed in to change notification settings - Fork 683
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-anchor] anchor-name should not leak out of a shadow tree #7916
Comments
I guess tree scoped names/references could be used here? |
The current implementation leaks anchor-name out of shadow trees. That is probably not what we want. See w3c/csswg-drafts#7916 Change-Id: I2f79f68b5595749a6802b753395cb7ee535c5778
Hm, yeah that needs clarification. Scoping to the document tree is indeed what you want, so the anchor-name is a tree-scoped reference. Otherwise two components would have to defensively uniquify their names to make sure they didn't overlap, which sucks. |
Done, please let me know if there's anything else that you think needs to be said here. |
Tree-scoped names generally capture the tree-scope of the element where the property is specified, so if it's inherited it is still associated with the tree-scope of the element it is ultimately inherited from. The new spec text says it compares the tree-scopes of the elements the computed value is on, which is slightly different. Is that intentional? That is, what should happen if an anchor-name is explicitly inherit through a shadow root? |
The current implementation leaks anchor-name out of shadow trees. That is probably not what we want. See w3c/csswg-drafts#7916 Change-Id: I2f79f68b5595749a6802b753395cb7ee535c5778
The current implementation leaks anchor-name out of shadow trees. That is probably not what we want. See w3c/csswg-drafts#7916 Change-Id: I2f79f68b5595749a6802b753395cb7ee535c5778
The current implementation leaks anchor-name out of shadow trees. That is probably not what we want. See w3c/csswg-drafts#7916 Change-Id: I2f79f68b5595749a6802b753395cb7ee535c5778
The current implementation leaks anchor-name out of shadow trees. That is not according to spec. See also w3c/csswg-drafts#7916 Bug: 1376917 Change-Id: I2f79f68b5595749a6802b753395cb7ee535c5778
Ah, hm, you're right. My algo is wrong, it should be allowed for light dom to set |
In other words, I need to check that the tree-scoped reference and the tree-scoped name are scoped to the same tree, not that the querying element and target element are in the same tree. |
The current implementation leaks anchor-name out of shadow trees. That is not according to spec. See also w3c/csswg-drafts#7916 Bug: 1376917 Change-Id: I2f79f68b5595749a6802b753395cb7ee535c5778 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3966354 Reviewed-by: Xiaocheng Hu <[email protected]> Commit-Queue: Rune Lillesveen <[email protected]> Cr-Commit-Position: refs/heads/main@{#1061842}
The current implementation leaks anchor-name out of shadow trees. That is not according to spec. See also w3c/csswg-drafts#7916 Bug: 1376917 Change-Id: I2f79f68b5595749a6802b753395cb7ee535c5778 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3966354 Reviewed-by: Xiaocheng Hu <[email protected]> Commit-Queue: Rune Lillesveen <[email protected]> Cr-Commit-Position: refs/heads/main@{#1061842}
The current implementation leaks anchor-name out of shadow trees. That is not according to spec. See also w3c/csswg-drafts#7916 Bug: 1376917 Change-Id: I2f79f68b5595749a6802b753395cb7ee535c5778 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3966354 Reviewed-by: Xiaocheng Hu <[email protected]> Commit-Queue: Rune Lillesveen <[email protected]> Cr-Commit-Position: refs/heads/main@{#1061842}
Hm, I think it's still wrong actually. References are meant to search their own tree first, then search their host tree if they didn't find anything, etc, so an So the currently specced behavior is just a little too restrictive. I probably need to define a callable algo in Scoping that makes this Just Work Correctly, considering I wrote the Scoping spec and still did this wrong. |
Do you mean the currently specced behavior is the expected behavior or not? I think the current behavior is already current, and we just need to restrict the current tree-scoped reference spec's inheritance behavior to names defined by at-rules only. My understanding of that spec is: it was designed with only at-rules in mind. In particular, the "inheritance" behavior makes sense to me only for at-rules. For other names, it seems better to just restrict them to the same tree scope. (Btw, I think we have the same issue for counter names) |
Not. Per the Scoping spec, we should expect
It was def written with at-rules foremost in mind, but I think the reasoning for the "inheritance" still makes decent sense - it means components can anchor to things outside of themselves, but without polluting namespaces that don't expect it. Like, it'd probably be fine if we ended up restricting tree-scoped names created by properties rather than at-rules to be same-tree only, but I don't think there's a good reason to do so from a theoretical standpoint. |
Agreed that this should be achieved, but inheritance is not needed for this purpose. We can define anchor names in Do we have cases where If not, I think inheritance is doing more risk than good as it breaks shadow DOM encapsulation, and it's complicated to implement without good use cases. |
I find it a bit hard to wrap my head around the inheritance part since both ends can be inherited across trees. You can explicitly inherit an anchor-name from a slot or via a shadow root, but you could also inherit the inset property anchor() function across a shadow boundary. |
Yup, just remember that the important part is that we care about the trees that the styles come from, not what trees the elements are in that actually use the styles. Then the "name inheritance" is a convenience feature, intentionally leaking names defined in one tree into descendant trees, taking metaphorical advantage of the existing info-leak that inheritance represents.
If it's a significant complication I'm okay with dropping it and leaving the spec as is (and modifying the advice in Scoping to match up). The cases are reasonably minimal, yeah. |
This is similar to container-name for container queries, right? We don't make that one tree scoped? If you inherit container-name explicitly from a slot to a slotted element, we still find that container with a query in the slotted element's tree. |
From my experience dealing with I don't really see it as a convenience feature, but a workaround for backward-compatibility since browser support for tree-scoped names (for existing at-rules) have been always bad/none, and people have been relying on global at-rules. Also there are some other new features that explicitly reject inheritance for better encapsulation, like scoped custom element registries. I'll +1 to leaving this spec as-is and modify Scoping instead. |
What happens if you set
Your arguments are pretty reasonable. K, I'll leave the spec as-is and modify the advice in Scoping a bit. |
I don't understand what you mean. Do you have an example? |
So, my point about container queries is that the container queries spec is not making container-name a tree scoped name which means the text below is red (in both Safari and Chrome - I've checked Safari by using a non-declarative shadow dom): <!doctype html>
<style>
#slotted {
width: 200px;
container-type: inline-size;
}
@container shadow (width = 200px) {
span { color: red; }
}
</style>
<div id="host">
<template shadowroot="open">
<style>
::slotted(#slotted) {
container-name: shadow;
}
</style>
<slot></slot>
</template>
<div id="slotted">
<span>Red?</span>
</div>
</div> I guess this can be seen as a leak and an argument that container-name should be a tree-scoped name. My point is that anchor-name and container-name are so similar they should behave the same? |
My example would be something like, uh: <div id="host">
<template shadowroot="open">
<div part=foo>
<div id=child part=bar></div>
</div>
<style>
:host {
container-name: shadow;
container-type: inline-size;
width: 200px;
}
#child { color: red; }
@container shadow (width = 200px) {
#child { color: green; }
}
</style>
</template>
</div>
<style>
#host::part(foo) {
container-name: shadow;
container-type: inline-size:
width: 150px;
}
</style> That is, the shadow establishes some CQ container and queries it, but a light-DOM stylesheet sets the same container name/type on a different element. Does this work, intercepting the CQ and making it evaluate false? Then if the outer page does @container shadow (width = 150px) {
#host::part(bar) { color: blue; }
} does it match? I think the most reasonable answer is that both CQs should be true, referring to different container elements, because the (Letting the light-dom CQ resolve to anything inside the shadow also reveals internal details, namely that part=foo is an ancestor of part=bar, but I think that's somewhat unavoidable.) |
…ame across trees, a=testonly Automatic update from web-platform-tests [css-anchor] Add some tests for anchor-name across trees The current implementation leaks anchor-name out of shadow trees. That is not according to spec. See also w3c/csswg-drafts#7916 Bug: 1376917 Change-Id: I2f79f68b5595749a6802b753395cb7ee535c5778 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3966354 Reviewed-by: Xiaocheng Hu <[email protected]> Commit-Queue: Rune Lillesveen <[email protected]> Cr-Commit-Position: refs/heads/main@{#1061842} -- wpt-commits: f249b439de786da11083591131a5bd6729740e11 wpt-pr: 36551
Yes, the part rule will make this case go red because we don't use tree-scoped names for container-name because the spec doesn't say anything about it, we just look up shadow-including ancestors of the element being matched.
Yes, the container queries try to match the same container in both cases.
That might be the best behavior, but the spec does not mention container-name as a tree-scoped name at all.
We should open an issue against the css-contain spec for this. |
What makes this super complex in the case of anchor-name and container-name compared to at-rules like keyframes is that we have values both on the name and reference side which can be associated with a different tree scope than the scope of the element that the computed value is associated with. |
Hm, so given that css-scoping-1 says names/references must be tree-scoped names, perhaps the anchor positioning and container queries specs don't strictly need to say anything about it. |
I'm utterly confused about the container queries case and need to go back to study it. There is the pseudo element thing affecting ::part() that I didn't think about in my comments above: #5984 (comment) |
…ame across trees, a=testonly Automatic update from web-platform-tests [css-anchor] Add some tests for anchor-name across trees The current implementation leaks anchor-name out of shadow trees. That is not according to spec. See also w3c/csswg-drafts#7916 Bug: 1376917 Change-Id: I2f79f68b5595749a6802b753395cb7ee535c5778 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3966354 Reviewed-by: Xiaocheng Hu <[email protected]> Commit-Queue: Rune Lillesveen <[email protected]> Cr-Commit-Position: refs/heads/main@{#1061842} -- wpt-commits: f249b439de786da11083591131a5bd6729740e11 wpt-pr: 36551
Some spec, somewhere, needs clarification, because the current text in css-scoping-1 can't easily be applied to name-defining things that aren't "global" . @tabatkins seems to have agreed with that earlier:
If we've changed our mind on that, should also change the spec. :-) |
Ah, thanks for reminding me about #5984! So, argh, this is tough. In 5984 we decided on using a shadow-including ancestor search, and to jump straight to a pseudo's originating element when starting that search. This means that a My previous example's behavior under the 5984 decisionSo in my example, the `part=foo` element in the shadow *does* get a `container-name` from the light DOM styles, which is visible to the CQ in the shadow (possibly inadvertently). This causes the CQ in the shadow to fail, since the nearest "shadow" container for #child is 150px wide, not 200px. The subsequent light-DOM CQ *also* fails, since it starts searching at the host element, not the part in the shadow tree, and thus sees a "shadow" container that is 200px wide, not 150px. But this worked because we're doing this stuff at selector-resolution time, where we know that we're resolving a pseudo-element reference. Is it still possible to do when the search is initiated due to a property value rather than a selector? That's well after we're done with Selectors. |
You are probably saying it's unavoidable more generally, but at least for the specific case you mention there, that is avoidable if container names were tree-scoped?
I think you're right. The fact that a value "originates" from a pseudo-element-rule is long gone by the time we're ready to apply the effects of that value. We'd have to invent some new concept which would retain that fact. And at that point it seems preferable to re-use tree-scoped references. Not that I love tree-scoped references so much (they're expensive: we have to hold an entire TreeScope pointer next to each name, and lookup of things generally becomes more complicated), but trying to apply the 5984 CQ behavior to situations that can't be resolved selector-matching-time (e.g. anchor names, timeline names) seems like a worse alternative in practice. |
Right, we could additionally add tree-scoping and avoid some additional leakage. But also some amount of boundary leakage is still unavoidable, since if both light and shadow set a name on the host one of them will win and cause the other to lose. ^_^ Tree-scoped would prevent If they were tree-scoped, my example would result in the CQ in the shadow succeeding (it would skip the container-name set from the light and find the expected CQ on the host) and the CQ in the light failing (it starts its search for container names on the host, and fails to find any container names at all).
Ok, thanks for confirming my intuition. I'll get something written more explicitly in Scoping defining the behavior of tree-scoped names established by properties. |
The spec currently says: "find the first element el in tree order". It should be more specific about which tree.
The current implementation in Chromium looks walks the box tree, essentially the flat tree, which means it is leaking anchor-names out of shadow trees.
The text was updated successfully, but these errors were encountered: