Bug 569719 part 10: refactor parsing of !important and the end of a property declaration. r=dbaron
authorZack Weinberg <zweinberg@mozilla.com>
Fri, 23 Jul 2010 11:00:37 -0700
changeset 48858 3649275f5d64f8caa97b93f60f9cad9d5c593c7c
parent 48857 0d351a1bae2bb4731d1e4845ced2f12458e913f3
child 48859 794e9603cbbf7c8bb3264d1266d0365830f08e37
push idunknown
push userunknown
push dateunknown
reviewersdbaron
bugs569719
milestone2.0b4pre
Bug 569719 part 10: refactor parsing of !important and the end of a property declaration. r=dbaron
layout/style/nsCSSParser.cpp
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -432,16 +432,23 @@ protected:
   // the source and destination locations point to the right kind of objects
   // for the property id.  This can only be used for non-shorthand properties.
   void CopyValue(void *aSource, void *aDest, nsCSSProperty aPropID,
                  PRBool* aChanged);
   PRBool ParseProperty(nsCSSProperty aPropID);
   PRBool ParseSingleValueProperty(nsCSSValue& aValue,
                                   nsCSSProperty aPropID);
 
+  enum PriorityParsingStatus {
+    ePriority_None,
+    ePriority_Important,
+    ePriority_Error
+  };
+  PriorityParsingStatus ParsePriority();
+
 #ifdef MOZ_XUL
   PRBool ParseTreePseudoElement(nsPseudoClassList **aPseudoElementArgs);
 #endif
 
   void InitBoxPropsAsPhysical(const nsCSSProperty *aSourceProperties);
 
   // Property specific parsing routines
   PRBool ParseAzimuth(nsCSSValue& aValue);
@@ -1412,16 +1419,44 @@ CSSParserImpl::ExpectEndProperty()
   if (CheckEndProperty())
     return PR_TRUE;
 
   // If we're here, we read something incorrect, so we should report it.
   REPORT_UNEXPECTED_TOKEN(PEExpectEndValue);
   return PR_FALSE;
 }
 
+// Parses the priority suffix on a property, which at present may be
+// either '!important' or nothing.
+CSSParserImpl::PriorityParsingStatus
+CSSParserImpl::ParsePriority()
+{
+  if (!GetToken(PR_TRUE)) {
+    return ePriority_None; // properties may end with EOF
+  }
+  if (!mToken.IsSymbol('!')) {
+    UngetToken();
+    return ePriority_None; // dunno what it is, but it's not a priority
+  }
+
+  if (!GetToken(PR_TRUE)) {
+    // EOF is not ok after !
+    REPORT_UNEXPECTED_EOF(PEImportantEOF);
+    return ePriority_Error;
+  }
+
+  if (mToken.mType != eCSSToken_Ident ||
+      !mToken.mIdent.LowerCaseEqualsLiteral("important")) {
+    REPORT_UNEXPECTED_TOKEN(PEExpectedImportant);
+    UngetToken();
+    return ePriority_Error;
+  }
+
+  return ePriority_Important;
+}
 
 nsSubstring*
 CSSParserImpl::NextIdent()
 {
   // XXX Error reporting?
   if (!GetToken(PR_TRUE)) {
     return nsnull;
   }
@@ -3994,80 +4029,51 @@ CSSParserImpl::ParseDeclaration(css::Dec
     REPORT_UNEXPECTED_P(PEValueParsingError, params);
     REPORT_UNEXPECTED(PEDeclDropped);
     OUTPUT_ERROR();
     ClearTempData(propID);
     return PR_FALSE;
   }
   CLEAR_ERROR();
 
-  // See if the declaration is followed by a "!important" declaration
-  PRBool isImportant = PR_FALSE;
-  if (!GetToken(PR_TRUE)) {
-    // EOF is a perfectly good way to end a declaration and declaration block
-    TransferTempData(aDeclaration, propID, isImportant, PR_FALSE,
-                     aMustCallValueAppended, aChanged);
-    return PR_TRUE;
-  }
-
-  if (eCSSToken_Symbol == tk->mType && '!' == tk->mSymbol) {
-    // Look for important ident
+  // Look for "!important".
+  PriorityParsingStatus status = ParsePriority();
+
+  // Look for a semicolon or close brace.
+  if (status != ePriority_Error) {
     if (!GetToken(PR_TRUE)) {
-      // Premature eof is not ok
-      REPORT_UNEXPECTED_EOF(PEImportantEOF);
-      ClearTempData(propID);
-      return PR_FALSE;
-    }
-    if ((eCSSToken_Ident != tk->mType) ||
-        !tk->mIdent.LowerCaseEqualsLiteral("important")) {
-      REPORT_UNEXPECTED_TOKEN(PEExpectedImportant);
-      OUTPUT_ERROR();
+      // EOF is always ok
+    } else if (mToken.IsSymbol(';')) {
+      // semicolon is always ok
+    } else if (mToken.IsSymbol('}')) {
+      // brace is ok if aCheckForBraces, but don't eat it
       UngetToken();
-      ClearTempData(propID);
-      return PR_FALSE;
-    }
-    isImportant = PR_TRUE;
-  }
-  else {
-    // Not a !important declaration
-    UngetToken();
-  }
-
-  // Make sure valid property declaration is terminated with either a
-  // semicolon, EOF or a right-curly-brace (this last only when
-  // aCheckForBraces is true).
-  if (!GetToken(PR_TRUE)) {
-    // EOF is a perfectly good way to end a declaration and declaration block
-    TransferTempData(aDeclaration, propID, isImportant, PR_FALSE,
-                     aMustCallValueAppended, aChanged);
-    return PR_TRUE;
-  }
-  if (eCSSToken_Symbol == tk->mType) {
-    if (';' == tk->mSymbol) {
-      TransferTempData(aDeclaration, propID, isImportant, PR_FALSE,
-                       aMustCallValueAppended, aChanged);
-      return PR_TRUE;
-    }
-    if (aCheckForBraces && '}' == tk->mSymbol) {
-      // Unget the '}' so we'll be able to tell that this is the end
-      // of the declaration block when we unwind from here.
-      UngetToken();
-      TransferTempData(aDeclaration, propID, isImportant, PR_FALSE,
-                       aMustCallValueAppended, aChanged);
-      return PR_TRUE;
-    }
-  }
-  if (aCheckForBraces)
-    REPORT_UNEXPECTED_TOKEN(PEBadDeclOrRuleEnd2);
-  else
-    REPORT_UNEXPECTED_TOKEN(PEBadDeclEnd);
-  REPORT_UNEXPECTED(PEDeclDropped);
-  OUTPUT_ERROR();
-  ClearTempData(propID);
-  return PR_FALSE;
+      if (!aCheckForBraces) {
+        status = ePriority_Error;
+      }
+    } else {
+      status = ePriority_Error;
+    }
+  }
+
+  if (status == ePriority_Error) {
+    if (aCheckForBraces) {
+      REPORT_UNEXPECTED_TOKEN(PEBadDeclOrRuleEnd2);
+    } else {
+      REPORT_UNEXPECTED(PEBadDeclEnd);
+    }
+    REPORT_UNEXPECTED(PEDeclDropped);
+    OUTPUT_ERROR();
+    ClearTempData(propID);
+    return PR_FALSE;
+  }
+
+  TransferTempData(aDeclaration, propID, status == ePriority_Important,
+                   PR_FALSE, aMustCallValueAppended, aChanged);
+  return PR_TRUE;
 }
 
 void
 CSSParserImpl::ClearTempData(nsCSSProperty aPropID)
 {
   if (nsCSSProps::IsShorthand(aPropID)) {
     CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES(p, aPropID) {
       mTempData.ClearProperty(*p);