Bug 1273706 - Part 17: Add an aAllowVariableReferences parameter to ParseVariableDeclaration. r?dbaron draft
authorJonathan Chan <jyc@eqv.io>
Thu, 18 Aug 2016 15:30:37 -0700
changeset 402906 100dd59be2f5f207dfdb81a0f0ebe1e87b0b1836
parent 402905 9448fecb492ccf43274abf6962e9372db9f0f782
child 402907 de545674b6682178fab8665247e343f0583bed99
push id26775
push userjchan@mozilla.com
push dateThu, 18 Aug 2016 22:38:41 +0000
reviewersdbaron
bugs1273706
milestone51.0a1
Bug 1273706 - Part 17: Add an aAllowVariableReferences parameter to ParseVariableDeclaration. r?dbaron We allow variable references when parsing variable declarations in support conditions or to load into a |Declaration| object. However, we expect resolved variable declarations (that is, declarations with variable values substituted) to not contain variable references, so we disallow variable references there. In particular, a future patch in this series adds a ParseTypedValue method. This expects fully resolved values (otherwise they couldn't be typed), so it disallows variable references. MozReview-Commit-ID: 2Y2nBmngK82
layout/style/nsCSSParser.cpp
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -1103,21 +1103,24 @@ protected:
   bool ParseWebkitTextStroke();
 
   /**
    * Parses a variable value from a custom property declaration.
    *
    * @param aType Out parameter into which will be stored the type of variable
    *   value, indicating whether the parsed value was a token stream or one of
    *   the CSS-wide keywords.
+   * @param aAllowVariableReferences Whether to allow variable references
+   *   in the value.
    * @param aValue Out parameter into which will be stored the token stream
    *   as a string, if the parsed custom property did take a token stream.
    * @return Whether parsing succeeded.
    */
   bool ParseVariableDeclaration(CSSVariableDeclarations::Type* aType,
+                                bool aAllowVariableReferences,
                                 nsString& aValue);
 
   /**
    * Parses a CSS variable value.  This could be 'initial', 'inherit', 'unset'
    * or a token stream, which may or may not include variable references.
    *
    * @param aType Out parameter into which the type of the variable value
    *   will be stored.
@@ -2069,17 +2072,17 @@ CSSParserImpl::ParseVariable(const nsASt
   InitScanner(scanner, reporter, aSheetURI, aBaseURI, aSheetPrincipal);
   mSection = eCSSSection_General;
 
   *aChanged = false;
 
   CSSVariableDeclarations::Type variableType;
   nsString variableValue;
 
-  bool parsedOK = ParseVariableDeclaration(&variableType, variableValue);
+  bool parsedOK = ParseVariableDeclaration(&variableType, true, variableValue);
 
   // We should now be at EOF
   if (parsedOK && GetToken(true)) {
     REPORT_UNEXPECTED_TOKEN(PEExpectEndValue);
     parsedOK = false;
   }
 
   if (!parsedOK) {
@@ -2381,17 +2384,17 @@ CSSParserImpl::EvaluateSupportsDeclarati
 
   if (propID == eCSSPropertyExtra_variable) {
     MOZ_ASSERT(Substring(aProperty, 0,
                          CSS_CUSTOM_NAME_PREFIX_LENGTH).EqualsLiteral("--"));
     const nsDependentSubstring varName =
       Substring(aProperty, CSS_CUSTOM_NAME_PREFIX_LENGTH);  // remove '--'
     CSSVariableDeclarations::Type variableType;
     nsString variableValue;
-    parsedOK = ParseVariableDeclaration(&variableType, variableValue) &&
+    parsedOK = ParseVariableDeclaration(&variableType, true, variableValue) &&
                !GetToken(true);
   } else {
     parsedOK = ParseProperty(propID) && !GetToken(true);
 
     mTempData.ClearProperty(propID);
     mTempData.AssertInitialState();
   }
 
@@ -4709,17 +4712,17 @@ CSSParserImpl::ParseSupportsConditionInP
       } else if (propID == eCSSPropertyExtra_variable) {
         if (ExpectSymbol(')', false)) {
           UngetToken();
           return false;
         }
         CSSVariableDeclarations::Type variableType;
         nsString variableValue;
         aConditionMet =
-          ParseVariableDeclaration(&variableType, variableValue) &&
+          ParseVariableDeclaration(&variableType, true, variableValue) &&
           ParsePriority() != ePriority_Error;
         if (!aConditionMet) {
           SkipUntil(')');
           UngetToken();
         }
       } else {
         if (ExpectSymbol(')', true)) {
           UngetToken();
@@ -7327,17 +7330,17 @@ CSSParserImpl::ParseDeclaration(css::Dec
   nsString variableValue;
 
   // Check if the property name is a custom property.
   bool customProperty = nsLayoutUtils::CSSVariablesEnabled() &&
                         nsCSSProps::IsCustomPropertyName(propertyName) &&
                         aContext == eCSSContext_General;
 
   if (customProperty) {
-    if (!ParseVariableDeclaration(&variableType, variableValue)) {
+    if (!ParseVariableDeclaration(&variableType, true, variableValue)) {
       REPORT_UNEXPECTED_P(PEValueParsingError, propertyName);
       REPORT_UNEXPECTED(PEDeclDropped);
       OUTPUT_ERROR();
       return false;
     }
   } else {
     // Map property name to its ID.
     propID = LookupEnabledProperty(propertyName);
@@ -17250,31 +17253,42 @@ CSSParserImpl::ParseAll()
   // to all content-visible properties.
   CSSPROPS_FOR_SHORTHAND_SUBPROPERTIES(p, eCSSProperty_all,
                                        CSSEnabledState::eForAllContent) {
     AppendValue(*p, value);
   }
   return true;
 }
 
+static void
+RecordVariableReference(const nsAString&, void* aFound)
+{
+  *static_cast<bool*>(aFound) = true;
+}
+
 bool
 CSSParserImpl::ParseVariableDeclaration(CSSVariableDeclarations::Type* aType,
+                                        bool aAllowVariableReferences,
                                         nsString& aValue)
 {
   CSSVariableDeclarations::Type type;
   nsString variableValue;
   bool dropBackslash;
   nsString impliedCharacters;
+  bool gotDisallowedVariableReference = false;
 
   // Record the token stream while parsing a variable value.
   if (!mInSupportsCondition) {
     mScanner->StartRecording();
   }
   if (!ParseValueWithVariables(&type, &dropBackslash, impliedCharacters,
-                               nullptr, nullptr)) {
+                               aAllowVariableReferences ?
+                                 nullptr : &RecordVariableReference,
+                               &gotDisallowedVariableReference) ||
+      gotDisallowedVariableReference) {
     if (!mInSupportsCondition) {
       mScanner->StopRecording();
     }
     return false;
   }
 
   if (!mInSupportsCondition) {
     if (type == CSSVariableDeclarations::eTokenStream) {