Bug 77528 - <style scoped>: Implement scoped selector matching in the slow path
Summary: <style scoped>: Implement scoped selector matching in the slow path
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Roland Steiner
URL:
Keywords:
Depends on: 77525
Blocks: 73192 77527
  Show dependency treegraph
 
Reported: 2012-02-01 01:37 PST by Roland Steiner
Modified: 2012-02-16 01:31 PST (History)
4 users (show)

See Also:


Attachments
Patch (31.27 KB, patch)
2012-02-10 01:58 PST, Roland Steiner
koivisto: review+
koivisto: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Roland Steiner 2012-02-01 01:37:37 PST
Implement scoped selector matching, but don't allow rules from scoped stylesheets to use the fast selector checking path for the time being.
Comment 1 Roland Steiner 2012-02-10 01:58:14 PST
Created attachment 126478 [details]
Patch

Patch, doing the slow path only, incorporating the changes to SelectorChecker compared to the old patches in bug 73192, also somewhat simplifed logic.
Comment 2 Antti Koivisto 2012-02-13 15:50:56 PST
Comment on attachment 126478 [details]
Patch

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

r=me, but consider the comments

> Source/WebCore/css/CSSStyleSelector.cpp:176
> +    RuleData(CSSStyleRule*, CSSSelector*, unsigned position, bool canUseFastChecking = true);

Something like canUseFastCheckSelector would be a better name here and elsewhere (so it is clear what is being checked)

> Source/WebCore/css/CSSStyleSelector.h:283
> +#if ENABLE(STYLE_SCOPED)
> +        MatchOptions(bool includeEmptyRules, const Element* scope = 0) : scope(scope), includeEmptyRules(includeEmptyRules) { }
> +        const Element* scope;
> +        bool includeEmptyRules;
> +#else
> +        MatchOptions(bool includeEmptyRules) : includeEmptyRules(includeEmptyRules) { }
> +        bool includeEmptyRules;
> +#endif

I think you can remove #if ENABLE(STYLE_SCOPED) here and just default the scope to null.

> Source/WebCore/css/SelectorChecker.cpp:490
> -        for (; nextContext.element; nextContext.element = nextContext.element->parentElement()) {
> +        for (;;) {
> +            if (nextContext.element == nextContext.scope)
> +                return SelectorFailsCompletely;
> +            nextContext.element = nextContext.element->parentElement();
> +            if (!nextContext.element)
> +                return SelectorFailsCompletely;

You can keep the for loop as is if you do the following...

> Source/WebCore/css/SelectorChecker.cpp:498
>      case CSSSelector::Child:
> +        if (nextContext.element == nextContext.scope)
> +            return SelectorFailsCompletely;

This test (current element is the scope) is repeated for all cases except SubSelector. I think it could be moved before the nextContext initialization with a relation != SubSelector test.
Comment 3 Roland Steiner 2012-02-16 01:31:32 PST
Committed r107911: <http://trac.webkit.org/changeset/107911>