Bug 511909. Allow @-rules to nest when parsing CSS. In particular, allow them inside @media and @-moz-document. r=dbaron
authorJet Villegas <jet@mozilla.com>
Wed, 14 Dec 2011 23:42:15 -0500
changeset 82614 ba1d8b3a53e493de189974d450fb5c543d7fb27d
parent 82613 a3f62505cd16c5f4633bb3a0388ba1ac33e6aa06
child 82615 ae42e4497ff2129bd73a33d4316637a0b55961a0
push id21687
push userbzbarsky@mozilla.com
push dateThu, 15 Dec 2011 04:44:47 +0000
treeherdermozilla-central@ae42e4497ff2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs511909
milestone11.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 511909. Allow @-rules to nest when parsing CSS. In particular, allow them inside @media and @-moz-document. r=dbaron
dom/locales/en-US/chrome/layout/css.properties
layout/style/nsCSSParser.cpp
layout/style/test/Makefile.in
layout/style/test/test_bug511909.html
--- a/dom/locales/en-US/chrome/layout/css.properties
+++ b/dom/locales/en-US/chrome/layout/css.properties
@@ -54,16 +54,17 @@ PEGatherMediaNotIdent=Expected identifie
 PEImportNotURI=Expected URI in @import rule but found '%1$S'.
 PEImportBadURI=Invalid URI in @import rule: '%1$S'.
 PEImportUnexpected=Found unexpected '%1$S' within @import.
 PEGroupRuleEOF=end of @media or @-moz-document rule
 PEGroupRuleNestedAtRule=%1$S rule not allowed within @media or @-moz-document rule.
 PEMozDocRuleBadFunc=Expected url(), url-prefix(), or domain() in @-moz-document rule but found '%1$S'.
 PEMozDocRuleNotURI=Expected URI in @-moz-document rule but found '%1$S'.
 PEMozDocRuleNotString=Expected string in @-moz-document rule regexp() function but found '%1$S'.
+PEMozDocRuleEOF=next URI in @-moz-document rule
 PEAtNSPrefixEOF=namespace prefix in @namespace rule
 PEAtNSURIEOF=namespace URI in @namespace rule
 PEAtNSUnexpected=Unexpected token within @namespace: '%1$S'.
 PEKeyframeNameEOF=name of @keyframes rule.
 PEKeyframeBadName=Expected identifier for name of @keyframes rule.
 PEKeyframeBrace=Expected opening { of @keyframes rule.
 PESkipDeclBraceEOF=closing } of declaration block
 PESkipRSBraceEOF=closing } of invalid rule set
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -330,43 +330,44 @@ protected:
   void SkipRuleSet(bool aInsideBraces);
   bool SkipAtRule(bool aInsideBlock);
   bool SkipDeclaration(bool aCheckForBraces);
 
   void PushGroup(css::GroupRule* aRule);
   void PopGroup();
 
   bool ParseRuleSet(RuleAppendFunc aAppendFunc, void* aProcessData,
-                      bool aInsideBraces = false);
-  bool ParseAtRule(RuleAppendFunc aAppendFunc, void* aProcessData);
+                    bool aInsideBraces = false);
+  bool ParseAtRule(RuleAppendFunc aAppendFunc, void* aProcessData,
+                   bool aInAtRule);
   bool ParseCharsetRule(RuleAppendFunc aAppendFunc, void* aProcessData);
   bool ParseImportRule(RuleAppendFunc aAppendFunc, void* aProcessData);
   bool ParseURLOrString(nsString& aURL);
   bool GatherMedia(nsMediaList* aMedia,
-                     bool aInAtRule);
+                   bool aInAtRule);
   bool ParseMediaQuery(bool aInAtRule, nsMediaQuery **aQuery,
-                         bool *aHitStop);
+                       bool *aHitStop);
   bool ParseMediaQueryExpression(nsMediaQuery* aQuery);
   void ProcessImport(const nsString& aURLSpec,
                      nsMediaList* aMedia,
                      RuleAppendFunc aAppendFunc,
                      void* aProcessData);
   bool ParseGroupRule(css::GroupRule* aRule, RuleAppendFunc aAppendFunc,
-                        void* aProcessData);
+                      void* aProcessData);
   bool ParseMediaRule(RuleAppendFunc aAppendFunc, void* aProcessData);
   bool ParseMozDocumentRule(RuleAppendFunc aAppendFunc, void* aProcessData);
   bool ParseNameSpaceRule(RuleAppendFunc aAppendFunc, void* aProcessData);
   void ProcessNameSpace(const nsString& aPrefix,
                         const nsString& aURLSpec, RuleAppendFunc aAppendFunc,
                         void* aProcessData);
 
   bool ParseFontFaceRule(RuleAppendFunc aAppendFunc, void* aProcessData);
   bool ParseFontDescriptor(nsCSSFontFaceRule* aRule);
   bool ParseFontDescriptorValue(nsCSSFontDesc aDescID,
-                                  nsCSSValue& aValue);
+                                nsCSSValue& aValue);
 
   bool ParsePageRule(RuleAppendFunc aAppendFunc, void* aProcessData);
   bool ParseKeyframesRule(RuleAppendFunc aAppendFunc, void* aProcessData);
   already_AddRefed<nsCSSKeyframeRule> ParseKeyframeRule();
   bool ParseKeyframeSelectorList(InfallibleTArray<float>& aSelectorList);
 
   enum nsSelectorParsingStatus {
     // we have parsed a selector and we saw a token that cannot be
@@ -914,17 +915,17 @@ CSSParserImpl::ParseSheet(const nsAStrin
     if (!GetToken(true)) {
       OUTPUT_ERROR();
       break;
     }
     if (eCSSToken_HTMLComment == tk->mType) {
       continue; // legal here only
     }
     if (eCSSToken_AtKeyword == tk->mType) {
-      ParseAtRule(AppendRuleToSheet, this);
+      ParseAtRule(AppendRuleToSheet, this, false);
       continue;
     }
     UngetToken();
     if (ParseRuleSet(AppendRuleToSheet, this)) {
       mSection = eCSSSection_General;
     }
   }
   ReleaseScanner();
@@ -1045,17 +1046,17 @@ CSSParserImpl::ParseRule(const nsAString
   mSection = eCSSSection_Charset; // callers are responsible for rejecting invalid rules.
 
   nsCSSToken* tk = &mToken;
   // Get first non-whitespace token
   if (!GetToken(true)) {
     REPORT_UNEXPECTED(PEParseRuleWSOnly);
     OUTPUT_ERROR();
   } else if (eCSSToken_AtKeyword == tk->mType) {
-    ParseAtRule(AppendRuleToArray, &aResult);
+    ParseAtRule(AppendRuleToArray, &aResult, false);
   }
   else {
     UngetToken();
     ParseRuleSet(AppendRuleToArray, &aResult);
   }
   OUTPUT_ERROR();
   ReleaseScanner();
   // XXX check for low-level errors
@@ -1503,24 +1504,20 @@ CSSParserImpl::SkipAtRule(bool aInsideBl
       SkipUntil(')');
     }
   }
   return true;
 }
 
 bool
 CSSParserImpl::ParseAtRule(RuleAppendFunc aAppendFunc,
-                           void* aData)
-{
-  // If we ever allow nested at-rules, we need to be very careful about
-  // the error handling rules in the CSS spec.  In particular, we need
-  // to pass in to ParseAtRule whether we're inside a block, we need to
-  // ensure that all the individual at-rule parsing functions terminate
-  // immediately when they hit a '}', and then we need to pass whether
-  // we're inside a block to SkipAtRule below.
+                           void* aData,
+                           bool aInAtRule)
+{
+
   nsCSSSection newSection;
   bool (CSSParserImpl::*parseFunc)(RuleAppendFunc, void*);
 
   if ((mSection <= eCSSSection_Charset) &&
       (mToken.mIdent.LowerCaseEqualsLiteral("charset"))) {
     parseFunc = &CSSParserImpl::ParseCharsetRule;
     newSection = eCSSSection_Import;  // only one charset allowed
 
@@ -1555,26 +1552,37 @@ CSSParserImpl::ParseAtRule(RuleAppendFun
     newSection = eCSSSection_General;
 
   } else {
     if (!NonMozillaVendorIdentifier(mToken.mIdent)) {
       REPORT_UNEXPECTED_TOKEN(PEUnknownAtRule);
       OUTPUT_ERROR();
     }
     // Skip over unsupported at rule, don't advance section
-    return SkipAtRule(false);
-  }
-
-  if (!(this->*parseFunc)(aAppendFunc, aData)) {
+    return SkipAtRule(aInAtRule);
+  }
+
+  // Inside of @-rules, only the rules that can occur anywhere
+  // are allowed.
+  bool unnestable = aInAtRule && newSection != eCSSSection_General;
+  if (unnestable) {
+    REPORT_UNEXPECTED_TOKEN(PEGroupRuleNestedAtRule);
+  }
+  
+  if (unnestable || !(this->*parseFunc)(aAppendFunc, aData)) {
     // Skip over invalid at rule, don't advance section
     OUTPUT_ERROR();
-    return SkipAtRule(false);
-  }
-
-  mSection = newSection;
+    return SkipAtRule(aInAtRule);
+  }
+
+  // Nested @-rules don't affect the top-level rule chain requirement
+  if (!aInAtRule) {
+    mSection = newSection;
+  }
+  
   return true;
 }
 
 bool
 CSSParserImpl::ParseCharsetRule(RuleAppendFunc aAppendFunc,
                                 void* aData)
 {
   if (!GetToken(true)) {
@@ -1632,17 +1640,17 @@ CSSParserImpl::ParseMediaQuery(bool aInA
       return true;
 
     // unexpected termination by EOF
     REPORT_UNEXPECTED_EOF(PEGatherMediaEOF);
     return true;
   }
 
   if (eCSSToken_Symbol == mToken.mType && aInAtRule &&
-      (mToken.mSymbol == ';' || mToken.mSymbol == '{')) {
+      (mToken.mSymbol == ';' || mToken.mSymbol == '{' || mToken.mSymbol == '}' )) {
     *aHitStop = true;
     UngetToken();
     return true;
   }
   UngetToken();
 
   nsMediaQuery* query = new nsMediaQuery;
   *aQuery = query;
@@ -1696,17 +1704,17 @@ CSSParserImpl::ParseMediaQuery(bool aInA
         break;
 
       // unexpected termination by EOF
       REPORT_UNEXPECTED_EOF(PEGatherMediaEOF);
       break;
     }
 
     if (eCSSToken_Symbol == mToken.mType && aInAtRule &&
-        (mToken.mSymbol == ';' || mToken.mSymbol == '{')) {
+        (mToken.mSymbol == ';' || mToken.mSymbol == '{' || mToken.mSymbol == '}')) {
       *aHitStop = true;
       UngetToken();
       break;
     }
     if (eCSSToken_Symbol == mToken.mType && mToken.mSymbol == ',') {
       // Done with the expressions for this query
       break;
     }
@@ -1737,24 +1745,24 @@ CSSParserImpl::GatherMedia(nsMediaList* 
                          &hitStop)) {
       NS_ASSERTION(!hitStop, "should return true when hit stop");
       OUTPUT_ERROR();
       if (query) {
         query->SetHadUnknownExpression();
       }
       if (aInAtRule) {
         const PRUnichar stopChars[] =
-          { PRUnichar(','), PRUnichar('{'), PRUnichar(';'), PRUnichar(0) };
+          { PRUnichar(','), PRUnichar('{'), PRUnichar(';'), PRUnichar('}'), PRUnichar(0) };
         SkipUntilOneOf(stopChars);
       } else {
         SkipUntil(',');
       }
       // Rely on SkipUntilOneOf leaving mToken around as the last token read.
       if (mToken.mType == eCSSToken_Symbol && aInAtRule &&
-          (mToken.mSymbol == '{' || mToken.mSymbol == ';')) {
+          (mToken.mSymbol == '{' || mToken.mSymbol == ';'  || mToken.mSymbol == '}')) {
         UngetToken();
         hitStop = true;
       }
     }
     if (query) {
       aMedia->AppendQuery(query);
     }
     if (hitStop) {
@@ -1988,19 +1996,18 @@ CSSParserImpl::ParseGroupRule(css::Group
       REPORT_UNEXPECTED_EOF(PEGroupRuleEOF);
       break;
     }
     if (mToken.IsSymbol('}')) { // done!
       UngetToken();
       break;
     }
     if (eCSSToken_AtKeyword == mToken.mType) {
-      REPORT_UNEXPECTED_TOKEN(PEGroupRuleNestedAtRule);
-      OUTPUT_ERROR();
-      SkipAtRule(true); // group rules cannot contain @rules
+      // Parse for nested rules
+      ParseAtRule(aAppendFunc, aData, true);
       continue;
     }
     UngetToken();
     ParseRuleSet(AppendRuleToSheet, this, true);
   }
   PopGroup();
 
   if (!ExpectSymbol('}', true)) {
@@ -2035,23 +2042,29 @@ CSSParserImpl::ParseMediaRule(RuleAppend
 // of a medium it has a nonempty list of items where each item is either
 // url(), url-prefix(), or domain().
 bool
 CSSParserImpl::ParseMozDocumentRule(RuleAppendFunc aAppendFunc, void* aData)
 {
   css::DocumentRule::URL *urls = nsnull;
   css::DocumentRule::URL **next = &urls;
   do {
-    if (!GetToken(true) ||
-        !(eCSSToken_URL == mToken.mType ||
+    if (!GetToken(true)) {
+      REPORT_UNEXPECTED_EOF(PEMozDocRuleEOF);
+      delete urls;
+      return false;
+    }
+        
+    if (!(eCSSToken_URL == mToken.mType ||
           (eCSSToken_Function == mToken.mType &&
            (mToken.mIdent.LowerCaseEqualsLiteral("url-prefix") ||
             mToken.mIdent.LowerCaseEqualsLiteral("domain") ||
             mToken.mIdent.LowerCaseEqualsLiteral("regexp"))))) {
       REPORT_UNEXPECTED_TOKEN(PEMozDocRuleBadFunc);
+      UngetToken();
       delete urls;
       return false;
     }
     css::DocumentRule::URL *cur = *next = new css::DocumentRule::URL;
     next = &cur->next;
     if (mToken.mType == eCSSToken_URL) {
       cur->func = css::DocumentRule::eURL;
       CopyUTF16toUTF8(mToken.mIdent, cur->url);
--- a/layout/style/test/Makefile.in
+++ b/layout/style/test/Makefile.in
@@ -114,16 +114,17 @@ GARBAGE += css_properties.js
 		test_bug405818.html \
 		test_bug412901.html \
 		test_bug437915.html \
 		test_bug450191.html \
 		test_bug453896_deck.html \
 		test_bug470769.html \
 		test_bug499655.html \
 		test_bug499655.xhtml \
+		test_bug511909.html \
 		test_bug517224.html \
 		test_bug524175.html \
 		test_bug534804.html \
 		test_bug573255.html \
 		test_bug580685.html \
 		test_bug635286.html \
 		test_bug652486.html \
 		test_bug657143.html \
new file mode 100644
--- /dev/null
+++ b/layout/style/test/test_bug511909.html
@@ -0,0 +1,205 @@
+<html><!--
+   https://bugzilla.mozilla.org/show_bug.cgi?id=511909
+   --><head>
+<meta http-equiv="content-type" content="text/html; charset=UTF-8">
+<title>@media and @-moz-document testcases</title>
+  
+<script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css">
+
+<style type="text/css">
+a {
+    font-weight: bold; 
+}
+  #pink {
+    color: pink;
+  }
+  
+  #green {
+    color: green;
+  }
+  
+  #blue {
+    color: blue;
+  }
+
+
+pre {
+   border: 1px solid black;
+}
+</style>
+  
+<style type="text/css">
+@-moz-document regexp(".*test_bug511909.*"){
+    #d {
+      color: pink;
+    }
+}
+
+</style>
+  
+<style type="text/css">
+    
+@media screen {
+   #m {
+      color: green;
+   }
+}
+
+</style>
+  
+<style type="text/css">
+  
+@-moz-document regexp(".*test_bug511909.*"){
+  @media screen {
+      #dm {
+         color: blue;
+      }
+   }
+}
+
+</style>
+  
+<!-- should parse -->
+<style type="text/css">
+
+@media print {
+  @-moz-document regexp("not_this_url"),}
+    #mx {
+        color: pink;
+    }
+  }
+}
+
+</style>
+  
+<!-- should parse -->
+<style type="text/css">
+    
+@-moz-document regexp("not_this_url"){
+  @media print ,}
+    #mxx {
+      color: blue;
+    }
+  }
+}
+
+</style>
+  
+<style type="text/css">  
+  
+@media screen {
+  @-moz-document regexp(".*test_bug511909.*"){
+      #md {
+         color: green;
+      }
+   }
+}
+
+</style>
+  
+<style type="text/css">
+    
+@media screen {
+  @-moz-document regexp(".*test_bug511909.*"){
+      @media screen { 
+        @-moz-document regexp(".*test_bug511909.*"){
+          @media screen {
+            #me {
+             color: blue; 
+            }
+          }
+        }
+     }
+   }
+}
+  
+</style>
+</head>
+<body>
+  <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=511909">Mozilla Bug 511909</a>
+  <p id="display"></p>
+  <div id="content" style="display: none">
+    
+  </div>
+  
+  <script class="testbody" type="text/javascript">
+
+    SimpleTest.waitForExplicitFinish();
+    
+    addLoadEvent(function() {
+    var pink = getComputedStyle(document.getElementById("pink"), "");
+    var green = getComputedStyle(document.getElementById("green"), "");
+    var blue = getComputedStyle(document.getElementById("blue"), "");
+                 
+    var cs1 = getComputedStyle(document.getElementById("d"), "");
+    var cs2 = getComputedStyle(document.getElementById("m"), "");
+    var cs3 = getComputedStyle(document.getElementById("dm"), "");
+    var cs4 = getComputedStyle(document.getElementById("md"), "");
+    var cs5 = getComputedStyle(document.getElementById("mx"), "");                 
+    var cs6 = getComputedStyle(document.getElementById("mxx"), "");                  
+    var cs7 = getComputedStyle(document.getElementById("me"), "");  
+                 
+    is(cs1.color, pink.color, "@-moz-document applies");
+    is(cs2.color, green.color, "@media applies");
+    is(cs3.color, blue.color, "@media nested in @-moz-document applies");
+    is(cs4.color, green.color, "@-moz-document nested in @media applies");
+    is(cs5.color, pink.color, "broken @media nested in @-moz-document correctly handled");      
+    is(cs6.color, blue.color, "broken @-moz-document nested in @media correctly handled");   
+    is(cs7.color, blue.color, "@media nested in @-moz-document nested in @media applies");
+    SimpleTest.finish();
+    });
+  </script>
+<div>
+<pre>default style
+</pre>
+<a id="pink">This line should be pink</a><br>
+  
+<a id="green">This line should be green</a><br>
+  
+<a id="blue">This line should be blue</a><br>
+  
+<pre>@-moz-document {...}
+</pre>
+<a id="d">This line should be pink</a><br>
+<pre>@media screen {...}
+</pre>
+<a id="m">This line should be green</a><br>
+<pre>@-moz-document {
+   @media screen {...}
+}
+</pre>
+<a id="dm">This line should be blue</a><br>
+<pre>@media print {
+  @-moz-document regexp("not_this_url"),}
+    #mx {
+        color: pink;
+    }
+  }
+}
+</pre>
+<a id="mx">This line should be pink</a><br></div>
+<pre>@-moz-document regexp("not_this_url"){
+  @media print ,}
+    #mxx {
+      color: blue;
+    }
+  }
+}
+</pre>
+<a id="mxx">This line should be blue</a><br>
+<pre>@media screen {
+  @-moz-documen {...}
+}
+</pre>
+<a id="md">This line should be green</a><br>
+<pre>@media screen {
+  @-moz-document {
+    @media screen {...}
+  }
+}
+</pre>
+<a id="me">This line should be blue</a><br>
+
+
+</body></html>