Bug 154765 - ::slotted doesn't work in nested shadow trees
Summary: ::slotted doesn't work in nested shadow trees
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-02-26 17:29 PST by Ryosuke Niwa
Modified: 2016-02-29 00:37 PST (History)
5 users (show)

See Also:


Attachments
Test case (2.33 KB, text/html)
2016-02-26 17:29 PST, Ryosuke Niwa
no flags Details
patch (6.87 KB, patch)
2016-02-28 02:40 PST, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (6.88 KB, patch)
2016-02-28 02:47 PST, Antti Koivisto
rniwa: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2016-02-26 17:29:37 PST
Created attachment 272386 [details]
Test case

As I commented in https://bugs.webkit.org/show_bug.cgi?id=149441#c8,
::slotted doesn't work when there are nested shadow trees.

See the attached test case.
Comment 1 Radar WebKit Bug Importer 2016-02-26 17:31:33 PST
<rdar://problem/24870995>
Comment 2 Antti Koivisto 2016-02-26 23:48:28 PST
The test results look correct to me. The ::slotted selectors in the inner shadow tree match the children of its host, the <inner-host> element in the outer shadow tree. Nothing styles the FAIL2 to case in the <outer-host> so it stays red.
Comment 3 Antti Koivisto 2016-02-27 00:01:21 PST
Anything else requires selector matching for ::slotted() contents to happen in the composed tree which is not consistent with how things work otherwise.
Comment 4 Antti Koivisto 2016-02-27 02:02:08 PST
Hmm, maybe it makes sense to do this. It seems logical ::slotted(span) styles all spans in the slot whether they ended up there directly or indirectly.

I think you test case is wrong though. I don't see a rule that would style the foreground of the FAIL2 case to green. This rule

slot[name=inner]::slotted([slot=inner]) { color: green; }

doesn't apply since FAIL2 is not in the <slot name="inner"> of the inner tree. The left side of ::slotted definitely only applies to the current tree.
Comment 5 Ryosuke Niwa 2016-02-27 20:58:37 PST
(In reply to comment #4)
>
> slot[name=inner]::slotted([slot=inner]) { color: green; }
> 
> doesn't apply since FAIL2 is not in the <slot name="inner"> of the inner
> tree. The left side of ::slotted definitely only applies to the current tree.

Oh oops, yeah, we need to get rid of the leading "slot[name=inner]" there.
Comment 6 Antti Koivisto 2016-02-28 02:40:56 PST
Created attachment 272448 [details]
patch
Comment 7 Antti Koivisto 2016-02-28 02:47:39 PST
Created attachment 272449 [details]
patch
Comment 8 Antti Koivisto 2016-02-28 06:53:57 PST
Comment on attachment 272449 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=272449&action=review

> LayoutTests/fast/shadow-dom/css-scoping-shadow-slotted-nested.html:55
> +            var outerHost = document.querySelector('outer-host');
> +            outerShadow = outerHost.attachShadow({mode: 'closed'});
> +            outerShadow.appendChild(document.getElementById('outer-host-template').content.cloneNode(true));

BTW, I feel the current set of specs desperately needs a declarative way to attach shadow based on a template. Something like <outer-host shadow="outer-host-template"> to replace all this boilerplate.
Comment 9 Ryosuke Niwa 2016-02-28 14:07:18 PST
Comment on attachment 272449 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=272449&action=review

>> LayoutTests/fast/shadow-dom/css-scoping-shadow-slotted-nested.html:55
>> +            outerShadow.appendChild(document.getElementById('outer-host-template').content.cloneNode(true));
> 
> BTW, I feel the current set of specs desperately needs a declarative way to attach shadow based on a template. Something like <outer-host shadow="outer-host-template"> to replace all this boilerplate.

Indeed.  We've suggested that three years but we've agreed to punt it in v1 so I guess that's that :(
It's really silly that attachShadow or defineElement doesn't take an optional argument though.
e.g.
document.defineElement('my-element', MyElementClass, {template: MyElementClass.template});
would go a long way to simplify this whole setup.
Comment 10 Ryosuke Niwa 2016-02-28 14:09:16 PST
Comment on attachment 272449 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=272449&action=review

> LayoutTests/fast/shadow-dom/css-scoping-shadow-slotted-nested.html:4
> +    <title>CSS Scoping - ::slotted pseudo element must allow selecting elements assigned to a slot element</title>

Perhaps we should update this title as well?
e.g. ::slotted pseudo element rule must apply to an element that got slotted via another slot
Comment 11 Ryosuke Niwa 2016-02-28 14:10:18 PST
Comment on attachment 272449 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=272449&action=review

> Source/WebCore/ChangeLog:8
> +

We should probably refer to https://github.com/w3c/webcomponents/issues/331#issuecomment-189191593
since that clarifies this behavior.
Comment 12 Antti Koivisto 2016-02-29 00:37:57 PST
http://trac.webkit.org/changeset/197316