-
Notifications
You must be signed in to change notification settings - Fork 668
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-scoping] Handling global name-defining constructs in shadow trees #1995
Comments
Ojan privately points out that it's very likely the integer is over-engineering, and virtually every case will just be wanting the nearest-local or the global version of a name. You still need the ability for a value to refer to an arbitrary scope for inheritance to work properly, but you don't actually need to be able to specify that reference - in other words, we don't really need syntax for such a thing. This simplifies it considerably. We can just define that the keyword always refers to the globally-defined name, and have |
Alternate argument in favor of the integer: it means the value is locally interpretable at all times. If we make the bare keyword (like Common usage will still be limited to just |
I would argue that the expected behaviour is for the bare reference ( @font-face { font-family: foo; ... }
p { font-family: foo; } The expectation from a developer standpoint is that when using the normal syntax, styles within a ShadowRoot behave like they are in their own global scope, and do not interact with outside styles. There are some cases where alternative syntax is needed to resolve scoping issues (like the I am suggesting this behaviour:
The difference between @tabatkins' suggested behaviour is that the bare reference |
I chose the current behavior for bare keywords to match what I thought I remembered browser behavior was, or at least Chrome's. Per #715, tho, it looks like Safari does the opposite, and treats @Keyframes as local and references to it as referring to the local definition? If we have to swap the bare keyword to be the local version, that's fine. |
(I do think, tho, that the "bare keyword means global" behavior is slightly better aesthetically, as it means that the common case from today doesn't have to invoke the magical "rewrite yourself to refer to the parent scope" behavior; you only get that if you explicitly use |
I think we are actually seeing a different issue here. All browsers with Web Component support use local definitions by default (including Chrome), at least for @Keyframes definitions (I will check @font-face behaviours and report back) but there is an inconsistency in how slotted elements are treated. Chrome uses local definition in the scope where the DOM node the animation is targeting is defined, as per this example: <my-component>
<div id="one"></div>
<::shadow>
<style>
@keyframes some-animation { ... };
div {
animation-name: some-animation;
}
::slotted(div) {
animation-name: some-animation;
}
</style>
<div id="two"></div>
<slot></slot>
</>
</> In this case, Safari will apply the animation correctly in both cases, because it uses the scope of the CSS reference (the I feel like the correct scope to use should be the scope where the CSS reference is made, and not the scope where the affected node is attached to the DOM. A more complete reproduction with comments can be found here: https://codepen.io/ruphin/pen/zPQvXw Both browsers are in agreement on the scoping rules of @Keyframes definitions except for the case of slotted elements styled from within the shadowroot. Chrome renders a red and two green squares, and Safari renders a blue and two green squares. |
@ruphin: Anonymous tags |
It's a pseudo-code example like in the original post. A working example with valid syntax can be found in the codepen. |
@ruphin Ah, cool, thanks for the compat research. I agree then that we should match Safari's behavior and let the bare keyword refer to the local definition. I'll update the OP. |
Current chrome behavior is somewhat broken, leaking <!doctype html>
<style>
#host {
width: 100px;
height: 100px;
background: green;
animation: myanim 10s infinite;
}
</style>
<div id="host"></div>
<script>
host.attachShadow({ mode: "open" }).innerHTML = `
<style>
@keyframes myanim {
from { background: red; }
to { background: red; }
}
</style>
`;
</script> What WebKit does makes sense (it keeps track of the scope the rule that ended up in the declaration). But that feels somewhat like a layering violation, having to propagate the cascade order down so much. |
Hmm, the problem with what WebKit does is that it doesn't work if you explicitly inherit the name from an scope you don't have access to, because suddenly it's from an scope you don't have access to. Or worse, it's from a different scope. I'm having a hard time deciding what to implement in Firefox here. :( |
Ugh, apparently Blink's behavior is pretty intentional judging their document lookup: Blink doesn't make animations in |
I consider the Blink implementation to be broken, and I would urge you to reconsider your position and follow the Safari implementation instead. From a developer point of view, the Blink implementation breaks the Web Components contract of code encapsulation. With their implementation, it is impossible to style a slotted component without breaking the encapsulation of the component and injecting style into the global document. |
With that I mean that in specific cases, as a Web Component author, with the Blink implementation, you must literally inject a <style> node outside your component in order style elements inside your component with the CSS ::slotted() spec. I consider this a strong violation of the core principles of Web Components. |
Oh, fwiw I totally agree. I have WIP patches to switch to WebKit's approach here: https://bugzilla.mozilla.org/show_bug.cgi?id=1458192, with tests at https://bugzilla.mozilla.org/show_bug.cgi?id=1458189. Note that WebKit breaks when inheriting across components. Getting that right is a bit more complex. That's why for now I landed a Blink-like approach. As soon as the spec authors confirm that the intention of the spec is to do what WebKit does, or at least confirm that what Blink does is not the intent of the spec, I can put them up for review and land them in Firefox. But until then I don't think it's worth complicating the code if the spec is going to change substantively. |
That sounds like a good approach. Who would be spec authors for this case? We have a comment from @tabatkins up in this thread. Can you provide an example of component inheritance that breaks in the WebKit implementation? I am not aware of any issues |
If you use <!doctype html>
<style>
#host {
animation: myanim 10s infinite;
}
</style>
<div id="host"></div>
<script>
host.attachShadow({ mode: "open" }).innerHTML = `
<style>
@keyframes myanim {
from { background: red; }
to { background: red; }
}
#inner {
width: 100px;
height: 100px;
background: green;
animation: inherit;
}
</style>
<div id="inner"></div>
`;
</script> Which shows red. In this case it doesn't seem like a big deal because we're using just the shadow tree rules, but I think I can come up with more confusing test-cases involving nested slots. |
Basically, putting Shadow-related information in the computed style, even if not exposed, looks like a bit of a layering violation, causing issues like that. |
You don't just have a "comment" from me, I started the thread and have an active proposal for how to handle these things. ^_^ Yes, Blink's behavior is bad. I'm not sure of the exact details of WebKit's implementation, but I'm pretty certain it's also still not what we finally want. Thus my proposal up top: name-defining things should scope their names to the TreeScope their stylesheet appears in; name-referencing things default to only referring to the top-level definition; |
Apple's WebKit team's feedback for the proposal will be that:
Basically our preferred approach here is to adopt WebKit's current approach, add the "inheritance" of names from the parent tree, then fix the inheritance issue since inheritance leaking the existence of shadow tree is a generic problem tracked by whatwg/html#3748. |
I concur that the current implementation in WebKit is the most sensible approach to this problem. If it is extended with fallbacks to ancestor scopes, that would be more than sufficient for almost all purposes. I also do not see a strong use for the |
No, recursive search does not work. If the outer page has a @font-face declaring a "foo" family, and sets With recursive search, as this declaration crosses the shadow boundary, it will suddenly start referencing the inner "foo" @font-face, which is completely unexpected. It means that shadows aren't encapsulated from the outer page; if they declare a font-family that happens to have the same name as one the outer page uses, it'll suddenly break the display of the component. The same applies to every other inheritable reference; it means an inherited With my proposal (all referencing things are implicitly a (scope, reference) pair), all of this works as expected. The outer page's
Chrome's behavior is definitely broken in that example; the two styles should act identically, either both seeing the shadow's @Keyframes or neither seeing it (depending on how the resolution rules work out). Compat's thornier than that tho, isn't it? In Chrome, @font-face simple doesn't work in shadows at all, and all |
True, there's at least precedence for this behavior in CSS custom properties which have to handle almost exactly the same problem. I think I would have preferred both to have been more strictly scoped but, given how similar they are and that one is certainly set in stone, having the scoping behavior be consistent is probably more important. |
@rniwa I want to make sure that y'all have the correct "inheritance" behavior, because I know we've discussed this in the past and you described your behavior as always looking up the tree for the nearest defining construct, even if the value is inherited. That is, in Example 5, according to what I understood from your previous descriptions all three elements in the shadow ( I can't easily test Safari right now; is that still y'all's behavior? Or do you match the spec here? (The behavior I described is broken, because it violates encapsulation in an important way - if the inner component ever defines a @font-face or similar construct with the same name as one in the outer page, it'll break any property that inherits in completely by accident, with no way for the component author to predict that this'll happen ahead of time, or fix it. The only way around it is to always author your components to use UUIDs for their @font-face and similar constructs, to guarantee that a 'font-family' inheriting in won't accidentally change meaning to suddenly refer to your @font-face. I'm not willing to spec that behavior, as it's just broken from an authoring point of view.) |
@font-face rules in WebKit seem like being global - exposing @font-face into and out of the shadow tree looking at this in Safari TP: <!doctype html>
<style>
@font-face {
font-family: myfont;
font-weight: bold;
src: local(Arial);
}
</style>
<div style="font-family: myfont; font-weight: normal">Normal - Document</div>
<div style="font-family: myfont; font-weight: bold">Bold - Document</div>
<div id="host"></div>
<script>
let root = host.attachShadow({mode:"open"});
root.innerHTML = `
<style>
@font-face {
font-family: myfont;
font-weight: normal;
src: local("Times New Roman");
}
</style>
<div style="font-family: myfont; font-weight: normal">Normal - Shadow</div>
<div style="font-family: myfont; font-weight: bold">Bold - Shadow</div>
`;
</script> |
The CSS Working Group just discussed
The full IRC log of that discussion<dael> Topic: [css-scoping] Handling global name-defining constructs in shadow trees<dael> github: https://github.com//issues/1995 <Rossen_> https://github.com//issues/1995#issuecomment-840895144 <dael> Rossen_: This is being brought a second time. Link to more relevant discussions ^ <Rossen_> q? <Rossen_> ack fantasai <dael> TabAtkins: Since into of shadow dom and we isolate chunks of html it beacme unclear how name defining contructs work. Can fontface defined in outer be in shadow dom? Defined in shadow dom in outer? If same name in both which wins? <dael> TabAtkins: All undefined <dael> TabAtkins: Had a prop for a long time.. Put it in scoping. Tweaked to match reality <dael> TabAtkins: Summary of proposal for informative- Whenever you have a name defining construct the names are defined in a css context, a tree scope. Inherit into nested tree scopes. A component with shadow dom can see names in outer scope and override <dael> TabAtkins: Same logic as inheritence. Within the shadowdom the inner defined one wins and doens't inerfere with rest of page <dael> TabAtkins: References to that. font-family: foo on an element. It sees the font-face rule from the context it's in. If you write it int he shadow dom you see the shadow dom defined. Carries that around as it inherits. Writing font-family:foo in the outer page. Inherits into shadow dom it still referes to outer page, even if there's a font-face:foo in the shadow dom <dael> TabAtkins: Required so you don't have to worry about collisions as you inherit. <dael> TabAtkins: [missed example about how this can be very bad] <dael> TabAtkins: That's the proposal. All reflected in scoping spec. To best of my knowledge the spec text is completed and well defined. Somewhat matches some browsers impl. none do everything correctly. for font-faces specifically there's a mix across browsres. I don't know what plan is to update to correct. <astearns> missed example is you can only avoid collisions by adding a bunch of random noise to names <Rossen_> q <dael> TabAtkins: I htink any behavior except what's in the spec is bad for authors and users. There might be compat we have to worry about and work around b/c so many years of the wild west. Would appriciate feedback to that <dael> TabAtkins: Need to get these on a level to where things work consistantly. Right now authors cannot use name defining contrstructs in shadow dom. <dael> TabAtkins: I tried to go through the specs I owned to make sure it's all well defined. Want to make sure everyone invokes properly. I'll ping authors of other specs <dael> TabAtkins: If a browser impl has objections or concerns I'd love to hear them <Rossen_> ack fantasai <dael> fantasai: Thanks for the overall explanation model you proposed makes a lot of sense. I think we need a WG resolution on this b/c not discussed before. <emilio> q+ <dael> Rossen_: Other opinions or concerns about impl or from impl with respect to compat risks? <Rossen_> ack emilio <dael> emilio: When I impl keyframe lookups in gecko and how they work in shadow dom I recalled weird behavior from other browsers. I want to make sure we have a place where we can see current state in all browsers. <dael> emilio: I think font-face doesn't work at all in showdow trees. Maybe in safari but prob show up in document.fonts which is wrong <dael> emilio: Usually we have something to say current state of compat <futhark> q+ <dael> TabAtkins: Reasonable. I can put together a wiki page and we can collect data <tantek> regrets+ <dael> emilio: That would be great. I think only keyframes work in gecko <dael> emilio: Other thing is do we need a shadow root prototype of fonts and how does that work. For now getting @fontface working is more important <dael> futhark: I wanted to mention we started working on this in blink. Deprioritized. Main concern is font caching. Had a plan to do it, but how do you do font caching in tree scopes; it's a complication but not impossible. not straightforward to make it performant <dael> Rossen_: Other opinions? <dael> Rossen_: Sounds like TabAtkins you will start collecting the current state of interop between browsers which will be great <bkardell_> q+ <dael> Rossen_: futhark is expressing some concern about impl complexity which is good but doesn't seem like a blocker? <Rossen_> ack futhark <dael> futhark: Not blocking. Explanation that this isn't trivial to impl <dael> bkardell_: Apologies if this is just noise. Conceptually document scoped things, we have a bunch of open questions and ongoing things. not nec css. Like id ref for aria. Conceptually they're very similar. <dael> bkardell_: Feels like they shouldn't be considered completely in isolation. Not sure if that's helpful <dael> emilio: id ref is slightly different case. That's more about allowing to reference stuff inside shadow tree in a global way. This is eq. to shadow root get element by ID not doing anything <dael> bkardell_: Not saying they're the same. Conceptually there's a lot of issues where there's this boundary and need cooperation mechanism. Would be great if we didn't have to have 10 of them. <dael> bkardell_: Probably just noise but in my head feels like having a lot of convos that are spiritually related. <dael> emilio: That is true <Rossen_> q? <Rossen_> ack bkardell_ <dael> Rossen_: Thank you bkardell_ <dael> Rossen_: On the issue here, other opinions? <dael> Rossen_: If not let's see if we can resolve <dael> TabAtkins: Prop: Tentative agreement on the spec text as in the draft. Subject to further review of current interop behavior <dael> fantasai: Can you summerize? Binding the name per context through inheritence <dael> TabAtkins: I don't want to try and nail down to that level of summary b/c that admits some bad behaviors. There's varients that can break. Details matter a lot and would rather point to spec <dael> Rossen_: Objections to TabAtkins prop text? <dael> RESOLVED: Tentative agreement on the spec text as in the draft. Subject to further review of current interop behavior <dael> fantasai: Spec is super out of date. When repub? <dael> TabAtkins: Fine with now if we want to resolve. <dael> fantasai: Do you have a changes summary? <dael> TabAtkins: Need to collect it. I'm sure a lot <dael> fantasai: Last pub was 2014 <dael> florian: Let's republish <dael> Rossen_: TabAtkins would you gather summary of changes and we'll resolve over email. It's just WD update, right? <dael> florian: Why would we not republish? <dael> fantasai: In case there's unreviewed new things <dael> florian: Alright <dael> Rossen_: We have a path forward. Please get the set of changes and we'll resolve over email |
Dropped the skeleton of an interop table in https://wiki.csswg.org/spec/css-scoping. I'll fill in what I can figure out in Chrome and Firefox, but any specific details filled in by people with more direct info would be great. |
I filled in the Firefox details. |
Note: this test case is the same as example FOUR in w3c/csswg-drafts#1995 (comment) Bug: 1225033 Change-Id: Ic02c6c0e9c623c4eb12ac4bc152716f63f6ebd52
Note: this test case is the same as example FOUR in w3c/csswg-drafts#1995 (comment) Bug: 1225033 Change-Id: Ic02c6c0e9c623c4eb12ac4bc152716f63f6ebd52 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3025847 Reviewed-by: Ian Kilpatrick <[email protected]> Commit-Queue: Xiaocheng Hu <[email protected]> Cr-Commit-Position: refs/heads/master@{#901552}
Note: this test case is the same as example FOUR in w3c/csswg-drafts#1995 (comment) Bug: 1225033 Change-Id: Ic02c6c0e9c623c4eb12ac4bc152716f63f6ebd52 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3025847 Reviewed-by: Ian Kilpatrick <[email protected]> Commit-Queue: Xiaocheng Hu <[email protected]> Cr-Commit-Position: refs/heads/master@{#901552}
Note: this test case is the same as example FOUR in w3c/csswg-drafts#1995 (comment) Bug: 1225033 Change-Id: Ic02c6c0e9c623c4eb12ac4bc152716f63f6ebd52 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3025847 Reviewed-by: Ian Kilpatrick <[email protected]> Commit-Queue: Xiaocheng Hu <[email protected]> Cr-Commit-Position: refs/heads/master@{#901552}
…ist markers via shadow DOM part, a=testonly Automatic update from web-platform-tests [counter-style] Add a test for styling list markers via shadow DOM part Note: this test case is the same as example FOUR in w3c/csswg-drafts#1995 (comment) Bug: 1225033 Change-Id: Ic02c6c0e9c623c4eb12ac4bc152716f63f6ebd52 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3025847 Reviewed-by: Ian Kilpatrick <[email protected]> Commit-Queue: Xiaocheng Hu <[email protected]> Cr-Commit-Position: refs/heads/master@{#901552} -- wpt-commits: 0993872f0a326817e4acb9b462930517816f82c4 wpt-pr: 29662
…ist markers via shadow DOM part, a=testonly Automatic update from web-platform-tests [counter-style] Add a test for styling list markers via shadow DOM part Note: this test case is the same as example FOUR in w3c/csswg-drafts#1995 (comment) Bug: 1225033 Change-Id: Ic02c6c0e9c623c4eb12ac4bc152716f63f6ebd52 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3025847 Reviewed-by: Ian Kilpatrick <[email protected]> Commit-Queue: Xiaocheng Hu <[email protected]> Cr-Commit-Position: refs/heads/master@{#901552} -- wpt-commits: 0993872f0a326817e4acb9b462930517816f82c4 wpt-pr: 29662
…ist markers via shadow DOM part, a=testonly Automatic update from web-platform-tests [counter-style] Add a test for styling list markers via shadow DOM part Note: this test case is the same as example FOUR in w3c/csswg-drafts#1995 (comment) Bug: 1225033 Change-Id: Ic02c6c0e9c623c4eb12ac4bc152716f63f6ebd52 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3025847 Reviewed-by: Ian Kilpatrick <[email protected]> Commit-Queue: Xiaocheng Hu <[email protected]> Cr-Commit-Position: refs/heads/master@{#901552} -- wpt-commits: 0993872f0a326817e4acb9b462930517816f82c4 wpt-pr: 29662
…ist markers via shadow DOM part, a=testonly Automatic update from web-platform-tests [counter-style] Add a test for styling list markers via shadow DOM part Note: this test case is the same as example FOUR in w3c/csswg-drafts#1995 (comment) Bug: 1225033 Change-Id: Ic02c6c0e9c623c4eb12ac4bc152716f63f6ebd52 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3025847 Reviewed-by: Ian Kilpatrick <[email protected]> Commit-Queue: Xiaocheng Hu <[email protected]> Cr-Commit-Position: refs/heads/master@{#901552} -- wpt-commits: 0993872f0a326817e4acb9b462930517816f82c4 wpt-pr: 29662
Note: this test case is the same as example FOUR in w3c/csswg-drafts#1995 (comment) Bug: 1225033 Change-Id: Ic02c6c0e9c623c4eb12ac4bc152716f63f6ebd52 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3025847 Reviewed-by: Ian Kilpatrick <[email protected]> Commit-Queue: Xiaocheng Hu <[email protected]> Cr-Commit-Position: refs/heads/master@{#901552} NOKEYCHECK=True GitOrigin-RevId: a5cb2f8b37a6f0127ece4a213084fb332ac5bfb4
Likely of interest to people here: https://github.com/gnat/css-scope-inline These 16 lines scopes your animation |
Since the introduction of Shadow DOM, we've been struggling with what to do with name-defining things, like @font-face, which define global names for other things to reference.
Right now the answer is a collective shrug. I think I have an answer, however:
@font-face
) is valid inside of shadow trees, and is scoped to the TreeScope that its defining stylesheet is in. Nested shadows can use name-defining things defined in higher trees (see below), but can't directly refer to them.font-family
font name) is implicitly a tuple of (name, defining scope), where the defining scope is the TreeScope the reference's stylesheet is in. (In other words, it's always a reference to the local thing defining the name, not something further up the scope tree.)::part()
) thus resolves the name against the stylesheet's TreeScope, even if the element's TreeScope has that name redefined to something else. (So an outer page setting an::part(foo) { animation: foo 1s; }
uses the@keyframes foo {...}
from the outer page, not anything from inside the shadow tree that that "foo" part is in.).scope
attribute or something, which points to the tree scope the keyword is being resolved against.This has some implications. Since the defining scope is implicitly captured by a reference, it doesn't change as the value inherits. Thus, in this situation:
Scripting is a slightly thornier problem here. When setting styles, we can use the rules I've already laid out - you're always setting the style in some stylesheet (perhaps the implicit one attached to an element and accessed via
el.style
), so there's a consistent notion of an associated TreeScope. (This may not always be obvious, but it's clear - a script that pokes around inside of the shadows of its components and sets styles needs to be aware of what scope the stylesheet is in and what scope the name-defining thing it's trying to reference is in.)(Edited to take into account the compromise to drop the
scoped()
syntax and only allow implicit references via the string-based API.)The text was updated successfully, but these errors were encountered: