Bug 100582 – Get rid of StyleResolver state related to unknown pseudo-elements.

• Component: New Bugs
• Status: RESOLVED FIXED

[Dimitri Glazkov (Google)]

Get rid of StyleResolver state related to unknown pseudo-elements.

[Dimitri Glazkov (Google)]

Created attachment 171056 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 171056 [details]
Patch

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

> Source/WebCore/css/StyleResolver.h:364
> +    bool checkSelector(const RuleData&, const ContainerNode* scope, RuleApplicability);

Should this default to CanBeAnything? (Which is a confusing name, btw.)

Maybe these should be RestrictToUnknownPseudoElements, NoRestrictions?

I"m not fully sure what this does to provide better naming help...

[Dimitri Glazkov (Google)]

Comment on attachment 171056 [details]
Patch

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

>> Source/WebCore/css/StyleResolver.h:364
>> +    bool checkSelector(const RuleData&, const ContainerNode* scope, RuleApplicability);
> 
> Should this default to CanBeAnything? (Which is a confusing name, btw.)
> 
> Maybe these should be RestrictToUnknownPseudoElements, NoRestrictions?
> 
> I"m not fully sure what this does to provide better naming help...

Sure, I'd love naming help:

CanBeAnything --> there are no restrictions in this TreeScope (which is like a little document-type subtree -- think Shadow DOM subtree), the rule can just do normal matching
NeedsUnknownPseudoElement --> this rule is very likely _not_ to match, because it's probably in a different TreeScope, but just in case it contains an unknown pseudo-element, it _could_ match, so we have to check the full selector.

Does this make a bit more sense?

[Eric Seidel (no email)]

Comment on attachment 171056 [details]
Patch

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

>>> Source/WebCore/css/StyleResolver.h:364
>>> +    bool checkSelector(const RuleData&, const ContainerNode* scope, RuleApplicability);
>> 
>> Should this default to CanBeAnything? (Which is a confusing name, btw.)
>> 
>> Maybe these should be RestrictToUnknownPseudoElements, NoRestrictions?
>> 
>> I"m not fully sure what this does to provide better naming help...
> 
> Sure, I'd love naming help:
> 
> CanBeAnything --> there are no restrictions in this TreeScope (which is like a little document-type subtree -- think Shadow DOM subtree), the rule can just do normal matching
> NeedsUnknownPseudoElement --> this rule is very likely _not_ to match, because it's probably in a different TreeScope, but just in case it contains an unknown pseudo-element, it _could_ match, so we have to check the full selector.
> 
> Does this make a bit more sense?

What's an unknown pseudo element? :)

[Dimitri Glazkov (Google)]

(In reply to comment #4)
> What's an unknown pseudo element? :)

It's a pseudo-element that's known neither to standard CSS3/4 pseudo-elements nor WebKit's internal plumbing (like scrollbars pseudo-elements). An unknown pseudo-element in a selector means one of two things:

1) the author made a typo
2) the author is using custom pseudo-elements in shadow DOM (http://dvcs.w3.org/hg/webcomponents/raw-file/tip/spec/shadow/index.html#custom-pseudo-elements).

We use these in pretty much all of the built-in HTML controls (like "-webkit-slider-thumb" is used to style the little knob on the range slider). Did I succeed in explaining? :)

[Dimitri Glazkov (Google)]

Comment on attachment 171056 [details]
Patch

I have a much better patch. Please hold.

[Dimitri Glazkov (Google)]

Created attachment 171130 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 171130 [details]
Patch

LGTM.

[WebKit Review Bot]

Comment on attachment 171130 [details]
Patch

Clearing flags on attachment: 171130

Committed r132754: <http://trac.webkit.org/changeset/132754>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.