Bug 111314 – Web Inspector: Track CSS error location information.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Sergey Ryazanov]

In preparation to handle CSS diagnostics (http://cebug.com/140608) grammar needs to be changed to make syntax error reporting possible and track error location information.

[Sergey Ryazanov]

Created attachment 191228 [details]
Patch

[Sergey Ryazanov]

Comment on attachment 191228 [details]
Patch

Patch extracted from https://bugs.webkit.org/show_bug.cgi?id=110483

[Pavel Feldman]

Comment on attachment 191228 [details]
Patch

I still think you should change this grammar gradually, i.e. step by step.

[Sergey Ryazanov]

Created attachment 191243 [details]
Patch

[Pavel Feldman]

Comment on attachment 191243 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=191243&action=review

> Source/WebCore/ChangeLog:5
> +        Grammar has been refactored.

You should explain what is going to happen, link to the meta bug that lists all the steps that you are going to make.

> Source/WebCore/css/CSSParser.cpp:9308
> +}

As I suggested, you should start with landing the plumbing.

> Source/WebCore/css/CSSParser.cpp:10469
> +                getCurrentToken<SrcCharacterType>(&yylval->string);

This change seems to be unrelated to what you are doing.

[Sergey Ryazanov]

Created attachment 194079 [details]
Patch

[Pavel Feldman]

Comment on attachment 194079 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194079&action=review

> Source/WebCore/ChangeLog:10
> +        Test: inspector/console/console-css-warnings.html

Please explain what is changing and why.

> Source/WebCore/css/CSSGrammar.y.in:1515
> +    /* empty */ { $$ = false; }

Why did this change?

> Source/WebCore/css/CSSGrammar.y.in:1527
> +    | errors invalid_block_list error {

Here and below. Why did this change? It seems like you are replacing "error" with the sequence of "error" here.

[Sergey Ryazanov]

Created attachment 194194 [details]
Patch

[Sergey Ryazanov]

Comment on attachment 194079 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=194079&action=review

>> Source/WebCore/css/CSSGrammar.y.in:1515
>> +    /* empty */ { $$ = false; }
> 
> Why did this change?

Before this change the following CSS text "body {}" was handled by rule "declaration: error". It used to be acceptable before I added error reporting code. With old rgammar it'd indistinguishable from "body {#}".

>> Source/WebCore/css/CSSGrammar.y.in:1527
>> +    | errors invalid_block_list error {
> 
> Here and below. Why did this change? It seems like you are replacing "error" with the sequence of "error" here.

Because of special handling for built in nontermial 'error' resulting grammar is equivalent. Sequence of errors is always reduced to single error symbol. "error_location error invalid_block error" could be used with the same effect but this is shorter.

[WebKit Review Bot]

Comment on attachment 194194 [details]
Patch

Clearing flags on attachment: 194194

Committed r146452: <http://trac.webkit.org/changeset/146452>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.