Html: UA shadow roots are observable via inheritance.

Created on 8 Jun 2018 · 27Comments · Source: whatwg/html

See https://bugs.chromium.org/p/chromium/issues/detail?id=850872 for example, where implementing the right <slot> behavior regressed a website in Chrome because they use a UA shadow root for <object>.

A way to minimize the impact would be to spec that UA ShadowRoots should have slot { all: inherit; display: contents; } instead of just the default display: contents. But you could still observe it using display: inherit...

I'm not sure this is a frequent problem, I guess not, so feel free to resolve as you think it's more appropriate, but sounded worth letting the spec people know...

rendering shadow

Source

emilio

Most helpful comment

Can we do it for all shadow roots?

annevk on 15 Jun 2018

👍2

All 27 comments

@whatwg/components I think it would be good if we kept UA shadow roots unobservable. Special casing inheritance of display seems unfortunate though. (And it's unclear if this is the only observable effect.)

annevk on 8 Jun 2018

If we're special-casing inheritance I'd rather special-case inheritance across UA shadow trees entirely (that is, making the content inheriting from the light tree parent in this case)...

That would be easy-ish to implement for Firefox, but not sure that's something that other engines would like to implement.

emilio on 8 Jun 2018

👍1

I haven't checked which UA roots + slots we have, but slot { all: inherit; display: contents } would still not work if the slot is not a direct descendant of the shadow root.

lilles on 8 Jun 2018

👍2

Oh, yeah, that's for sure. I was assuming all the UA shadow roots had <slot> as a direct descendant, but I guess that's not necessarily the case.

emilio on 8 Jun 2018

Seems <meter> with -webkit-appearance:none has a slot wrapped in a div in Chrome.

This does not give a green border on the first one even with SlotInFlatTree disabled.

<!DOCTYPE html>
<p>Both borders green?</p>
<meter style="border-color:green; -webkit-appearance:none">
  <div style="border-style:solid; border-width:10px; border-color:inherit">Meter</div>
  <div style="border-style:solid; border-width:10px; border-color:green">Meter</div>
</meter>

The other element in Chrome I found is <marquee>

lilles on 8 Jun 2018

I wonder if @tabatkins / @rniwa have any opinions about this.

emilio on 8 Jun 2018

I don't really understand this issue; are UA shadow roots something that specifications govern? I thought it was just a way in which implementations reused some web technologies for their own internal implementation details, and thus outside the scope of specs.

domenic on 8 Jun 2018

@domenic the HTML Standard suggests using them for <details>. Also, if this cannot be fixed and leads sites to depend on the observable behavior we'll have to say something I suspect.

annevk on 8 Jun 2018

I haven't checked which UA roots + slots we have, but slot { all: inherit; display: contents } would still not work if the slot is not a direct descendant of the shadow root.

Yeah, this is my problem with the attempted hack. It might work for current elements, but there's no guarantee that the slot will be top-level for future elements using UA shadow trees.

So I suspect we probably do want to special-case the inherit keyword here, at least for UA shadows. But maybe we should do it for all shadows? I mean, if a page author writes:

<foo-component>
  <p style="inherit: background">...
</foo-component>

They probably intend for the p to inherit from foo-component, regardless of what the shadow tree looks like.

So yeah, while standard inheritance still goes thru the flat tree, maybe the inherit keyword itself should inherit from the tree-of-trees parent?

tabatkins on 8 Jun 2018

I tend to agree @domenic here. Basically, I think it is a browser vendor's responsibility to make "implementation details" un-observable.

@emilio Have you filed an issue for Chromium? At least, I am not aware of this issue so far.

At the same time, it would be unfortunate that browser vendor can't use UA shadow roots because of these accidental side effects. I wouldn't be surprised if there are other side effects which can be observable. Can someone notice other side effects?

hayatoito on 11 Jun 2018

Ah, it is already tracked at https://bugs.chromium.org/p/chromium/issues/detail?id=850872.
@emilio Sorry for the noise.

hayatoito on 11 Jun 2018

I'd prefer not special-casing inherit FWIW, that means we'd need to track two inherited styles instead of just one through the style system.

Changing the whole inheritance parent would be easier IMO, I don't see the benefit of special-casing inherit: If UA shadow roots change inherited properties they'd be observable by default (without inherit usage from the author) already, which would be wrong, right?

emilio on 11 Jun 2018

👍2

@hayatoito if the specification prescribes using shadow roots to implement something, it could be read as if the side effects are intended. So I think either way we need to clarify the specification here.

annevk on 11 Jun 2018

I think that implementation-wise in Blink, it would be less intrusive to have a different inheritance parent for UA slotted nodes than special-casing the inherit keyword, unless we already rely on inheritance down from shadow tree elements.

lilles on 11 Jun 2018

👍1

That seems fine to me. I can't imagine any UA shadow root wanting to affect the inheritance of a slotted element, and making this explicitly inherit differently prevents that from happening accidentally.

(And I'm fine with rejecting the proposal to make inherit work differently from inheritance in general, I was just chasing a thought.)

tabatkins on 11 Jun 2018

👍1

Would we want to do this for closed shadow roots as well? Or just for UA shadow roots?

emilio on 14 Jun 2018

I can go either way on that, whatever implementations feel would be better.

tabatkins on 14 Jun 2018

Can we do it for all shadow roots?

annevk on 15 Jun 2018

👍2

I agree that doing this for all shadow roots probably makes the most sense, as Shadow DOM exists to hide implementation details.

ExE-Boss on 16 Jun 2018

Thinking more about it, it's likely such a model has somewhat subtle bugs. I've audited some places and we currently have flags that propagate on the style tree which assume they're following the flattened tree, and this could potentially break, like text-decoration-lines:

https://searchfox.org/mozilla-central/rev/6eea08365e7386a2b81c044e7cc8a3daa51d8754/servo/components/style/properties/computed_value_flags.rs#20

emilio on 16 Jun 2018

I agree that doing this for all shadow roots probably makes the most sense, as Shadow DOM exists to hide implementation details.

That is not always correct. Shadow DOM exists to hide internal DOM tree structure, however, hiding every implementation details has never been a design goal of Shadow DOM. That would be a desired property, from some perspectives, however, that has been out of Shadow DOM's scope.

Shadow DOM affects almost every rendering result (e.g. how host or host's child is rendered), which are observable.

hayatoito on 25 Jun 2018

I think we should do this for all shadow DOM if we're doing it at all.

rniwa on 25 Jun 2018

Hiding the implementation details for UA shadow and applying it to author shadow are two separate issues, and let's focus on the original UA shadow's case here.

The problem is, that if an element is implemented using UA shadow root, not only the existence is observable but also it has interoperability issue with browsers implementing the element natively.

TakayoshiKochi on 26 Jun 2018

Hiding the implementation details for UA shadow and applying it to author shadow are two separate issues, and let's focus on the original UA shadow's case here.

I don't see why these are two separate concerns. The fundamental problem here is that the presence of a slot element is causing the inheritance to not work the way users of a builtin element expected. I see no difference in this versus the expectation users of a Web component would have.

The fact even UA implementors screw this up is a good evidence that we need to provide a better default behavior. Our expectation can't be that developers who write web components are more knowledgeable about CSS than Web engine developers.

rniwa on 26 Jun 2018

👍1

It's totally fine from your perspective "always inheriting styles from shadow host to slot assignees" is a superset of the solution to "UA shadow is leaking the implementation details". I'm saying that fixing UA shadow case does not have to be tied together to non-UA shadows.

Changing the inheritance for non-UA shadows would involve spec change and potential breaks in existing sites, fixing only UA shadows does not require spec change, and should improve interoperability whether or not an element is implemented using UA shadow.

TakayoshiKochi on 27 Jun 2018

TPAC F2F WebPlat: This probably needs to be discussed in the CSS WG since this would affect inheritance.

We think this is a high priority bug because it leaks the information that shadow root exists on a given element thereby breaking the encapsulation.

rniwa on 25 Oct 2018

CSSWG issue: https://github.com/w3c/csswg-drafts/issues/3248