Bug 161749

Summary: [CSS Parser] Add support for new CSS selector parsing
Product: WebKit Reporter: Dave Hyatt <hyatt>
Component: CSSAssignee: Dave Hyatt <hyatt>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 46591    
Attachments:
Description Flags
Patch for bots
none
Patch
none
Patch dino: review+

Dave Hyatt
Reported 2016-09-08 11:56:44 PDT
Add support for new CSS selector parsing to the new parser. This is imported from Blink but heavily modified to work with our existing selector structures.
Attachments
Patch for bots (92.41 KB, patch)
2016-09-08 12:17 PDT, Dave Hyatt
no flags
Patch (98.75 KB, patch)
2016-09-08 12:28 PDT, Dave Hyatt
no flags
Patch (99.18 KB, patch)
2016-09-08 12:33 PDT, Dave Hyatt
dino: review+
Dave Hyatt
Comment 1 2016-09-08 12:17:51 PDT
Created attachment 288301 [details] Patch for bots
WebKit Commit Bot
Comment 2 2016-09-08 12:19:26 PDT
Attachment 288301 [details] did not pass style-queue: ERROR: Source/WebCore/css/parser/CSSParserValues.cpp:400: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WebCore/css/parser/CSSParserValues.cpp:512: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:62: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:68: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:85: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:92: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:114: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:116: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:143: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:154: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:157: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:159: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:218: Non-label code inside switch statements should be indented. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:218: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:229: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:274: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:276: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:293: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:466: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:499: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:513: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:524: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:734: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:766: Use 'WTFMove()' instead of 'std::move()'. [runtime/wtf_move] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.h:32: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 25 in 34 files If any of these errors are false positives, please file a bug against check-webkit-style.
Dave Hyatt
Comment 3 2016-09-08 12:28:13 PDT
WebKit Commit Bot
Comment 4 2016-09-08 12:29:46 PDT
Attachment 288304 [details] did not pass style-queue: ERROR: Source/WebCore/css/parser/CSSSelectorParser.cpp:155: Should have a space between // and comment [whitespace/comments] [4] ERROR: Source/WebCore/css/parser/CSSSelectorParser.h:32: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 35 files If any of these errors are false positives, please file a bug against check-webkit-style.
Dave Hyatt
Comment 5 2016-09-08 12:33:27 PDT
Dean Jackson
Comment 6 2016-09-08 13:24:13 PDT
Comment on attachment 288306 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=288306&action=review > Source/WebCore/ChangeLog:8 > + [CSS Parser] Add support for new CSS selector parsing > + https://bugs.webkit.org/show_bug.cgi?id=161749 > + > + Reviewed by NOBODY (OOPS!). > + > + * CMakeLists.txt: Maybe add some kind of summary? :) > Source/WebCore/css/SelectorChecker.cpp:465 > } > > + > ASSERT_NOT_REACHED(); Oops. > Source/WebCore/css/parser/CSSParserMode.h:57 > + return mode == HTMLQuirksMode; // || mode == HTMLAttributeMode; Did you mean to leave this comment? > Source/WebCore/css/parser/CSSParserMode.h:84 > inline bool isStrictParserMode(CSSParserMode cssParserMode) > { > - return cssParserMode == CSSStrictMode || cssParserMode == SVGAttributeMode; > + return cssParserMode == UASheetMode || cssParserMode == HTMLStandardMode || cssParserMode == SVGAttributeMode; > } Isn't this the same as you just added to CSSParser? > Source/WebCore/css/parser/CSSSelectorParser.cpp:102 > +namespace { Why do you do this? > Source/WebCore/css/parser/CSSSelectorParser.cpp:136 > + return nullptr; > + > + > + unsigned previousCompoundFlags = 0; Nit: Double blank lines. > Source/WebCore/css/parser/CSSSelectorParser.cpp:166 > +namespace { And this one?
Dave Hyatt
Comment 7 2016-09-08 14:11:15 PDT
Landed in r205660.
Note You need to log in before you can comment on or make changes to this bug.