Bug 841983: Require whitespace around 'not', 'and', and 'or' keywords in @supports rules. r=heycam
authorL. David Baron <dbaron@dbaron.org>
Sat, 16 Feb 2013 21:29:38 -0800
changeset 122165 20abb67042df3176230fec417c84dd0a0fb636fc
parent 122164 a235e0d9dd7042ef059c92af53d3589d9c375662
child 122166 e4831a8b042cdcca130dc85f0e6f8ebb23eaa47b
push id23135
push userdbaron@mozilla.com
push dateSun, 17 Feb 2013 05:29:49 +0000
treeherdermozilla-inbound@20abb67042df [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs841983
milestone21.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 841983: Require whitespace around 'not', 'and', and 'or' keywords in @supports rules. r=heycam Matches spec change in https://dvcs.w3.org/hg/csswg/rev/34b185ae3bac .
dom/locales/en-US/chrome/layout/css.properties
layout/style/nsCSSParser.cpp
layout/style/test/test_supports_rules.html
--- a/dom/locales/en-US/chrome/layout/css.properties
+++ b/dom/locales/en-US/chrome/layout/css.properties
@@ -114,13 +114,14 @@ PEMQNoMinMaxWithoutValue=Media features 
 PEMQExpectedFeatureValue=Found invalid value for media feature.
 PEBadFontBlockStart=Expected '{' to begin @font-face rule but found '%1$S'.
 PEBadFontBlockEnd=Expected '}' to end @font-face rule but found '%1$S'.
 PEAnonBoxNotAlone=Did not expect anonymous box.
 PEBadDirValue=Expected 'ltr' or 'rtl' in direction selector but found '%1$S'.
 PESupportsConditionStartEOF2='not', '(', or function
 PESupportsConditionInParensEOF=')'
 PESupportsConditionNotEOF='not'
+PESupportsWhitespaceRequired=Expected whitespace after 'not', 'and', or 'or'.
 PESupportsConditionExpectedOpenParenOrFunction=Expected '(' or function while parsing supports condition but found '%1$S'.
 PESupportsConditionExpectedCloseParen=Expected ')' while parsing supports condition but found '%1$S'.
 PESupportsConditionExpectedStart2=Expected 'not', '(', or function while parsing supports condition but found '%1$S'.
 PESupportsConditionExpectedNot=Expected 'not' while parsing supports condition but found '%1$S'.
 PESupportsGroupRuleStart=Expected '{' to begin @supports rule but found '%1$S'.
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -2532,32 +2532,37 @@ CSSParserImpl::ParseSupportsCondition(bo
     return ParseSupportsConditionNegation(aConditionMet);
   }
 
   REPORT_UNEXPECTED_TOKEN(PESupportsConditionExpectedStart);
   return false;
 }
 
 // supports_condition_negation
-//   : 'not' S* supports_condition_in_parens
+//   : 'not' S+ supports_condition_in_parens
 //   ;
 bool
 CSSParserImpl::ParseSupportsConditionNegation(bool& aConditionMet)
 {
   if (!GetToken(true)) {
     REPORT_UNEXPECTED_EOF(PESupportsConditionNotEOF);
     return false;
   }
 
   if (mToken.mType != eCSSToken_Ident ||
       !mToken.mIdent.LowerCaseEqualsLiteral("not")) {
     REPORT_UNEXPECTED_TOKEN(PESupportsConditionExpectedNot);
     return false;
   }
 
+  if (!RequireWhitespace()) {
+    REPORT_UNEXPECTED(PESupportsWhitespaceRequired);
+    return false;
+  }
+
   if (ParseSupportsConditionInParens(aConditionMet)) {
     aConditionMet = !aConditionMet;
     return true;
   }
 
   return false;
 }
 
@@ -2660,24 +2665,24 @@ CSSParserImpl::ParseSupportsConditionInP
   }
 
   UngetToken();
   return ParseSupportsConditionInParens(aConditionMet) &&
          ParseSupportsConditionTerms(aConditionMet);
 }
 
 // supports_condition_terms
-//   : 'and' S* supports_condition_terms_after_operator('and')
-//   | 'or' S* supports_condition_terms_after_operator('or')
+//   : S+ 'and' supports_condition_terms_after_operator('and')
+//   | S+ 'or' supports_condition_terms_after_operator('or')
 //   |
 //   ;
 bool
 CSSParserImpl::ParseSupportsConditionTerms(bool& aConditionMet)
 {
-  if (!GetToken(true)) {
+  if (!RequireWhitespace() || !GetToken(false)) {
     return true;
   }
 
   if (mToken.mType != eCSSToken_Ident) {
     UngetToken();
     return true;
   }
 
@@ -2689,23 +2694,28 @@ CSSParserImpl::ParseSupportsConditionTer
     return ParseSupportsConditionTermsAfterOperator(aConditionMet, eOr);
   }
 
   UngetToken();
   return true;
 }
 
 // supports_condition_terms_after_operator(operator)
-//   : supports_condition_in_parens ( <operator> supports_condition_in_parens )*
+//   : S+ supports_condition_in_parens ( <operator> supports_condition_in_parens )*
 //   ;
 bool
 CSSParserImpl::ParseSupportsConditionTermsAfterOperator(
                          bool& aConditionMet,
                          CSSParserImpl::SupportsConditionTermOperator aOperator)
 {
+  if (!RequireWhitespace()) {
+    REPORT_UNEXPECTED(PESupportsWhitespaceRequired);
+    return false;
+  }
+
   const char* token = aOperator == eAnd ? "and" : "or";
   for (;;) {
     bool termConditionMet = false;
     if (!ParseSupportsConditionInParens(termConditionMet)) {
       return false;
     }
     aConditionMet = aOperator == eAnd ? aConditionMet && termConditionMet :
                                         aConditionMet || termConditionMet;
--- a/layout/style/test/test_supports_rules.html
+++ b/layout/style/test/test_supports_rules.html
@@ -7,20 +7,18 @@ https://bugzilla.mozilla.org/show_bug.cg
   <title>Test for Bug 649740</title>
   <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
   <style id="style">
   </style>
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=649740">Mozilla Bug 649740</a>
-<p id="display"></p>
-<div id="content" style="display: none">
-
-</div>
+<p id="display1"></p>
+<p id="display2"></p>
 <pre id="test">
 <script type="application/javascript">
 
 /** Test for Bug 649740 **/
 
 function condition(s) {
   return s.replace(/^@supports\s*/, '').replace(/ \s*{\s*}\s*$/, '');
 }
@@ -39,16 +37,51 @@ function runTest() {
   var sheet = style.sheet;
 
   is(condition(sheet.cssRules[0].cssText), "(color: green)");
   is(condition(sheet.cssRules[1].cssText), "(color: green)");
   is(condition(sheet.cssRules[2].cssText), "((color: green))");
   is(condition(sheet.cssRules[3].cssText), "(color: green) and (color: blue)");
   is(condition(sheet.cssRules[4].cssText), "( Font:  20px serif ! Important)");
 
+  var cs1 = getComputedStyle(document.getElementById("display1"), "");
+  var cs2 = getComputedStyle(document.getElementById("display2"), "");
+  function check_balanced_condition(condition, expected_match) {
+    style.textContent = "#display1, #display2 { text-decoration: overline }\n" +
+                        "@supports " + condition + "{\n" +
+                        "  #display1 { text-decoration: line-through }\n" +
+                        "}\n" +
+                        "#display2 { text-decoration: underline }\n";
+    is(cs1.textDecoration,
+       expected_match ? "line-through" : "overline",
+       "@supports condition \"" + condition + "\" should " +
+       (expected_match ? "" : "NOT ") + "match");
+    is(cs2.textDecoration, "underline",
+       "@supports condition \"" + condition + "\" should be balanced");
+  }
+
+  check_balanced_condition("not (color: green)", false);
+  check_balanced_condition("not (colour: green)", true);
+  check_balanced_condition("not(color: green)", false);
+  check_balanced_condition("not(colour: green)", false);
+  check_balanced_condition("not/* */(color: green)", false);
+  check_balanced_condition("not/* */(colour: green)", false);
+  check_balanced_condition("not /* */ (color: green)", false);
+  check_balanced_condition("not /* */ (colour: green)", true);
+  check_balanced_condition("(color: green) and (color: blue)", true);
+  check_balanced_condition("(color: green) /* */ /* */ and /* */ /* */ (color: blue)", true);
+  check_balanced_condition("(color: green) and(color: blue)", false);
+  check_balanced_condition("(color: green) and/* */(color: blue)", false);
+  check_balanced_condition("(color: green)and (color: blue)", false);
+  check_balanced_condition("(color: green) or (color: blue)", true);
+  check_balanced_condition("(color: green) /* */ /* */ or /* */ /* */ (color: blue)", true);
+  check_balanced_condition("(color: green) or(color: blue)", false);
+  check_balanced_condition("(color: green) or/* */(color: blue)", false);
+  check_balanced_condition("(color: green)or (color: blue)", false);
+
   SimpleTest.finish();
 }
 
 SimpleTest.waitForExplicitFinish();
 SpecialPowers.pushPrefEnv({ "set": [["layout.css.supports-rule.enabled", true]] }, runTest);
 </script>
 </pre>
 </body>