Invalid block doesn't make declaration invalid.

Source/core/css/CSSGrammar.y.in

 /*
  *  Copyright (C) 2002-2003 Lars Knoll (knoll@kde.org)
  *  Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013 Apple Inc. All rights reserved.
  *  Copyright (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  *  Copyright (C) 2008 Eric Seidel <eric@webkit.org>
  *  Copyright (C) 2012 Intel Corporation. All rights reserved.
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Lesser General Public
  *  License as published by the Free Software Foundation; either
@@ skipping 69 matching lines @@
     case VAR_DEFINITION:
     case UNICODERANGE:
         return true;
     default:
         return false;
     }
 }
 
 %}
 
-%expect 38
+%expect 35
 
 %nonassoc LOWEST_PREC
 
 %left UNIMPORTANT_TOK
 
 %token WHITESPACE SGML_CD
 %token TOKEN_EOF 0
 
 %token INCLUDES
 %token DASHMATCH
@@ skipping 206 matching lines @@
 %type <valueList> calc_func_expr
 %type <valueList> calc_func_expr_list
 %type <valueList> calc_func_paren_expr
 %type <value> calc_function
 %type <string> min_or_max
 %type <value> min_or_max_function
 
 %type <string> element_name
 %type <string> attr_name
 
-%type <location> errors
 %type <location> error_location
 
 %%
 
 stylesheet:
     maybe_space maybe_charset maybe_sgml rule_list
   | internal_decls maybe_space
   | internal_rule maybe_space
   | internal_selector maybe_space
   | internal_value maybe_space
@@ skipping 1191 matching lines @@
     | decl_list decl_list_recovery {
         $$ = $1;
     }
     ;
 
 decl_list:
     declaration ';' maybe_space {
         parser->startProperty();
         $$ = $1;
     }
-    | declaration error_location invalid_block_list maybe_space {

[SeRya, 2013/04/29 18:01:26]

What's wrong with this rule?
1. It covers "body { color: red {} background-color: blue; }". It must be
invalid.
2. It covers "color: red {}{}{}{}" but not "color: red {}#".

How to make sure any CSS code covered by this rule still covered by new grammar?
Let's introduce an invariant (#inv1): each rule reducable to declaration has a
counterpart ending with declaration_recovery which able to cover any
invalid_block_list maybe_space.

-        parser->syntaxError($2);
-        parser->startProperty();
-        $$ = false;
-    }
-    | declaration error_location invalid_block_list ';' maybe_space {

[SeRya, 2013/04/29 18:01:26]

Same as above. Just add that #inv1 guarantees this rule is removed correctly.

-        parser->syntaxError($2);
-        parser->startProperty();
-        $$ = false;
-    }
     | decl_list_recovery ';' maybe_space {
         parser->startProperty();
         $$ = false;
     }
     | decl_list declaration ';' maybe_space {
         parser->startProperty();
         $$ = $1;
         if ($2)
             $$ = $2;
     }
@@ skipping 27 matching lines @@
             $$ = parser->parseValue(static_cast<CSSPropertyID>($1), $5);
             if (!$$)
                 parser->rollbackLastProperties(parser->m_parsedProperties.size() - oldParsedProperties);
             else
                 isPropertyParsed = true;
             parser->m_valueList = nullptr;
         }
         parser->endProperty($5, isPropertyParsed);
     }
     |
-    property errors {

[SeRya, 2013/04/29 18:01:26]

Why this change? To inforce #inv1.
"error" is covered by "declaration_recovery" so any text covered by old code
still covered by new one.

-        parser->syntaxError($2);
+    property declaration_recovery {
         parser->endProperty(false, false, CSSParser::GeneralSyntaxError);
         $$ = false;
     }
     |
-    property ':' maybe_space errors expr prio {

[SeRya, 2013/04/29 18:01:26]

What's wrong with this rule?
1. It covers exactly one error. If next token followed by one generated error
generate error as well it would be covered by property ':' maybe_space errors.
2. In violates #inv1.

Why any case covered by this reule covered in new grammar?
"property ':' maybe_space declaration_recovery" just enough.

-        /* The default movable type template has letter-spacing: .none;  Handle this by looking for
-        error tokens at the start of an expr, recover the expr and then treat as an error, cleaning
-        up and deleting the shifted expr.  */
-        parser->syntaxError($4);
-        parser->endProperty(false, false);
-        $$ = false;
-    }
-    |
-    property ':' maybe_space expr prio error {
+    property ':' maybe_space expr prio declaration_recovery {
         /* When we encounter something like p {color: red !important fail;} we should drop the declaration */
         parser->endProperty(false, false);
         $$ = false;
     }
     |
-    IMPORTANT_SYM maybe_space error_location {

[SeRya, 2013/04/29 18:01:26]

What's wrong?
Violate #inv1.

"body { !important ; color: red; }" "body { !important }" as well as is covered
by changed variant.

+    IMPORTANT_SYM maybe_space declaration_recovery {
         /* Handle this case: div { text-align: center; !important } Just reduce away the stray !important. */
-        parser->syntaxError($3);
         parser->endProperty(false, false, CSSParser::GeneralSyntaxError);
         $$ = false;
     }
     |
-    property ':' maybe_space error_location {

[SeRya, 2013/04/29 18:01:26]

What's wrong with this rule:
1. It's redundant. "property ':' maybe_space declaration_recovery" covers cases
like "body { color: }" or "body { color: ; }

[SeRya, 2013/04/29 18:01:26]

What's wrong with this rule?
It's redundant. "div { font-family: }" is perfectly covered by "property ':'
maybe_space declaration_recovery".

-        /* div { font-family: } Just reduce away this property with no value. */
-        parser->syntaxError($4);
+    property ':' maybe_space declaration_recovery {
+        /* if we come across rules with invalid values like this case: p { weight: *; }, just discard the rule */
         parser->endProperty(false, false);
         $$ = false;
     }
-    |
-    property ':' maybe_space errors {

[SeRya, 2013/04/29 18:01:26]

Tranformed to "property ':' maybe_space declaration_recovery".

[SeRya, 2013/04/29 18:01:26]

Transformed to "property ':' maybe_space declaration_recovery"

-        /* if we come across rules with invalid values like this case: p { weight: *; }, just discard the rule */
-        parser->syntaxError($4);
-        parser->endProperty(false, false);
-        $$ = false;
-    }
-    |
-    property error_location invalid_block {

[SeRya, 2013/04/29 18:01:26]

It perfectly covered by "property declaration_recovery".

-        /* if we come across: div { color{;color:maroon} }, ignore everything within curly brackets */
-        parser->syntaxError($2);
-        parser->endProperty(false, false, CSSParser::GeneralSyntaxError);
-        $$ = false;
-    }
   ;
 
+declaration_recovery:
+    error error_location {
+        parser->syntaxError($2);
+    }
+    | declaration_recovery invalid_block
+    | declaration_recovery error
+  ;
+
 property:
     IDENT maybe_space {
         $$ = cssPropertyID($1);
     }
   ;
 
 prio:
     IMPORTANT_SYM maybe_space { $$ = true; }
     | /* empty */ { $$ = false; }
   ;
@@ skipping 351 matching lines @@
   | '{' error closing_brace {
         parser->invalidBlockHit();
     }
     ;
 
 invalid_block_list:
     invalid_block
   | invalid_block_list error invalid_block
 ;
 
-errors:
-    error error_location {
-        $$ = $2;
-    }
-    ;
-
 error_location: {
         $$ = parser->currentLocation();
     }
     ;
 
 %%