csswg-drafts icon indicating copy to clipboard operation
csswg-drafts copied to clipboard

[css-anchor] anchor-name should not leak out of a shadow tree

Open lilles opened this issue 2 years ago • 3 comments

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.

lilles avatar Oct 19 '22 11:10 lilles

I guess tree scoped names/references could be used here?

lilles avatar Oct 19 '22 11:10 lilles

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.

tabatkins avatar Oct 19 '22 21:10 tabatkins

Done, please let me know if there's anything else that you think needs to be said here.

tabatkins avatar Oct 19 '22 22:10 tabatkins

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?

lilles avatar Oct 20 '22 10:10 lilles

Ah, hm, you're right. My algo is wrong, it should be allowed for light dom to set ::part(foo) { position: absolute; top: anchor(--target top); }, with anchor-name: --target; set in the light dom (and without fear of accidentally hitting an anchor-name: --target in the shadow). I'll need to tweak things a little to rely on the tree-scoped bit more explicitly.

tabatkins avatar Oct 20 '22 16:10 tabatkins

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.

tabatkins avatar Oct 20 '22 16:10 tabatkins

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 anchor(--foo) in a shadow tree should be able to find an anchor-name: --foo in the light DOM (but not in reverse). See the last paragraph of the tree-scoped references section.

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.

tabatkins avatar Oct 21 '22 15:10 tabatkins

So the currently specced behavior is just a little too restrictive

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)

xiaochengh avatar Oct 21 '22 18:10 xiaochengh

Do you mean the currently specced behavior is the expected behavior or not?

Not. Per the Scoping spec, we should expect anchor(--foo ...) in a shadow tree to be capable of seeing anchor-name: --foo in the light tree, if nothing in the shadow tree defines that name already.

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.

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.

tabatkins avatar Oct 21 '22 19:10 tabatkins

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.

Agreed that this should be achieved, but inheritance is not needed for this purpose. We can define anchor names in :host and ::part rules.

Do we have cases where :host and ::part don't suffice, or other scenarios that require inheritance?

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.

xiaochengh avatar Oct 21 '22 21:10 xiaochengh

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.

lilles avatar Oct 21 '22 21:10 lilles

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 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.

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.

tabatkins avatar Oct 21 '22 21:10 tabatkins

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.

lilles avatar Oct 21 '22 21:10 lilles

From my experience dealing with @keyframes, @counter-style and @font-face, name inheritance is quite complicated to support.

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.

xiaochengh avatar Oct 21 '22 22:10 xiaochengh

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.

What happens if you set container-name on a ::part()? (Given the way CQ and container-name interact, I think the answer should be "literally nothing happens".

I'll +1 to leaving this spec as-is and modify Scoping instead.

Your arguments are pretty reasonable. K, I'll leave the spec as-is and modify the advice in Scoping a bit.

tabatkins avatar Oct 21 '22 22:10 tabatkins

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.

What happens if you set container-name on a ::part()? (Given the way CQ and container-name interact, I think the answer should be "literally nothing happens".

I don't understand what you mean. Do you have an example?

lilles avatar Oct 24 '22 16:10 lilles

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?

lilles avatar Oct 24 '22 17:10 lilles

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 @container foo is a tree-scoped reference and container-name is a tree-scoped name, so the CQ inside the shadow resolves against the host element, while the one in the light resolves against the part=foo element. Anything else exposes internal shadow details.

(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.)

tabatkins avatar Oct 25 '22 20:10 tabatkins

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?

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.

Then if the outer page does

@container shadow (width = 150px) {
  #host::part(bar) { color: blue; }
}

does it match?

Yes, the container queries try to match the same container in both cases.

I think the most reasonable answer is that both CQs should be true, referring to different container elements, because the @container foo is a tree-scoped reference and container-name is a tree-scoped name, so the CQ inside the shadow resolves against the host element, while the one in the light resolves against the part=foo element. Anything else exposes internal shadow details.

That might be the best behavior, but the spec does not mention container-name as a tree-scoped name at all.

(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.)

We should open an issue against the css-contain spec for this.

lilles avatar Nov 08 '22 13:11 lilles

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.

lilles avatar Nov 08 '22 14:11 lilles

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.

lilles avatar Nov 08 '22 18:11 lilles

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: https://github.com/w3c/csswg-drafts/issues/5984#issuecomment-980694443

lilles avatar Nov 08 '22 20:11 lilles

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.

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:

Container names don't need to be tree-scoped; they're not globally registered/visible, but are instead solely looked up via an ancestor search.

If we've changed our mind on that, should also change the spec. :-)

andruud avatar Nov 28 '22 10:11 andruud

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 ::part() in a CQ won't see any containers on elements in the shadow (which is good); instead it'll start looking at the host element. (The ::part() will be able to see a container name set on the host element from within the shadow, tho, so there's still some shadow-leakage right at the boundary. But boundary leakage on the host is unavoidable so it's okay.) (And you can set a container-name using ::part(), which'll be visible to CQs inside the shadow, so there's leakage the other way too. But since light-DOM styles can't use a container-name set on a ::part(), there's no reason to ever set that property except to purposely screw with a shadow, so that's probably okay.)

My previous example's behavior under the 5984 decision

So 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.

tabatkins avatar Nov 29 '22 22:11 tabatkins

The ::part() will be able to see a container name set on the host element from within the shadow, tho, so there's still some shadow-leakage right at the boundary. But boundary leakage on the host is unavoidable so it's okay.

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?

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.

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.

andruud avatar Nov 30 '22 10:11 andruud

that is avoidable if container names were tree-scoped?

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 ::part(foo) { container-name: foo; } from messing with the shadow content, too, tho as I note there's no use-case for doing so right now except messing with the shadow so that's probably not very important.

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).

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.

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.

tabatkins avatar Nov 30 '22 15:11 tabatkins