Bug 100582 - Get rid of StyleResolver state related to unknown pseudo-elements.
Summary: Get rid of StyleResolver state related to unknown pseudo-elements.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dimitri Glazkov (Google)
URL:
Keywords:
Depends on:
Blocks: 89879
  Show dependency treegraph
 
Reported: 2012-10-26 17:06 PDT by Dimitri Glazkov (Google)
Modified: 2012-10-28 15:35 PDT (History)
7 users (show)

See Also:


Attachments
Patch (8.00 KB, patch)
2012-10-26 17:13 PDT, Dimitri Glazkov (Google)
no flags Details | Formatted Diff | Diff
Patch (16.53 KB, patch)
2012-10-28 09:09 PDT, Dimitri Glazkov (Google)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dimitri Glazkov (Google) 2012-10-26 17:06:30 PDT
Get rid of StyleResolver state related to unknown pseudo-elements.
Comment 1 Dimitri Glazkov (Google) 2012-10-26 17:13:43 PDT
Created attachment 171056 [details]
Patch
Comment 2 Eric Seidel (no email) 2012-10-26 17:59:06 PDT
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...
Comment 3 Dimitri Glazkov (Google) 2012-10-26 18:49:00 PDT
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?
Comment 4 Eric Seidel (no email) 2012-10-26 20:41:18 PDT
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? :)
Comment 5 Dimitri Glazkov (Google) 2012-10-26 20:50:30 PDT
(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? :)
Comment 6 Dimitri Glazkov (Google) 2012-10-27 21:12:03 PDT
Comment on attachment 171056 [details]
Patch

I have a much better patch. Please hold.
Comment 7 Dimitri Glazkov (Google) 2012-10-28 09:09:24 PDT
Created attachment 171130 [details]
Patch
Comment 8 Eric Seidel (no email) 2012-10-28 12:50:25 PDT
Comment on attachment 171130 [details]
Patch

LGTM.
Comment 9 WebKit Review Bot 2012-10-28 15:35:51 PDT
Comment on attachment 171130 [details]
Patch

Clearing flags on attachment: 171130

Committed r132754: <http://trac.webkit.org/changeset/132754>
Comment 10 WebKit Review Bot 2012-10-28 15:35:55 PDT
All reviewed patches have been landed.  Closing bug.