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-scoping] explicit inherit values for non-inherited properties give unexpected results inside shadow DOM transformations such as <details> #8184

Closed
infinity0 opened this issue Dec 5, 2022 · 14 comments

Comments

@infinity0
Copy link

Original issue at https://bugzilla.mozilla.org/show_bug.cgi?id=1803927

Basically the bug is caused by * selecting only "element nodes", and not selecting the shadow-root "non-element node" [1] that is created by the definition of <details>. [2]

As I understand, and according to various other people's understanding of Shadow DOM as described in various web documents, shadow DOM is meant to provide encapsulation, e.g. so you can include other people's content in your own content without their scripts/styles breaking your scripts/styles, including browser UI elements. (Your scripts/styles OTOH can affect their scripts/styles, which makes sense as you are in control, and can write your scripts/styles in a way so as not to break theirs. All good.)

Normally when people use <details> however, there is no encapsulation intended. The content inside and outside the <details> is my own content, and I do not want there to be any separation. In particular, I want elements inside the <details> to inherit CSS rules as normal. Because * does not select the shadow-root, there is no way to have it pass through inherited properties like box-sizing, even though my CSS rules can reach into its children directly and apply there.

[1] https://www.w3.org/TR/css-scoping-1/#shadow-dom
[2] https://html.spec.whatwg.org/#the-details-and-summary-elements

@infinity0
Copy link
Author

infinity0 commented Dec 5, 2022

Alternatively if it is not possible to select the shadow-root (e.g. because it is not allowed to carry CSS properties? not familiar enough with the spec to know this for sure), then the meaning of "inherit" should be tweaked so as to treat the shadow-host as the "effective parent" for CSS inheritance purposes, if the "real parent" is a shadow-root.

@infinity0
Copy link
Author

the meaning of "inherit" should be tweaked

Precisely, this would amend 3.3.2 inheritance to say that

  • when CSS from the host tree is applied to elements in the flattened shadow tree,
    • any elements of the flattened shadow tree, that were originally taken from the host tree (these are not necessarily top-level elements of the shadow tree)
    • these elements should inherit CSS from their parent node in the original host tree, rather than their current parent in the shadow tree.

The overall justification being that the current behaviour of

details > summary { box-sizing: inherit; }

is absurd - summary gets selected as a direct child of details, but then inherits from a different parent element in the shadow tree. I guess in implementations, the <details> node still keeps child-links to the <summary> node, and that is how it gets selected by ">", but when walking back up the tree for "inherit" the <summary> node's parent now points to the <slot> in the shadow tree.

@emilio
Copy link
Collaborator

emilio commented Dec 5, 2022

Selectors from outside the shadow tree not matching the shadow tree is a feature, not a bug. What you're seeing is one of the subtle ways shadow dom is observable, because the inheritance parent is different from the DOM parent. This is described in https://drafts.csswg.org/css-scoping-1/#flattening

@infinity0
Copy link
Author

This feature becomes a bug when it is used to implement things native to HTML like <details>, as opposed to browser elements or intentional uses of the shadow DOM, because it results in

details > summary { box-sizing: inherit; }

behaving absurdly.

usage intended by user? controllable by user? works
regular HTML without internal shadow DOM elements Y Y
<details> etc with internal shadow DOM elements N semi
browser elements using shadow DOM N N
explicit uses of shadow DOM via attachShadow Y Y

Can you read the rest of my comments including my suggestion about changing the behaviour of inheritance so it behaves more intuitively without absurdities?

@Loirooriol
Copy link
Contributor

It seems to me you should just use

:root { --box-sizing: border-box; }
*, *:before, *:after { box-sizing: var(--box-sizing); }

And then change --box-sizing when you want to affect the subtree. You are in control of --box-sizing, so it won't happen unexpectedly.
You say box-sizing: inherit is a standard way but TBH it's the 1st time that I have seen it and I don't think it's the right solution. It's not just shadow DOM, you also risk that important UA rules or adjuster changes at computed-value time will unexpectedly affect whole subtrees.

@infinity0
Copy link
Author

infinity0 commented Dec 5, 2022

Never mind box-sizing, it's not the point of the overall bug even if it's how I originally discovered it. The bug exists for all similar non-inherited properties, e.g. border: inherit. The bug can be summarised as:

$a > $b { $property: inherit; }

intuitively, one would expect that $b inherits $a's $property in all circumstances. But that's not the case when shadow DOM is used and $b is cut away from $a (and $property isn't an inherited property, otherwise the bug exists but isn't visible as $property gets inherited "through" the shadow DOM via other CSS rules) . OTOH, $b is in fact selected via $a >; you can apply other styles to it and that works. However inherit doesn't work.

@infinity0 infinity0 changed the title [css-scoping] * should also select shadow roots, a non-element node [css-scoping] $a > $b { $property: inherit; } behaves absurdly under shadow DOM transformations such as <details> Dec 5, 2022
@infinity0
Copy link
Author

Would something break if we were to implement what I suggested? Does something important on the web strongly depend on the fact that inheritance works differently than selecting descendants in the case of a shadow DOM?

@Loirooriol
Copy link
Contributor

See this testcase. Would you really prefer inheriting the magenta border instead of the cyan one? I wouldn't.

@tabatkins
Copy link
Member

Yup, as others have said, while Selectors works on the original tree(s), inheritance (and the entire rest of CSS) works on the "flattened tree" - that is, a shadow host's children get replaced by its shadow tree, its original children are reparented to their slots, etc. This is the tree that layout works on, by both intention and necessity, and most of CSS wants to be compatible with this tree as a result. In particular, inheritance needs to be compatible with it, since inheritance happens after we figure out computed values, and computed value determination works on the flattened tree (so it's consistent with layout)

This also means that properties that inherit by default take their inheritance from the shadow, when an element is slotted. This, too, is a good thing - an element slotted into a shadow would generally like its text color to be consistent with the background color coming from the shadow elements, for example.

There are a few places where this is a less convenient, or more confusing, behavior - you've found one, where explicit inheritance of a non-inherited property "breaks" when the element is slotted, because the shadow parent isn't explicitly inheriting as well. Unfortunately, we have to choose a behavior, and the current behavior makes the most sense in the most cases, despite the occasional mismatch in expectations.

@infinity0
Copy link
Author

See this testcase. Would you really prefer inheriting the magenta border instead of the cyan one? I wouldn't.

Unfortunately, we have to choose a behavior, and the current behavior makes the most sense in the most cases, despite the occasional mismatch in expectations.

The difference between the above testcase (and most other explicit use cases of shadow DOM I've seen described) vs <details> is that the former one is opt-in and extremely obvious to the end user (web developer) whereas the second one isn't. Major resources on <details> e.g. [1] [2] do not mention the shadow DOM at all.

It's an exception that makes <details> special, and most web developers will have no idea it's special in this way. Are there other HTML elements that use the shadow DOM as part of its official definition? I have no idea and I bet most web developers don't either - nor should they need to.

Selectors from outside the shadow tree not matching the shadow tree is a feature, not a bug. [..]

In other words, the shadow DOM is intended to provide some level of encapsulation.

However, its usage in <details> forces an encapsulation that the web developer does not intend i.e. a weird separation between the outside vs the inside of <details>.

Forced encapsulation when the user does not request it, is a deficiency of any language. Can you please re-open the bug - since this issue exists and is not fixed. If changing the behaviour of inheritance is too awkward, there are other options:

  1. adding another "actually-universal" selector e.g. ** or something, that at least selects into shadow roots that are not explicitly created by the web developer nor is an internal web browser element, which would include <details>
  2. changing the definition of <details> so it doesn't force unrequested encapsulation onto the end user

@infinity0
Copy link
Author

infinity0 commented Dec 6, 2022

Or even more bluntly,

  1. a concrete flag for shadow roots saying which ones are explicitly intended by the user, vs implicitly defined by some standard that is hidden from users, and having inheritance work differently (as suggested above) based on this flag

The options for fixing are all pretty simple, I'm not sure why people are continuing to play "bug denial" here.

@tabatkins
Copy link
Member

The options to "fix" it are not simple. We can change things to fix your case, but only by breaking other cases. As with many API design choices, there are tradeoffs that have to be made.

@dbaron dbaron changed the title [css-scoping] $a > $b { $property: inherit; } behaves absurdly under shadow DOM transformations such as <details> explicit inherit values for non-inherited properties give unexpected results inside shadow DOM transformations such as <details> Dec 6, 2022
@dbaron
Copy link
Member

dbaron commented Dec 6, 2022

Another idea that might fix it is to make explicit inherit values on non-inherited properties behave differently from those on inherited properties (in terms of shadow DOM). This would make the presence or absence of a shadow dom for an element less visible.

In the past I think we discussed something similar for ::first-line and/or ::first-letter pseudo-elements, although I'm not sure if we did it.

@dbaron dbaron changed the title explicit inherit values for non-inherited properties give unexpected results inside shadow DOM transformations such as <details> [css-scoping] explicit inherit values for non-inherited properties give unexpected results inside shadow DOM transformations such as <details> Dec 6, 2022
@infinity0
Copy link
Author

infinity0 commented Dec 7, 2022

We can change things to fix your case, but only by breaking other cases.

Of course I can understand, if this is the case for a particular suggestion, then it is no good.

But specifically, how do my suggestions break other use cases? @Loirooriol pointed out one issue above and I followed this up with a tweak of the suggestion to address it. That is how I imagined the discussion would proceed, by iterating on successive ideas.

Instead however, most of the rest of the previous comments have been to just deny there is a problem in the first place, which IMO is not constructive.

I can also understand if this issue is treated as low-priority if there are lots of other things to do by this team. But in this case the appropriate thing to do is to comment as such, say you have no time, mark the issue as low-priority, but leave it open, and not deny there is a problem.

Another idea that might fix it is to make explicit inherit values on non-inherited properties behave differently from those on inherited properties (in terms of shadow DOM). This would make the presence or absence of a shadow dom for an element less visible.

Yes, this is similar to my (3) above except we allow the user to explicitly select which option they want.

We can split the thinking of the solution into two parts, the mechanism vs the exposed interface. As I see it there are 2 possible mechanisms:

  1. support attaching properties to the shadow root so that it can inherit properties from the main tree, then its children can inherit these properties from it, fixing this issue. this is currently not possible.
  2. allow the children to inherit from their original parent, instead of the new parent in the shadow root, this is currently not possible either.

Either of these two mechanisms will fix the issue. There is a third option i.e. redefine <details> not to use shadow DOM ( (2) above) but I guess this will be too disruptive, and I imagine there's not many other alternatives.

For mechanism (1), the natural interface is to add a selector to allow the user/webdev to actually select the shadow root, currently there is no interface to do this. This was my original suggestion (the very first title of this bug report) as well as a tweaked suggestion in (1) above.

For mechanism (2), there are a few options for interfaces, either implicit (my (3) above, or @dbaron's idea), or even explicit, e.g.

a. via a inherit-shadow value, which IMO is clearest (users have to explicitly opt into it, solving the current problem where <details> forces unrequested encapsulation), but breaks backwards compatibility of the current behaviour (i.e. inherit always refers to the declared parent in the source code, similar to the > selector), or
b. via a inherit-declared value, which retains the current behaviour, giving users the ability to opt-out (but the current problem is that most users are unaware that <details> needs to be opted-out-of in this way)

Again, I would appreciate if the discussion could focus on downsides of any of these suggestions, rather than just flat-out denying that the problem exists. (And re-open the issue, ofc.)

fantasai added a commit to fantasai/csswg-drafts that referenced this issue Dec 31, 2022
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