Changeset 150803 in webkit

Timestamp:

May 28, 2013 7:31:11 AM (11 years ago)

Author:

sergio@webkit.org

Message:

Invalid block doesn't make declaration invalid
​https://bugs.webkit.org/show_bug.cgi?id=115709

Reviewed by Andreas Kling.

From Blink r149446 by <​serya@chromium.org>

Source/WebCore:

Added a new rule "declaration_recovery:" which covers any kind of
"invalid_block_list maybe_space" sequence. By appending this new
rule to every rule reductable to "declaration:" we can remove some
redundant rules and improve error recovery of some others that
were only accepting exactly one error.

One of the rules being removed was preventing invalid blocks in a
declaration to invalidate the whole declaration. The rule was
reporting the syntax error but at the same time accepting the
invalid block as part of a valid declaration.

• css/CSSGrammar.y.in:

LayoutTests:

• fast/css/parsing-error-recovery.html: Added 2 new test cases.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/css/parsing-error-recovery.html (modified) (2 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/css/CSSGrammar.y.in (modified) (6 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-05-28  Sergio Villar Senin  <svillar@igalia.com>
+
+        Invalid block doesn't make declaration invalid
+        https://bugs.webkit.org/show_bug.cgi?id=115709
+
+        Reviewed by Andreas Kling.
+
+        From Blink r149446 by <serya@chromium.org>
+
+        * fast/css/parsing-error-recovery.html: Added 2 new test cases.
+
 2013-05-28  Ádám Kallai  <kadam@inf.u-szeged.hu>
 

trunk/LayoutTests/fast/css/parsing-error-recovery.html

@@ -49 +49 @@
         }
 
+        #test6 {
+            display: none !important;
+            display: block !important {invalid_block};
+        }
+
+        #test7 {
+            display: none !important;
+        }
+        #test7 {
+            display: block !important {invalid_block}
+        }
+
         /* Successfully parsed */
         #last {
@@ -61 +73 @@
   <div class="to_be_hidden" id="test4">FAIL: Test 4</div>
   <div class="to_be_hidden" id="test5">FAIL: Test 5</div>
+  <div class="to_be_hidden" id="test6">FAIL: Test 6</div>
+  <div class="to_be_hidden" id="test7">FAIL: Test 7</div>
   <div class="to_be_shown" id="last">PASS</div>
 </body>

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-05-28  Sergio Villar Senin  <svillar@igalia.com>
+
+        Invalid block doesn't make declaration invalid
+        https://bugs.webkit.org/show_bug.cgi?id=115709
+
+        Reviewed by Andreas Kling.
+
+        From Blink r149446 by <serya@chromium.org>
+
+        Added a new rule "declaration_recovery:" which covers any kind of
+        "invalid_block_list maybe_space" sequence. By appending this new
+        rule to every rule reductable to "declaration:" we can remove some
+        redundant rules and improve error recovery of some others that
+        were only accepting exactly one error.
+
+        One of the rules being removed was preventing invalid blocks in a
+        declaration to invalidate the whole declaration. The rule was
+        reporting the syntax error but at the same time accepting the
+        invalid block as part of a valid declaration.
+
+        * css/CSSGrammar.y.in:
+
 2013-05-28  Andreas Kling  <akling@apple.com>
 

trunk/Source/WebCore/css/CSSGrammar.y.in

@@ -89 +89 @@
 
 #if ENABLE_SHADOW_DOM
-%expect 68
+%expect 65
 #else
-%expect 67
+%expect 64
 #endif
 
@@ -337 +337 @@
 %type <string> attr_name
 
-%type <location> errors
 %type <location> error_location
 
@@ -1546 +1545 @@
         $$ = $1;
     }
-    | declaration error_location invalid_block_list maybe_space {
-        parser->syntaxError($2);
-        $$ = false;
-    }
-    | declaration error_location invalid_block_list ';' maybe_space {
-        parser->syntaxError($2);
-        $$ = false;
-    }
     | decl_list_recovery ';' maybe_space {
         parser->markPropertyStart();
@@ -1605 +1596 @@
     }
     |
-    property errors {
-        parser->syntaxError($2);
+    property declaration_recovery {
         $$ = false;
     }
     |
-    property ':' maybe_space errors expr prio {
-        /* 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->markPropertyEnd(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->markPropertyEnd(false, false);
@@ -1625 +1606 @@
     }
     |
-    IMPORTANT_SYM maybe_space error_location {
+    IMPORTANT_SYM maybe_space declaration_recovery {
         /* Handle this case: div { text-align: center; !important } Just reduce away the stray !important. */
-        parser->syntaxError($3);
         $$ = false;
     }
     |
-    property ':' maybe_space error_location {
-        /* 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->markPropertyEnd(false, false);
         $$ = false;
     }
-    |
-    property ':' maybe_space errors {
-        /* if we come across rules with invalid values like this case: p { weight: *; }, just discard the rule */
-        parser->syntaxError($4);
-        parser->markPropertyEnd(false, false);
-        $$ = false;
-    }
-    |
-    property error_location invalid_block {
-        /* if we come across: div { color{;color:maroon} }, ignore everything within curly brackets */
+  ;
+
+declaration_recovery:
+    error error_location {
         parser->syntaxError($2);
-        $$ = false;
-    }
+    }
+    | declaration_recovery invalid_block
+    | declaration_recovery error
   ;
 
@@ -2002 +1976 @@
 ;
 
-errors:
-    error error_location {
-        $$ = $2;
-    }
-    ;
-
 error_location: {
         $$ = parser->currentLocation();