Bug 163649

Summary: [CSS Parser] Fix compound selector parsing.
Product: WebKit Reporter: Dave Hyatt <hyatt>
Component: CSSAssignee: Dave Hyatt <hyatt>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch darin: review+

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.