Bug 81069 – Implement :scope for element.querySelector[All]()

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Roland Steiner]

If querySelector() or querySelectorAll() are called on an element, that element should act as the context node for :scope

[Roland Steiner]

Implementing this requires changing the code such that selector matching is allowed to exceed the scope/context element, e.g., as done in the patch for bug 79075.

[Bronislav Klucka]

(In reply to comment #1)
> Implementing this requires changing the code such that selector matching is allowed to exceed the scope/context element, e.g., as done in the patch for bug 79075.

Hello, what does it actually mean in terms of "when it will be done"? 

Brona

[Roland Steiner]

(In reply to comment #2)
> Hello, what does it actually mean in terms of "when it will be done"? 

It means we need to assess the benefits of this feature vs. the potential performance cost of its implementation. At least it will require some amount of refactoring for a nice implementation.

At the moment I don't have the spare cycles to implement this, but I'll get back to it in the (hopefully not so distant) future in case nobody else tackled it in the meantime.

[Roland Steiner]

Created attachment 134774 [details]
Layout test

Layout test (to go in LayoutTests/fast/dom/SelectorAPI/element-scope.html)

[Bronislav Klucka]

Any plans here?

[Bronislav Klucka]

hi, any news?

[Takashi Sakamoto]

Created attachment 191660 [details]
Patch

[Dimitri Glazkov (Google)]

Comment on attachment 191660 [details]
Patch

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

> Source/WebCore/css/SelectorChecker.h:83
> +        const Node* contextualReferenceNode;

Don't we already have scope stored? It seems bad to have both.

[Takashi Sakamoto]

Comment on attachment 191660 [details]
Patch

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

Thank you for reviewing.

>> Source/WebCore/css/SelectorChecker.h:83
>> +        const Node* contextualReferenceNode;
> 
> Don't we already have scope stored? It seems bad to have both.

If only scoped style uses "scope", I will use. However, the scope is also used by :distributed. So I would like to confirm whether we can use ":distributed" in element.querySelectorAll or something. If so, scope value set by distributed is the same as contextual reference node...? I will add hayato@ to CC.

[Takashi Sakamoto]

(In reply to comment #9)
> (From update of attachment 191660 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=191660&action=review
> 
> Thank you for reviewing.
> 
> >> Source/WebCore/css/SelectorChecker.h:83
> >> +        const Node* contextualReferenceNode;
> > 
> > Don't we already have scope stored? It seems bad to have both.
> 
> If only scoped style uses "scope", I will use. However, the scope is also used by :distributed. So I would like to confirm whether we can use ":distributed" in element.querySelectorAll or something. If so, scope value set by distributed is the same as contextual reference node...? I will add hayato@ to CC.

I found the following in http://www.w3.org/TR/selectors-api2/
"Authors are advised that while the use of pseudo-elements in selectors is permitted, they will not match any elements in the document, and thus would not result in any elements being returned. Therefore, authors are advised to avoid the use of pseudo-elements in selectors that are passed to the methods defined in this specification"

So querySelector with ::distributed returns no results. I will try to re-use "scope".
I have to add a new behavior to SelectorChecker::BehhaviorAtBoundary.

[Takashi Sakamoto]

Created attachment 192630 [details]
Patch

[Dimitri Glazkov (Google)]

Comment on attachment 192630 [details]
Patch

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

> Source/WebCore/css/SelectorChecker.h:51
> +    enum BehaviorAtBoundary { DoesNotCrossBoundary, CrossesBoundary, CrossesScopeNotTreeScope };

StaysWithinTreeScope?

[Takashi Sakamoto]

Created attachment 192645 [details]
Patch

[Takashi Sakamoto]

Comment on attachment 192630 [details]
Patch

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

>> Source/WebCore/css/SelectorChecker.h:51
>> +    enum BehaviorAtBoundary { DoesNotCrossBoundary, CrossesBoundary, CrossesScopeNotTreeScope };
> 
> StaysWithinTreeScope?

I see. Done.

[Dimitri Glazkov (Google)]

Okay. I'll let you land it when you're awake :).

[Dimitri Glazkov (Google)]

FWIW, I think that BehaviorAtBoundary enum looks slightly overloaded now. The difference between DoesNotCrossBoundary and StaysWithinTreeScope is too subtle.

[Takashi Sakamoto]

(In reply to comment #16)
> FWIW, I think that BehaviorAtBoundary enum looks slightly overloaded now. The difference between DoesNotCrossBoundary and StaysWithinTreeScope is too subtle.

I see. I would like to try this issue in another patch.

[WebKit Review Bot]

Comment on attachment 192645 [details]
Patch

Clearing flags on attachment: 192645

Committed r145691: <http://trac.webkit.org/changeset/145691>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.