Bug 511803 patch 2: Convert all eCSSUnit_Function storage to use nsCSSKeyword. This leads to one behavior change, which is case canonicalization for font-variant-alternates function values. r=bzbarsky

--HG--
rename : layout/style/test/test_bug721136.html => layout/style/test/test_specified_value_serialization.html

layout/style/nsCSSParser.cpp

@@ -673,7 +673,7 @@ protected:
 
   /* Functions for transform Parsing */
   bool ParseSingleTransform(bool aIsPrefixed, nsCSSValue& aValue, bool& aIs3D);
-  bool ParseFunction(const nsString &aFunction, const int32_t aAllowedTypes[],
+  bool ParseFunction(nsCSSKeyword aFunction, const int32_t aAllowedTypes[],
                      int32_t aVariantMaskAll, uint16_t aMinElems,
                      uint16_t aMaxElems, nsCSSValue &aValue);
   bool ParseFunctionInternals(const int32_t aVariantMask[],
@@ -8764,7 +8764,7 @@ CSSParserImpl::ParseSingleAlternate(int32_t& aWhichFeature,
       keyword == eCSSKeyword_character_variant) {
     maxElems = MAX_ALLOWED_FEATURES;
   }
-  return ParseFunction(mToken.mIdent, nullptr, VARIANT_IDENTIFIER,
+  return ParseFunction(keyword, nullptr, VARIANT_IDENTIFIER,
                        1, maxElems, aValue);
 }
 
@@ -9747,7 +9747,7 @@ CSSParserImpl::ParseFunctionInternals(const int32_t aVariantMask[],
  * @param aValue (out) The value that was parsed.
  */
 bool
-CSSParserImpl::ParseFunction(const nsString &aFunction,
+CSSParserImpl::ParseFunction(nsCSSKeyword aFunction,
                              const int32_t aAllowedTypes[],
                              int32_t aAllowedTypesAll,
                              uint16_t aMinElems, uint16_t aMaxElems,
@@ -9763,12 +9763,6 @@ CSSParserImpl::ParseFunction(const nsString &aFunction,
    */
   static const arrlen_t MAX_ALLOWED_ELEMS = 0xFFFE;
 
-  /* Make a copy of the function name, since the reference is _probably_ to
-   * mToken.mIdent, which is going to get overwritten during the course of this
-   * function.
-   */
-  nsString functionName(aFunction);
-
   /* Read in a list of values as an array, failing if we can't or if
    * it's out of bounds.
    */
@@ -9778,24 +9772,18 @@ CSSParserImpl::ParseFunction(const nsString &aFunction,
     return false;
   }
 
-  /* Now, convert this array into an nsCSSValue::Array object.
+  /*
-   * We'll need N + 1 spots, one for the function name and the rest for the
+   * In case the user has given us more than 2^16 - 2 arguments,
-   * arguments.  In case the user has given us more than 2^16 - 2 arguments,
    * we'll truncate them at 2^16 - 2 arguments.
    */
-  uint16_t numElements = (foundValues.Length() <= MAX_ALLOWED_ELEMS ?
+  uint16_t numArgs = std::min(foundValues.Length(), MAX_ALLOWED_ELEMS);
-                          foundValues.Length() + 1 : MAX_ALLOWED_ELEMS);
   nsRefPtr<nsCSSValue::Array> convertedArray =
-    nsCSSValue::Array::Create(numElements);
+    aValue.InitFunction(aFunction, numArgs);
 
   /* Copy things over. */
-  convertedArray->Item(0).SetStringValue(functionName, eCSSUnit_Ident);
+  for (uint16_t index = 0; index < numArgs; ++index)
-  for (uint16_t index = 0; index + 1 < numElements; ++index)
     convertedArray->Item(index + 1) = foundValues[static_cast<arrlen_t>(index)];
 
-  /* Fill in the outparam value with the array. */
-  aValue.SetArrayValue(convertedArray, eCSSUnit_Function);
-
   /* Return it! */
   return true;
 }
@@ -10022,37 +10010,7 @@ CSSParserImpl::ParseSingleTransform(bool aIsPrefixed,
                                    minElems, maxElems, variantMask, aIs3D))
     return false;
 
-  // Bug 721136: Normalize the identifier to lowercase, except that things
+  return ParseFunction(keyword, variantMask, 0, minElems, maxElems, aValue);
-  // like scaleX should have the last character capitalized.  This matches
-  // what other browsers do.
-  nsContentUtils::ASCIIToLower(mToken.mIdent);
-  switch (keyword) {
-    case eCSSKeyword_rotatex:
-    case eCSSKeyword_scalex:
-    case eCSSKeyword_skewx:
-    case eCSSKeyword_translatex:
-      mToken.mIdent.Replace(mToken.mIdent.Length() - 1, 1, PRUnichar('X'));
-      break;
-
-    case eCSSKeyword_rotatey:
-    case eCSSKeyword_scaley:
-    case eCSSKeyword_skewy:
-    case eCSSKeyword_translatey:
-      mToken.mIdent.Replace(mToken.mIdent.Length() - 1, 1, PRUnichar('Y'));
-      break;
-
-    case eCSSKeyword_rotatez:
-    case eCSSKeyword_scalez:
-    case eCSSKeyword_translatez:
-      mToken.mIdent.Replace(mToken.mIdent.Length() - 1, 1, PRUnichar('Z'));
-      break;
-
-    default:
-      break;
-  }
-
-  return ParseFunction(mToken.mIdent, variantMask, 0, minElems,
-                       maxElems, aValue);
 }
 
 /* Parses a transform property list by continuously reading in properties

layout/style/nsCSSValue.cpp

@@ -615,8 +615,7 @@ nsCSSValue::EqualsFunction(nsCSSKeyword aFunctionId) const
                     func->Item(0).GetUnit() == eCSSUnit_Enumerated,
                     "illegally structured function value");
 
-  nsCSSKeyword thisFunctionId =
+  nsCSSKeyword thisFunctionId = func->Item(0).GetKeywordValue();
-    static_cast<nsCSSKeyword>(func->Item(0).GetIntValue());
   return thisFunctionId == aFunctionId;
 }
 
@@ -775,13 +774,38 @@ nsCSSValue::AppendToString(nsCSSProperty aProperty, nsAString& aResult) const
     const nsCSSValue& functionName = array->Item(0);
     if (functionName.GetUnit() == eCSSUnit_Enumerated) {
       // We assume that the first argument is always of nsCSSKeyword type.
-      const nsCSSKeyword functionId =
+      const nsCSSKeyword functionId = functionName.GetKeywordValue();
-        static_cast<nsCSSKeyword>(functionName.GetIntValue());
+      NS_ConvertASCIItoUTF16 ident(nsCSSKeywords::GetStringValue(functionId));
-      nsStyleUtil::AppendEscapedCSSIdent(
+      // Bug 721136: Normalize the identifier to lowercase, except that things
-        NS_ConvertASCIItoUTF16(nsCSSKeywords::GetStringValue(functionId)),
+      // like scaleX should have the last character capitalized.  This matches
-        aResult);
+      // what other browsers do.
+      switch (functionId) {
+        case eCSSKeyword_rotatex:
+        case eCSSKeyword_scalex:
+        case eCSSKeyword_skewx:
+        case eCSSKeyword_translatex:
+          ident.Replace(ident.Length() - 1, 1, PRUnichar('X'));
+          break;
+
+        case eCSSKeyword_rotatey:
+        case eCSSKeyword_scaley:
+        case eCSSKeyword_skewy:
+        case eCSSKeyword_translatey:
+          ident.Replace(ident.Length() - 1, 1, PRUnichar('Y'));
+          break;
+
+        case eCSSKeyword_rotatez:
+        case eCSSKeyword_scalez:
+        case eCSSKeyword_translatez:
+          ident.Replace(ident.Length() - 1, 1, PRUnichar('Z'));
+          break;
+
+        default:
+          break;
+      }
+      nsStyleUtil::AppendEscapedCSSIdent(ident, aResult);
     } else {
-      functionName.AppendToString(aProperty, aResult);
+      MOZ_ASSERT(false, "should no longer have non-enumerated functions");
     }
     aResult.AppendLiteral("(");
 

layout/style/nsCSSValue.h

@@ -164,6 +164,7 @@ enum nsCSSUnit {
   eCSSUnit_Steps        = 24,     // (nsCSSValue::Array*) a list of (integer, enumerated)
   eCSSUnit_Function     = 25,     // (nsCSSValue::Array*) a function with
                                   //  parameters.  First elem of array is name,
+                                  //  an nsCSSKeyword as eCSSUnit_Enumerated,
                                   //  the rest of the values are arguments.
 
   // The top level of a calc() expression is eCSSUnit_Calc.  All
@@ -346,6 +347,12 @@ public:
     return mValue.mInt;
   }
 
+  nsCSSKeyword GetKeywordValue() const
+  {
+    NS_ABORT_IF_FALSE(mUnit == eCSSUnit_Enumerated, "not a keyword value");
+    return static_cast<nsCSSKeyword>(mValue.mInt);
+  }
+
   float GetPercentValue() const
   {
     NS_ABORT_IF_FALSE(mUnit == eCSSUnit_Percent, "not a percent value");

layout/style/nsStyleAnimation.cpp

@@ -150,9 +150,7 @@ AppendFunction(nsCSSKeyword aTransformFunction)
   }
 
   nsRefPtr<nsCSSValue::Array> arr = nsCSSValue::Array::Create(nargs + 1);
-  arr->Item(0).SetStringValue(
+  arr->Item(0).SetIntValue(aTransformFunction, eCSSUnit_Enumerated);
-    NS_ConvertUTF8toUTF16(nsCSSKeywords::GetStringValue(aTransformFunction)),
-    eCSSUnit_Ident);
 
   return arr.forget();
 }

layout/style/nsStyleTransformMatrix.cpp

@@ -597,9 +597,8 @@ MatrixForTransformFunction(gfx3DMatrix& aMatrix,
 nsCSSKeyword
 TransformFunctionOf(const nsCSSValue::Array* aData)
 {
-  nsAutoString keyword;
+  MOZ_ASSERT(aData->Item(0).GetUnit() == eCSSUnit_Enumerated);
-  aData->Item(0).GetStringValue(keyword);
+  return aData->Item(0).GetKeywordValue();
-  return nsCSSKeywords::LookupKeyword(keyword);
 }
 
 gfx3DMatrix

layout/style/nsStyleUtil.cpp

@@ -353,9 +353,7 @@ nsStyleUtil::ComputeFunctionalAlternates(const nsCSSValueList* aList,
     const nsCSSValue::Array *func = curr->mValue.GetArrayValue();
 
     // lookup propval
-    nsAutoString keywordStr;
+    nsCSSKeyword key = func->Item(0).GetKeywordValue();
-    func->Item(0).GetStringValue(keywordStr);
-    nsCSSKeyword key = nsCSSKeywords::LookupKeyword(keywordStr);
     NS_ASSERTION(key != eCSSKeyword_UNKNOWN, "unknown alternate property value");
 
     int32_t alternate;

layout/style/test/Makefile.in

@@ -149,6 +149,7 @@ MOCHITEST_FILES =	test_acid3_test46.html \
 		test_selectors.html \
 		test_selectors_on_anonymous_content.html \
 		test_shorthand_property_getters.html \
+		test_specified_value_serialization.html \
 		test_style_struct_copy_constructors.html \
 		test_supports_rules.html \
 		test_system_font_serialization.html \
@@ -206,7 +207,6 @@ MOCHITEST_FILES =	test_acid3_test46.html \
 		visited_image_loading_frame.html \
 		visited_image_loading_frame_empty.html \
 		test_load_events_on_stylesheets.html \
-		test_bug721136.html \
 		test_page_parser.html \
 		test_bug732153.html \
 		test_bug732209.html \

layout/style/test/test_specified_value_serialization.html

@@ -41,5 +41,22 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=721136
   is(document.documentElement.style.MozTransform, arr[1],
     "incorrect serialization");
 });
+
+var elt = document.documentElement;
+
+elt.setAttribute("style",
+                 "transform: tRANslatEX(5px) TRanslATey(10px) translatez(2px) ROTATEX(30deg) rotateY(30deg) rotatez(5deg) SKEWx(10deg) skewy(10deg) scaleX(2) SCALEY(0.5) scalez(2)");
+is(elt.style.getPropertyValue("transform"),
+   "translateX(5px) translateY(10px) translateZ(2px) rotateX(30deg) rotateY(30deg) rotateZ(5deg) skewX(10deg) skewY(10deg) scaleX(2) scaleY(0.5) scaleZ(2)",
+   "expected case canonicalization of transform functions");
+
+elt.setAttribute("style",
+                 "font-variant-alternates: SWASH(fOo) stYLIStiC(Bar)");
+is(elt.style.getPropertyValue("font-variant-alternates"),
+   "swash(fOo) stylistic(Bar)",
+   "expected case canonicalization of transform functions");
+
+elt.setAttribute("style", ""); // leave the page in a useful state
+
 </script>
 </pre>