Bug 163649 - [CSS Parser] Fix compound selector parsing.
Summary: [CSS Parser] Fix compound selector parsing.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-18 20:55 PDT by Dave Hyatt
Modified: 2016-10-19 08:10 PDT (History)
0 users

See Also:


Attachments
Patch (11.79 KB, patch)
2016-10-18 20:58 PDT, Dave Hyatt
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dave Hyatt 2016-10-18 20:55:37 PDT
Fix compound selector parsing. The new parser makes the assumption that SubSelector is the first value in the RelationType enum. Although this is kind of lame, it's easy for us to move it to the correct spot to fix.
Comment 1 Dave Hyatt 2016-10-18 20:58:52 PDT
Created attachment 292033 [details]
Patch
Comment 2 Darin Adler 2016-10-19 00:50:56 PDT
Comment on attachment 292033 [details]
Patch

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

> Source/WebCore/css/CSSSelector.h:84
> +        enum RelationType {

Should we change it to an enum class at some point?

> Source/WebCore/css/CSSSelector.h:85
> +            SubSelector,

That should be "Subselector" since it's one word.

Should we add a static_assert that subselector is 0 since the code assumes subselector is 0?

> Source/WebCore/css/SelectorChecker.cpp:637
> +        CSSSelector::RelationType relation = selector->relation();

I suggest auto instead.

> Source/WebCore/css/SelectorChecker.cpp:1204
> +        CSSSelector::RelationType relation = selector->relation();

I suggest auto instead.

> Source/WebCore/css/SelectorFilter.cpp:125
> +    CSSSelector::RelationType relation = selector->relation();

I suggest auto instead.

> Source/WebCore/css/parser/CSSSelectorParser.cpp:141
> +    while (CSSSelector::RelationType combinator = consumeCombinator(range)) {

I suggest auto instead.

> Source/WebCore/css/parser/CSSSelectorParser.cpp:538
> +    CSSSelector::RelationType fallbackResult = CSSSelector::SubSelector;

I suggest auto instead.

> Source/WebCore/cssjit/SelectorCompiler.cpp:1001
> +        CSSSelector::RelationType relation = selector->relation();

I suggest auto instead.
Comment 3 Dave Hyatt 2016-10-19 08:02:55 PDT
Yes, static_assert is a great idea here. Will also patch all the other places to use auto as suggested.
Comment 4 Dave Hyatt 2016-10-19 08:10:13 PDT
Landed in r207536.