Handle error recovery in @supports

https://bugs.webkit.org/show_bug.cgi?id=103934

Patch by Pablo Flouret <pablof@motorola.com> on 2013-02-12
Reviewed by Antti Koivisto.

Source/WebCore:

Tests 021, 024, 031, and 033 in
http://hg.csswg.org/test/file/5f94e4b03ed9/contributors/opera/submitted/css3-conditional
fail because there's no explicit error recovery in @support's grammar.
Opera and Firefox pass the tests.

No new tests, modified css3/supports{,-cssom}.html

* css/CSSGrammar.y.in:
* css/CSSParser.cpp:
(WebCore::CSSParser::createSupportsRule):
(WebCore::CSSParser::markSupportsRuleHeaderEnd):
(WebCore::CSSParser::popSupportsRuleData):
* css/CSSParser.h:

LayoutTests:

* css3/supports-cssom.html:
* css3/supports-expected.txt:
* css3/supports.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@142640 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 497b4b2cff5545a65fe226c3bf1eccbf927bfe8f..8e129b16453ca095f0040dca09aab939f7654cbd 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2013-02-12  Pablo Flouret  <pablof@motorola.com>
+
+        Handle error recovery in @supports
+        https://bugs.webkit.org/show_bug.cgi?id=103934
+
+        Reviewed by Antti Koivisto.
+
+        * css3/supports-cssom.html:
+        * css3/supports-expected.txt:
+        * css3/supports.html:
+
 2013-02-12  Emil A Eklund  <eae@chromium.org>
 
         TransformState::move should not round offset to int

diff --git a/LayoutTests/css3/supports-cssom.html b/LayoutTests/css3/supports-cssom.html
index 9897b60f005e271fc5e92aea8fb0e8278d09b06f..1931ac62068de15e15aca75fef48de42ba461d32 100644 (file)
--- a/LayoutTests/css3/supports-cssom.html
+++ b/LayoutTests/css3/supports-cssom.html
@@ -16,9 +16,25 @@
 
     @supports  ((  (     padding: 0)    and (display: none)) or (display: rainbow))  {
         dfn { width:0; }
+
         @supports (width: 0) {
             br { height:0; }
         }
+
+        /* Rules with syntax errors should be ignored. */
+        @supports (display: none);
+        @supports (display: none)) ;
+        @supports;
+        @supports ;
+        @supports (display: none)) {}
+        @supports (display: )) {}
+        @supports ((display: none) and {}
+        @supports (display: none) {}
+        );
+        @supports ((display: none) and ;
+        @supports (display: none) {}
+        );
+
         ol { display:none; }
     }
 

diff --git a/LayoutTests/css3/supports-expected.txt b/LayoutTests/css3/supports-expected.txt
index 41a76bac3f3bfb2fca4fb40dc8d04539b9af21c7..b182ac18ec7592065d30b428c022a90e90f35c9f 100644 (file)
--- a/LayoutTests/css3/supports-expected.txt
+++ b/LayoutTests/css3/supports-expected.txt
@@ -40,6 +40,14 @@ PASS getComputedStyle(document.getElementById('t33')).content is "APPLIED"
 PASS getComputedStyle(document.getElementById('t34')).content is "UNTOUCHED"
 PASS getComputedStyle(document.getElementById('t35')).content is "APPLIED"
 PASS getComputedStyle(document.getElementById('t36')).content is "UNTOUCHED"
+PASS getComputedStyle(document.getElementById('t37')).content is "APPLIED"
+PASS getComputedStyle(document.getElementById('t38')).content is "APPLIED"
+PASS getComputedStyle(document.getElementById('t39')).content is "APPLIED"
+PASS getComputedStyle(document.getElementById('t40')).content is "APPLIED"
+PASS getComputedStyle(document.getElementById('t41')).content is "APPLIED"
+PASS getComputedStyle(document.getElementById('t42')).content is "APPLIED"
+PASS getComputedStyle(document.getElementById('t43')).content is "UNTOUCHED"
+PASS getComputedStyle(document.getElementById('t44')).content is "UNTOUCHED"
 PASS successfullyParsed is true
 
 TEST COMPLETE

diff --git a/LayoutTests/css3/supports.html b/LayoutTests/css3/supports.html
index 44347dae598fca8ec0bb513ac864b7f9543ef20d..fb1638fabc671ebd5379c45ef3ddc8ccc8da5e3c 100644 (file)
--- a/LayoutTests/css3/supports.html
+++ b/LayoutTests/css3/supports.html
@@ -172,6 +172,57 @@
             #t36 { content: "FAIL" }
         }
     }
+
+    /* Invalid syntax error recovery */
+
+    @supports (display: none);
+    @supports (display: none) and ( (display: none) ) {
+        #t37 { content: "APPLIED" }
+    }
+
+    @supports (display: none)) ;
+    @supports (display: none) {
+        #t38 { content: "APPLIED" }
+    }
+
+    @supports;
+    @supports (display: none) {
+        #t39 { content: "APPLIED" }
+    }
+
+    @supports ;
+    @supports (display: none) {
+        #t40 { content: "APPLIED" }
+    }
+
+    @supports (display: none)) {
+        #t41 { content: "FAIL" }
+    }
+    @supports (display: none) {
+        #t41 { content: "APPLIED" }
+    }
+
+    @supports (display: )) {
+        #t42 { content: "FAIL" }
+    }
+    @supports (display: none) {
+        #t42 { content: "APPLIED" }
+    }
+
+    @supports ((display: none) and {
+        #t43 { content: "FAIL" }
+    }
+    @supports (display: none) {
+        #t43 { content: "FAIL" }
+    }
+    );
+
+    @supports ((display: none) and ;
+    @supports (display: none) {
+        #t44 { content: "FAIL" }
+    }
+    );
+
 </style>
 </head>
 <body>
@@ -179,8 +230,8 @@
 
 <script>
     description("Test the @supports rule.");
-    var numTests = 37;
-    var untouchedTests = [1, 3, 5, 8, 12, 13, 14, 18, 28, 29, 34, 36]; // Tests whose content shouldn't change from the UNTOUCHED default.
+    var numTests = 45;
+    var untouchedTests = [1, 3, 5, 8, 12, 13, 14, 18, 28, 29, 34, 36, 43, 44]; // Tests whose content shouldn't change from the UNTOUCHED default.
 
     var container = document.getElementById("test_container");
     for (var i=0; i < numTests; i++) {

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index ea9194e83a2f139134d02b972d55708c932c9b0d..e65d750c1df379b9b83f2a5786289c8297587326 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,24 @@
+2013-02-12  Pablo Flouret  <pablof@motorola.com>
+
+        Handle error recovery in @supports
+        https://bugs.webkit.org/show_bug.cgi?id=103934
+
+        Reviewed by Antti Koivisto.
+
+        Tests 021, 024, 031, and 033 in
+        http://hg.csswg.org/test/file/5f94e4b03ed9/contributors/opera/submitted/css3-conditional
+        fail because there's no explicit error recovery in @support's grammar.
+        Opera and Firefox pass the tests.
+
+        No new tests, modified css3/supports{,-cssom}.html
+
+        * css/CSSGrammar.y.in:
+        * css/CSSParser.cpp:
+        (WebCore::CSSParser::createSupportsRule):
+        (WebCore::CSSParser::markSupportsRuleHeaderEnd):
+        (WebCore::CSSParser::popSupportsRuleData):
+        * css/CSSParser.h:
+
 2013-02-12  Eric Carlson  <eric.carlson@apple.com>
 
         [Mac] guard against NULL languages array

diff --git a/Source/WebCore/css/CSSGrammar.y.in b/Source/WebCore/css/CSSGrammar.y.in
index 4ad333dc7ed13cb0f36d96c6cef53c44d7e15048..10544c3ba7178a1e54cbdbbfd5f2e15107751d03 100644 (file)
--- a/Source/WebCore/css/CSSGrammar.y.in
+++ b/Source/WebCore/css/CSSGrammar.y.in
@@ -249,6 +249,7 @@ static inline int cssyyerror(void*, const char*)
 %type <boolean> supports_conjunction
 %type <boolean> supports_disjunction
 %type <boolean> supports_declaration_condition
+%type <boolean> supports_error
 #endif
 
 %type <string> keyframe_name
@@ -692,8 +693,16 @@ supports:
     before_supports_rule SUPPORTS_SYM maybe_space supports_condition at_supports_rule_header_end '{' at_rule_body_start maybe_space block_rule_list save_block {
         $$ = parser->createSupportsRule($4, $9);
     }
+    | before_supports_rule SUPPORTS_SYM supports_error {
+        parser->popRuleData();
+        parser->popSupportsRuleData();
+    }
     ;
 
+supports_error:
+    error ';'
+    | error invalid_block
+
 before_supports_rule:
     /* empty */ {
         parser->markRuleHeaderStart(CSSRuleSourceData::SUPPORTS_RULE);
@@ -744,6 +753,7 @@ supports_condition_in_parens:
         $$ = $3;
     }
     | supports_declaration_condition
+    | '(' error ')'
     ;
 
 supports_declaration_condition:

diff --git a/Source/WebCore/css/CSSParser.cpp b/Source/WebCore/css/CSSParser.cpp
index 2098b1e3157f72011a72565c3bcfe24727022a2e..105f18c4dccc9c912b25ffba48ef6aaff75bb67d 100644 (file)
--- a/Source/WebCore/css/CSSParser.cpp
+++ b/Source/WebCore/css/CSSParser.cpp
@@ -10861,12 +10861,7 @@ StyleRuleBase* CSSParser::createSupportsRule(bool conditionIsSupported, RuleList
 {
     m_allowImportRules = m_allowNamespaceDeclarations = false;
 
-    ASSERT(!m_supportsRuleDataStack->isEmpty());
-    RefPtr<CSSRuleSourceData> data = m_supportsRuleDataStack->last();
-    m_supportsRuleDataStack->removeLast();
-    if (m_supportsRuleDataStack->isEmpty())
-        m_supportsRuleDataStack.clear();
-
+    RefPtr<CSSRuleSourceData> data = popSupportsRuleData();
     RefPtr<StyleRuleSupports> rule;
     String conditionText;
     unsigned conditionOffset = data->ruleHeaderRange.start + 9;
@@ -10903,13 +10898,22 @@ void CSSParser::markSupportsRuleHeaderStart()
 
 void CSSParser::markSupportsRuleHeaderEnd()
 {
-    ASSERT(!m_supportsRuleDataStack->isEmpty());
+    ASSERT(m_supportsRuleDataStack && !m_supportsRuleDataStack->isEmpty());
 
     if (is8BitSource())
         m_supportsRuleDataStack->last()->ruleHeaderRange.end = tokenStart<LChar>() - m_dataStart8.get();
     else
         m_supportsRuleDataStack->last()->ruleHeaderRange.end = tokenStart<UChar>() - m_dataStart16.get();
 }
+
+PassRefPtr<CSSRuleSourceData> CSSParser::popSupportsRuleData()
+{
+    ASSERT(m_supportsRuleDataStack && !m_supportsRuleDataStack->isEmpty());
+    RefPtr<CSSRuleSourceData> data = m_supportsRuleDataStack->last();
+    m_supportsRuleDataStack->removeLast();
+    return data.release();
+}
+
 #endif
 
 CSSParser::RuleList* CSSParser::createRuleList()

diff --git a/Source/WebCore/css/CSSParser.h b/Source/WebCore/css/CSSParser.h
index db9ea981556d134c601db764efcd69cf04e4a9ba..dcb5134c63f9c66db6bf6b22137c00accba0b2d9 100644 (file)
--- a/Source/WebCore/css/CSSParser.h
+++ b/Source/WebCore/css/CSSParser.h
@@ -300,6 +300,7 @@ public:
     StyleRuleBase* createSupportsRule(bool conditionIsSupported, RuleList*);
     void markSupportsRuleHeaderStart();
     void markSupportsRuleHeaderEnd();
+    PassRefPtr<CSSRuleSourceData> popSupportsRuleData();
 #endif
 #if ENABLE(SHADOW_DOM)
     StyleRuleBase* createHostRule(RuleList* rules);