Bug 161174 - Land support for tokenization in the new CSS Parser
Summary: Land support for tokenization in the new CSS Parser
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: 46591
  Show dependency treegraph
 
Reported: 2016-08-24 16:43 PDT by Dave Hyatt
Modified: 2016-09-22 13:37 PDT (History)
3 users (show)

See Also:


Attachments
Patch (430.92 KB, patch)
2016-08-24 16:45 PDT, Dave Hyatt
buildbot: commit-queue-
Details | Formatted Diff | Diff
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 Details
Patch to fix layout tests and style stuff (441.63 KB, patch)
2016-08-25 11:34 PDT, Dave Hyatt
no flags Details | Formatted Diff | Diff
Patch to fix layout tests and style stuff (440.62 KB, patch)
2016-08-25 11:44 PDT, Dave Hyatt
no flags Details | Formatted Diff | Diff
Patch to fix layout tests and style stuff (439.87 KB, patch)
2016-08-25 13:09 PDT, Dave Hyatt
no flags Details | Formatted Diff | Diff
Patch (440.41 KB, patch)
2016-08-25 14:21 PDT, Dave Hyatt
no flags Details | Formatted Diff | Diff
Patch (440.39 KB, patch)
2016-08-25 14:30 PDT, Dave Hyatt
no flags Details | Formatted Diff | Diff
Patch (442.00 KB, patch)
2016-08-25 15:10 PDT, Dave Hyatt
no flags Details | Formatted Diff | Diff
Patch (443.42 KB, patch)
2016-08-25 16:54 PDT, Dave Hyatt
simon.fraser: 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-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.
Comment 1 Dave Hyatt 2016-08-24 16:45:35 PDT
Created attachment 286909 [details]
Patch
Comment 2 WebKit Commit Bot 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.
Comment 3 Simon Fraser (smfr) 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.
Comment 4 Build Bot 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
Comment 5 Build Bot 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
Comment 6 Darin Adler 2016-08-25 10:01:35 PDT
Normally we don't keep the TODO, we just use FIXME or nothing at all.
Comment 7 Dave Hyatt 2016-08-25 11:34:03 PDT
Created attachment 286990 [details]
Patch to fix layout tests and style stuff
Comment 8 Dave Hyatt 2016-08-25 11:44:34 PDT
Created attachment 286992 [details]
Patch to fix layout tests and style stuff
Comment 9 WebKit Commit Bot 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.
Comment 10 Dave Hyatt 2016-08-25 13:09:55 PDT
Created attachment 286998 [details]
Patch to fix layout tests and style stuff
Comment 11 Dave Hyatt 2016-08-25 14:21:30 PDT
Created attachment 287016 [details]
Patch
Comment 12 Dave Hyatt 2016-08-25 14:30:19 PDT
Created attachment 287019 [details]
Patch
Comment 13 Dave Hyatt 2016-08-25 15:10:31 PDT
Created attachment 287029 [details]
Patch
Comment 14 Dave Hyatt 2016-08-25 16:54:51 PDT
Created attachment 287046 [details]
Patch
Comment 15 Simon Fraser (smfr) 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
Comment 16 Dave Hyatt 2016-08-28 08:48:10 PDT
Landed in r205103.