RESOLVED FIXED 161174
Land support for tokenization in the new CSS Parser
https://bugs.webkit.org/show_bug.cgi?id=161174
Summary Land support for tokenization in the new CSS Parser
Dave Hyatt
Reported 2016-08-24 16:43:52 PDT
Land support for tokenization in the new CSS Parser. Bring along as much as is needed to get that going.
Attachments
Patch (430.92 KB, patch)
2016-08-24 16:45 PDT, Dave Hyatt
buildbot: commit-queue-
Archive of layout-test-results from ews113 for mac-yosemite (1.47 MB, application/zip)
2016-08-24 18:24 PDT, Build Bot
no flags
Patch to fix layout tests and style stuff (441.63 KB, patch)
2016-08-25 11:34 PDT, Dave Hyatt
no flags
Patch to fix layout tests and style stuff (440.62 KB, patch)
2016-08-25 11:44 PDT, Dave Hyatt
no flags
Patch to fix layout tests and style stuff (439.87 KB, patch)
2016-08-25 13:09 PDT, Dave Hyatt
no flags
Patch (440.41 KB, patch)
2016-08-25 14:21 PDT, Dave Hyatt
no flags
Patch (440.39 KB, patch)
2016-08-25 14:30 PDT, Dave Hyatt
no flags
Patch (442.00 KB, patch)
2016-08-25 15:10 PDT, Dave Hyatt
no flags
Patch (443.42 KB, patch)
2016-08-25 16:54 PDT, Dave Hyatt
simon.fraser: review+
Dave Hyatt
Comment 1 2016-08-24 16:45:35 PDT
WebKit Commit Bot
Comment 2 2016-08-24 16:47:58 PDT
Attachment 286909 [details] did not pass style-queue: ERROR: Source/WebCore/css/CSSMarkup.cpp:94: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/WebCore/css/CSSMarkup.cpp:101: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/WebCore/css/CSSMarkup.cpp:118: One line control clauses should not use braces. [whitespace/braces] [4] ERROR: Source/WebCore/css/parser/CSSPropertyParser.h:26: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/css/parser/CSSParserTokenRange.h:44: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserTokenRange.h:45: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserTokenRange.h:95: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserTokenRange.h:96: Wrong number of spaces before statement. (expected: 8) [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSPropertyParser.cpp:32: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/css/parser/CSSPropertyParser.cpp:36: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/css/parser/CSSPropertyParser.cpp:62: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/css/parser/CSSPropertyParser.cpp:70: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/css/parser/CSSPropertyParser.cpp:93: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/WebCore/css/parser/CSSPropertyParser.cpp:150: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/WebCore/css/parser/CSSParserObserver.h:38: Missing space inside { }. [whitespace/braces] [5] ERROR: Source/WebCore/css/parser/CSSParserFastPaths.cpp:33: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/css/parser/CSSParserFastPaths.cpp:38: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/CMakeLists.txt:1414: Alphabetical sorting problem. "css/parser/CSSParserFastPaths.cpp" should be before "css/parser/CSSParserValues.cpp". [list/order] [5] ERROR: Source/WebCore/css/parser/CSSTokenizer.h:36: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/css/parser/CSSTokenizer.cpp:42: Wrong number of spaces before statement. (expected: 4) [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSTokenizer.cpp:72: Wrong number of spaces before statement. (expected: 4) [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSTokenizer.cpp:87: One line control clauses should not use braces. [whitespace/braces] [4] ERROR: Source/WebCore/css/parser/CSSTokenizer.cpp:513: One line control clauses should not use braces. [whitespace/braces] [4] ERROR: Source/WebCore/css/parser/CSSTokenizer.cpp:631: One line control clauses should not use braces. [whitespace/braces] [4] ERROR: Source/WebCore/css/parser/CSSTokenizer.cpp:825: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/WebCore/css/parser/CSSPropertyParserHelpers.h:34: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:36: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:47: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:49: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:55: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:57: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:65: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:85: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:93: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:103: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:113: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:115: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:131: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:145: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:147: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:149: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:171: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:173: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:189: A case label should not be indented, but line up with its switch statement. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:351: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:352: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:355: Should be indented on a separate line, with the colon or comma first on that line. [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:356: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:360: Wrong number of spaces before statement. (expected: 16) [whitespace/indent] [4] Total errors found: 49 in 25 files If any of these errors are false positives, please file a bug against check-webkit-style.
Simon Fraser (smfr)
Comment 3 2016-08-24 17:05:48 PDT
Comment on attachment 286909 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=286909&action=review > Source/WebCore/ChangeLog:8 > + Reviewed by NOBODY (OOPS!). > + > + * CMakeLists.txt: You should say this comes from Blink, and cite the Blink revision. > Source/WebCore/css/CSSMarkup.cpp:2 > +/* > + * Copyright (C) 2003 Lars Knoll (knoll@kde.org) Not sure if we need to do anything with licenses. Please check. > Source/WebCore/css/parser/CSSParserObserver.h:46 > + // TODO(timloh): Unused, should be removed Remove the "(timloh)" here and elsewhere.
Build Bot
Comment 4 2016-08-24 18:24:37 PDT
Comment on attachment 286909 [details] Patch Attachment 286909 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1936944 New failing tests: fast/css/selector-text-escape.html fast/css/parsing-css-nonascii.html
Build Bot
Comment 5 2016-08-24 18:24:40 PDT
Created attachment 286926 [details] Archive of layout-test-results from ews113 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews113 Port: mac-yosemite Platform: Mac OS X 10.10.5
Darin Adler
Comment 6 2016-08-25 10:01:35 PDT
Normally we don't keep the TODO, we just use FIXME or nothing at all.
Dave Hyatt
Comment 7 2016-08-25 11:34:03 PDT
Created attachment 286990 [details] Patch to fix layout tests and style stuff
Dave Hyatt
Comment 8 2016-08-25 11:44:34 PDT
Created attachment 286992 [details] Patch to fix layout tests and style stuff
WebKit Commit Bot
Comment 9 2016-08-25 11:49:22 PDT
Attachment 286992 [details] did not pass style-queue: ERROR: Source/WebCore/css/parser/CSSPropertyParser.cpp:32: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/css/parser/CSSPropertyParser.cpp:68: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/css/parser/CSSPropertyParser.cpp:70: Alphabetical sorting problem. [build/include_order] [4] ERROR: Source/WebCore/css/parser/CSSTokenizer.h:33: Bad include order. Mixing system and custom headers. [build/include_order] [4] ERROR: Source/WebCore/css/parser/CSSParserToken.cpp:349: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] Total errors found: 5 in 27 files If any of these errors are false positives, please file a bug against check-webkit-style.
Dave Hyatt
Comment 10 2016-08-25 13:09:55 PDT
Created attachment 286998 [details] Patch to fix layout tests and style stuff
Dave Hyatt
Comment 11 2016-08-25 14:21:30 PDT
Dave Hyatt
Comment 12 2016-08-25 14:30:19 PDT
Dave Hyatt
Comment 13 2016-08-25 15:10:31 PDT
Dave Hyatt
Comment 14 2016-08-25 16:54:51 PDT
Simon Fraser (smfr)
Comment 15 2016-08-25 16:56:55 PDT
Comment on attachment 287046 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=287046&action=review > Source/WebCore/css/CSSMarkup.cpp:4 > + * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2016 Apple Inc. All rights reserved. This can be changed to year ranges: 2004-2012, 2016
Dave Hyatt
Comment 16 2016-08-28 08:48:10 PDT
Landed in r205103.
Note You need to log in before you can comment on or make changes to this bug.