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

Issue 643316 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

querySelectorAll doesn't pierce shadow root with multiple selectors if one includes /deep/

Project Member Reported by michae...@chromium.org, Sep 1 2016

Issue description

Version: 54.0.2837.0
OS: Chrome

Caveat: i may have a poor understanding of how /deep/ is supposed to work inside Document.querySelectorAll.

I've found that the selectors string 'foo #bar, body /deep/ *' is equivalent to the selectors string 'body *', which is incorrect behavior AFAIK.

1. visit a page with shadow DOM:
   https://elements.polymer-project.org/browse?package=paper-elements&dom=shadow
2. open the console, ensure the context is "top"
3. execute document.querySelectorAll('body #foobar, body /deep/ *')

My understanding: this should return the union of:
 * body #foobar (which does not exist)
 * shadow-including descendants of body

Instead, the result is length 1, where the only element is the body's only normal descendant <app-shell>.

Similar test case:

https://elements.polymer-project.org/bower_components/paper-button/demo/index.html?dom=shadow

document.querySelectorAll('title #foobar, body /deep/ *')

Expected: All elements under <body> (shadow-piercing)
Actual: Light-DOM descendants of <body> (non-shadow-piercing), as if the selectors were 'title #foobar, body *'


One practical problem is that this:

  document.querySelectorAll('body, body /deep/ *')

(or something else in place of <body>) can easily be accidentally constructed as:

  document.querySelectorAll('body *, body /deep/ *')

which should still work, but doesn't.
 
Status: Assigned (was: Untriaged)
Thank you for reporting. It sounds a bug.
Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 2 2016

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

commit 78be6354d3b13dff165e2715957bab3a6d158098
Author: hayato <hayato@chromium.org>
Date: Fri Sep 02 08:43:22 2016

Fix the wrong usages of CSSSelectorList::selectorUsesXXX() functions

It looks that a wrong `selectorIndex` argument is passed to
CSSSelectorList::selectorUsesDeepCombinatorOrShadowPseudo(), and similar functions.

- The clients are passing the index of a *selector* in *selectors*, in the meaning used in the spec.
- The member functions are using this parameter as the index of  CSSSelecorList::m_selectorArray.

They are totally different if a selector list has more than one selectors.

This CL moved all functions to CSSSelector to prevent the wrong usage from happening
in the future. It is error-prone that CSSSelectorList has these functions.

This CL also fixes  bug 643316 .

BUG= 643316 

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

[add] https://crrev.com/78be6354d3b13dff165e2715957bab3a6d158098/third_party/WebKit/LayoutTests/shadow-dom/v0/deep-in-selectors.html
[modify] https://crrev.com/78be6354d3b13dff165e2715957bab3a6d158098/third_party/WebKit/Source/core/css/CSSSelector.cpp
[modify] https://crrev.com/78be6354d3b13dff165e2715957bab3a6d158098/third_party/WebKit/Source/core/css/CSSSelector.h
[modify] https://crrev.com/78be6354d3b13dff165e2715957bab3a6d158098/third_party/WebKit/Source/core/css/CSSSelectorList.cpp
[modify] https://crrev.com/78be6354d3b13dff165e2715957bab3a6d158098/third_party/WebKit/Source/core/css/CSSSelectorList.h
[modify] https://crrev.com/78be6354d3b13dff165e2715957bab3a6d158098/third_party/WebKit/Source/core/css/RuleSet.cpp
[modify] https://crrev.com/78be6354d3b13dff165e2715957bab3a6d158098/third_party/WebKit/Source/core/dom/SelectorQuery.cpp

Status: Fixed (was: Started)

Sign in to add a comment