New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 633007 link

Starred by 4 users
Status: Fixed
Owner:
█ kochi@chromium.org
Last visit > 30 days ago
Closed: Nov 2016
Cc:
hayato@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 670580
Blocking:
issue 668933



Sign in to add a comment

Should /deep/ work in JS for shadow dom v1?

Project Member Reported by ericbidelman@chromium.org, Jul 31 2016 
Version: canary 54.0.2813.0
OS: Mac OSX

Apologies for not remembering where we land on using `/deep/` and `::shadow` in JavaScript, but I thought folks were in favor of having a solution. If there is already an open/closed issue, feel free to close this.

Using `/deep/` in a `querySelectorAll` statement doesn't find the internal nodes, even in open mode:

var div = document.createElement('div');
var shadowRoot = div.attachShadow({mode: 'open'});
shadowRoot.innerHTML = '<div class="title">title</div>';
document.body.appendChild(div);
document.querySelectorAll('html /deep/ .title').length; // 0

In v0, this was possible:

var div = document.createElement('div');
var shadowRoot = div.createShadowRoot();
shadowRoot.innerHTML = '<div class="title">title</div>';
document.body.appendChild(div);
document.querySelectorAll('html /deep/ .title').length; // 1

The alternative is to traverse the DOM using .shadowRoot, which is pretty ugly and will be a lot of boilerplate code for developers.

If `/deep/` should no longer work in Shadow DOM v1, we should warn in the console when it's used in querySelector? It looks like v0 warns for CSS usage but note JS usage. Related: https://bugs.chromium.org/p/chromium/issues/detail?id=545726

Comment 1 Deleted

Comment 2 by hayato@chromium.org, Aug 1 2016

Cc: -hayato@chromium.org kochi@chromium.org
Owner: hayato@chromium.org
Status: Assigned (was: Untriaged)

Comment 3 by hayato@chromium.org, Aug 1 2016

Cc: -kochi@chromium.org hayato@chromium.org
Owner: kochi@chromium.org
kochi@, could you move the /deep/ forward?

Although we do not get agreements from other browser vendors, I think it is okay to support /deep/ in the static profile in v1, as the spec says.

https://drafts.csswg.org/css-scoping/#deep-combinator.

Note The spec is using ">>>", instead of "/deep/", thus, I would like to have '>>>' in the static profile in v1.

Comment 4 by kochi@chromium.org, Aug 1 2016 

Agreed, will work on this.

Comment 5 by annevank...@gmail.com, Aug 5 2016 

Do you really want to break ranks again so shortly after we have reached cross-browser agreement on custom elements and shadow trees?

Comment 6 by kochi@chromium.org, Aug 5 2016 

In some sense this is out of Shadow DOM v1 spec scope, we are exploring the
usefulness of having it in static profile.  We currently don't pierce v1 shadow
tree with /deep/, as the original issue description says.

Comment 7 by ericbide...@google.com, Aug 5 2016 

If ">>>" is not the answer, I think we need something else spec'd. If it's spec'd, it seems like a reasonable solution for JS.

We need a _sane_ way to drill into shadow trees from JS. The primary reason is that users will forget/neglect to provide css custom property hooks (#3 here: https://developers.google.com/web/fundamentals/primers/shadowdom/?hl=en#closed) and force consumers to find nodes and style them in JS. 

Secondly, you may want to find/target a node and adjust something because of engine rendering bugs. 

Thirdly, it's pretty sad how much code a dev has to write atm.  Basic example of finding all custom elements across trees: https://developers.google.com/web/fundamentals/primers/shadowdom/?hl=en#finall). Actually targeting internals would be even more boilerplate!

My big worry here is that the harder we make this stuff to use, the fewer devs that actually use web components. That's not good for growing an ecosystem :(

Comment 8 by hayato@chromium.org, Aug 9 2016 

Clarification:

kochi@, please do not ship any feature until we get consensus from other browser vendors. I meant that we should file a spec issue at first to get consensus.

Supporting '>>>" only in the static profile (only for open shadow roots) should not have any performance penalty which we encountered in '/deep/' in dynamic profiles. That is a just utility feature which can make the Web faster, I think.

Comment 9 by kochi@chromium.org, Aug 9 2016 

Okay, let me continue on the existing issue entry on GitHub first.
https://github.com/w3c/webcomponents/issues/78

Comment 10 by hayato@chromium.org, Aug 9 2016 

Yeah, I didn't realize that the issue was not closed. :)

Comment 11 by hayato@chromium.org, Oct 12 2016

Components: -Blink>WebComponents Blink>DOM>ShadowDOM

Comment 12 by kochi@chromium.org, Nov 14 2016

Status: Started (was: Assigned)
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/528e38c9ff62f9eee3ced46ba11f4450e2fd4616

commit 528e38c9ff62f9eee3ced46ba11f4450e2fd4616
Author: kochi <kochi@chromium.org>
Date: Tue Nov 15 06:47:07 2016

Introduce CSS parser mode for distinguishing static/dynamic profile

Static/dynamic profile is proposed at CSS Selectors Level 4 draft[1]
to selectively enable or disable CSS selectors depending on the
performance characteristics that selectors require.

'>>>' (aka /deep/) is planned to be enabled under the experimental
flag in static profile [2].  In the future :has() pseudo class will
also be enabled in static profile.

Implementation for '>>>' combinator is done in later CLs.

[1] https://drafts.csswg.org/selectors/#profiles
[2] https://github.com/w3c/webcomponents/issues/78

BUG= 633007 

Review-Url: https://codereview.chromium.org/2493003003
Cr-Commit-Position: refs/heads/master@{#432133}

[modify] https://crrev.com/528e38c9ff62f9eee3ced46ba11f4450e2fd4616/third_party/WebKit/Source/core/css/parser/CSSParserMode.cpp
[modify] https://crrev.com/528e38c9ff62f9eee3ced46ba11f4450e2fd4616/third_party/WebKit/Source/core/css/parser/CSSParserMode.h
[modify] https://crrev.com/528e38c9ff62f9eee3ced46ba11f4450e2fd4616/third_party/WebKit/Source/core/dom/SelectorQuery.cpp
[modify] https://crrev.com/528e38c9ff62f9eee3ced46ba11f4450e2fd4616/third_party/WebKit/Source/core/dom/SelectorQueryTest.cpp
Project Member

Comment 14 by bugdroid1@chromium.org, Nov 18 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/69635dc029d7ce77e7db12669a994295d5f93952

commit 69635dc029d7ce77e7db12669a994295d5f93952
Author: kochi <kochi@chromium.org>
Date: Fri Nov 18 06:34:31 2016

Parser support for >>> (shadow-piercing descendant) combinator.

'>>>' (shadow-piercing descendant) can only be parsed in
static profile. The parsing is guarded by a runtime enabled flag.

Matching part is done in the next CL:
https://codereview.chromium.org/2496123002/

BUG= 633007 

Review-Url: https://codereview.chromium.org/2500813003
Cr-Commit-Position: refs/heads/master@{#433133}

[modify] https://crrev.com/69635dc029d7ce77e7db12669a994295d5f93952/third_party/WebKit/Source/core/css/CSSSelector.cpp
[modify] https://crrev.com/69635dc029d7ce77e7db12669a994295d5f93952/third_party/WebKit/Source/core/css/CSSSelector.h
[modify] https://crrev.com/69635dc029d7ce77e7db12669a994295d5f93952/third_party/WebKit/Source/core/css/SelectorChecker.cpp
[modify] https://crrev.com/69635dc029d7ce77e7db12669a994295d5f93952/third_party/WebKit/Source/core/css/SelectorFilter.cpp
[modify] https://crrev.com/69635dc029d7ce77e7db12669a994295d5f93952/third_party/WebKit/Source/core/css/parser/CSSParserMode.cpp
[modify] https://crrev.com/69635dc029d7ce77e7db12669a994295d5f93952/third_party/WebKit/Source/core/css/parser/CSSParserMode.h
[modify] https://crrev.com/69635dc029d7ce77e7db12669a994295d5f93952/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp
[modify] https://crrev.com/69635dc029d7ce77e7db12669a994295d5f93952/third_party/WebKit/Source/core/css/parser/CSSSelectorParserTest.cpp
[modify] https://crrev.com/69635dc029d7ce77e7db12669a994295d5f93952/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in
Project Member

Comment 15 by bugdroid1@chromium.org, Nov 18 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fb35cc5796f051b0f83867417686746649eea618

commit fb35cc5796f051b0f83867417686746649eea618
Author: kochi <kochi@chromium.org>
Date: Fri Nov 18 09:06:42 2016

Use static profile in SelectorQueryTest

This change should have been included in
https://codereview.chromium.org/2493003003/

Only one instance of CSSParserContext constructor was rewritten
to use static profile, and the rest was unchanged.

BUG= 633007 

Review-Url: https://codereview.chromium.org/2514743002
Cr-Commit-Position: refs/heads/master@{#433156}

[modify] https://crrev.com/fb35cc5796f051b0f83867417686746649eea618/third_party/WebKit/Source/core/dom/SelectorQueryTest.cpp

Comment 17 by ericbidelman@chromium.org, Nov 20 2016 

For those following, I've written a little helper to find all nodes on the page. It works for shadow roots created with v0 and v1 and for browsers that do not support shadow dom:

https://gist.github.com/ebidel/fc1302d5fa1ef7c6d42fe8189acc3820

Comment 19 by kochi@chromium.org, Nov 21 2016

Status: Fixed (was: Started)
Now all changes landed and soon in canary you can use '>>>' in
querySelector(All) with chrome://flags/#enable-experimental-web-platform-features
enabled.

I plan to merge the changes to M55 in the next week.
The following 3 CLs needs merging:
69635dc029d7ce77e7db12669a994295d5f93952
04a4852c265670de8100ceee3e20215f4a56091f
ea3f73b7066df2f90e2ea56d280f6646760d4f35

@ericbidelman thanks for writing up the helper function!

Comment 20 by kochi@chromium.org, Nov 21 2016 

Oops, plan to merge the changes to *M56*, not to M55.
Project Member

Comment 21 by bugdroid1@chromium.org, Nov 24 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/23e53e844095c53dfa18d0b30b04f577c25cb897

commit 23e53e844095c53dfa18d0b30b04f577c25cb897
Author: kochi <kochi@chromium.org>
Date: Thu Nov 24 16:25:59 2016

Add a test for multiple >>>s in a selector.

Add a test case for matching multiple '>>>'
(shadow-piercing descendant combinator)s.

BUG= 633007 

Review-Url: https://codereview.chromium.org/2523673002
Cr-Commit-Position: refs/heads/master@{#434339}

[modify] https://crrev.com/23e53e844095c53dfa18d0b30b04f577c25cb897/third_party/WebKit/LayoutTests/shadow-dom/shadow-piercing-descendant-combinator.html

Comment 22 by kochi@chromium.org, Nov 28 2016

Labels: M-56 Merge-Request-56

Comment 23 by dimu@chromium.org, Nov 28 2016

Labels: -Merge-Request-56 Merge-Approved-56 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M56 (branch: 2924)
Project Member

Comment 24 by bugdroid1@chromium.org, Nov 28 2016

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/136e6bae2344ec661fb566a1740ad1ef4570351d

commit 136e6bae2344ec661fb566a1740ad1ef4570351d
Author: Takayoshi Kochi <kochi@chromium.org>
Date: Mon Nov 28 05:53:01 2016

Parser support for >>> (shadow-piercing descendant) combinator.

'>>>' (shadow-piercing descendant) can only be parsed in
static profile. The parsing is guarded by a runtime enabled flag.

Matching part is done in the next CL:
https://codereview.chromium.org/2496123002/

BUG= 633007 

Review-Url: https://codereview.chromium.org/2500813003
Cr-Commit-Position: refs/heads/master@{#433133}
(cherry picked from commit 69635dc029d7ce77e7db12669a994295d5f93952)

Review URL: https://codereview.chromium.org/2528303002 .

Cr-Commit-Position: refs/branch-heads/2924@{#106}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/136e6bae2344ec661fb566a1740ad1ef4570351d/third_party/WebKit/Source/core/css/CSSSelector.cpp
[modify] https://crrev.com/136e6bae2344ec661fb566a1740ad1ef4570351d/third_party/WebKit/Source/core/css/CSSSelector.h
[modify] https://crrev.com/136e6bae2344ec661fb566a1740ad1ef4570351d/third_party/WebKit/Source/core/css/SelectorChecker.cpp
[modify] https://crrev.com/136e6bae2344ec661fb566a1740ad1ef4570351d/third_party/WebKit/Source/core/css/SelectorFilter.cpp
[modify] https://crrev.com/136e6bae2344ec661fb566a1740ad1ef4570351d/third_party/WebKit/Source/core/css/parser/CSSParserMode.cpp
[modify] https://crrev.com/136e6bae2344ec661fb566a1740ad1ef4570351d/third_party/WebKit/Source/core/css/parser/CSSParserMode.h
[modify] https://crrev.com/136e6bae2344ec661fb566a1740ad1ef4570351d/third_party/WebKit/Source/core/css/parser/CSSSelectorParser.cpp
[modify] https://crrev.com/136e6bae2344ec661fb566a1740ad1ef4570351d/third_party/WebKit/Source/core/css/parser/CSSSelectorParserTest.cpp
[modify] https://crrev.com/136e6bae2344ec661fb566a1740ad1ef4570351d/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in
Project Member

Comment 25 by bugdroid1@chromium.org, Nov 28 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d36ebe53ac7fa1fada4bc0e68bd8cf12293489ab

commit d36ebe53ac7fa1fada4bc0e68bd8cf12293489ab
Author: Takayoshi Kochi <kochi@chromium.org>
Date: Mon Nov 28 05:58:15 2016

Matching part for >>> (shadow-piercing descendant combinator).

Parsing part is done in the previous dependent CL.
https://codereview.chromium.org/2500813003/

This CL implements matching '>>>' shadow-piercing descendant
combinator in static profile.

Note that the combinator only pierces through open shadow roots
and not V0 or closed shadow roots.

BUG= 633007 

Review-Url: https://codereview.chromium.org/2496123002
Cr-Commit-Position: refs/heads/master@{#433457}
(cherry picked from commit 04a4852c265670de8100ceee3e20215f4a56091f)

Review URL: https://codereview.chromium.org/2532813002 .

Cr-Commit-Position: refs/branch-heads/2924@{#107}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/d36ebe53ac7fa1fada4bc0e68bd8cf12293489ab/third_party/WebKit/LayoutTests/shadow-dom/query-selector.html
[add] https://crrev.com/d36ebe53ac7fa1fada4bc0e68bd8cf12293489ab/third_party/WebKit/LayoutTests/shadow-dom/shadow-piercing-descendant-combinator-in-static-profile.html
[add] https://crrev.com/d36ebe53ac7fa1fada4bc0e68bd8cf12293489ab/third_party/WebKit/LayoutTests/shadow-dom/shadow-piercing-descendant-combinator.html
[modify] https://crrev.com/d36ebe53ac7fa1fada4bc0e68bd8cf12293489ab/third_party/WebKit/Source/core/css/CSSSelector.cpp
[modify] https://crrev.com/d36ebe53ac7fa1fada4bc0e68bd8cf12293489ab/third_party/WebKit/Source/core/css/SelectorChecker.cpp

Comment 27 by kochi@chromium.org, Nov 28 2016 

All necessary changes are merged to M56.

Comment 29 by kochi@chromium.org, Nov 28 2016 

Let me work on the fix immediately.

Comment 30 by kochi@chromium.org, Nov 28 2016

Blocking: 668933

Comment 31 by kochi@chromium.org, Nov 28 2016

Status: Started (was: Fixed)

Comment 32 by vabr@chromium.org, Nov 28 2016 

Thanks!
(Acknowledging that I will not attempt to revert it.)
Project Member

Comment 33 by bugdroid1@chromium.org, Nov 28 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5c81a6afca093ccd23046188ee231bba4b3f4c84

commit 5c81a6afca093ccd23046188ee231bba4b3f4c84
Author: kochi <kochi@chromium.org>
Date: Mon Nov 28 10:16:32 2016

Fix build on 2924 branch

3 Merges for  crbug.com/633007  on M56 branch (2924) had a glich
that caused compile error.

This fixes the compilation error.

TBR=tkent@chromium.org
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
BUG= 633007 ,  668933 

Review-Url: https://codereview.chromium.org/2533743002
Cr-Commit-Position: refs/branch-heads/2924@{#110}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/5c81a6afca093ccd23046188ee231bba4b3f4c84/third_party/WebKit/Source/core/css/SelectorChecker.cpp

Comment 34 by kochi@chromium.org, Nov 28 2016 

Committed the fix.  Will look after the builders until it becomes green.

Comment 35 by kochi@chromium.org, Nov 28 2016

Status: Fixed (was: Started)
Confirmed it builds successfully on win beta builder and others.

https://uberchromegw.corp.google.com/i/official.desktop.continuous/builders/win%20beta/builds/5387

Comment 36 by kochi@chromium.org, Dec 2 2016

Blockedon: 670580

Comment 37 by kochi@chromium.org, Dec 28 2016 

For those who are following this issue about /deep/ and '>>>' combinator,
here are the status as of now:

- /deep/ is for Shadow DOM v0 and still available in Chrome, but deprecated.
- '>>>' is implemented for querySelector (not for CSS) behind the flag
  (chrome://flags/#enable-experimental-web-platform-features) on M56+.
  Only works for V1 open shadows.
- https://github.com/w3c/webcomponents/issues/78 is the active thread to
  discuss this in the spec.
  Apple recently proposed an idea of new API
  (collectMatchingElementsInFlatTree).

Please follow GitHub  issue#78  for any further updates.
►

Sign in to add a comment