WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
161749
[CSS Parser] Add support for new CSS selector parsing
https://bugs.webkit.org/show_bug.cgi?id=161749
Summary
[CSS Parser] Add support for new CSS selector parsing
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
Details
Formatted Diff
Diff
Patch
(98.75 KB, patch)
2016-09-08 12:28 PDT
,
Dave Hyatt
no flags
Details
Formatted Diff
Diff
Patch
(99.18 KB, patch)
2016-09-08 12:33 PDT
,
Dave Hyatt
dino
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
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
Created
attachment 288304
[details]
Patch
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
Created
attachment 288306
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug