Bug 941300 - Make circle radial-gradient invalid if two radii values are given. r=dbaron
authorMasatoshi Kimura <VYV03354@nifty.ne.jp>
Sat, 14 Dec 2013 13:22:48 +0900
changeset 160498 8bb981d3522bb4919f28f4aa74b0796741cd3914
parent 160497 be5f5ca171f029571171f2da960c910d3808ebd3
child 160499 616533049b4c0eaff5cda8b447359a9004afb661
push id37605
push userVYV03354@nifty.ne.jp
push dateSat, 14 Dec 2013 04:23:11 +0000
treeherdermozilla-inbound@8bb981d3522b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs941300
milestone29.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 941300 - Make circle radial-gradient invalid if two radii values are given. r=dbaron
layout/style/nsCSSParser.cpp
layout/style/nsCSSValue.h
layout/style/test/property_database.js
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -6891,39 +6891,54 @@ CSSParserImpl::ParseRadialGradient(nsCSS
                                nsCSSProps::kRadialGradientSizeKTable);
   if (haveSize) {
     if (!haveShape) {
       // <size> <shape>
       haveShape = ParseVariant(cssGradient->GetRadialShape(), VARIANT_KEYWORD,
                                nsCSSProps::kRadialGradientShapeKTable);
     }
   } else if (!aIsLegacy) {
+    // Save RadialShape before parsing RadiusX because RadialShape and
+    // RadiusX share the storage.
+    int32_t shape =
+      cssGradient->GetRadialShape().GetUnit() == eCSSUnit_Enumerated ?
+      cssGradient->GetRadialShape().GetIntValue() : -1;
     // <length> | [<length> | <percentage>]{2}
+    cssGradient->mIsExplicitSize = true;
     haveSize =
       ParseNonNegativeVariant(cssGradient->GetRadiusX(), VARIANT_LP, nullptr);
-    if (haveSize) {
+    if (!haveSize) {
+      // It was not an explicit size after all.
+      // Note that ParseNonNegativeVariant may have put something
+      // invalid into our storage, but only in the case where it was
+      // rejected only for being negative.  Since this means the token
+      // was a length or a percentage, we know it's not valid syntax
+      // (which must be a comma, the 'at' keyword, or a color), so we
+      // know this value will be dropped.  This means it doesn't matter
+      // that we have something invalid in our storage.
+      cssGradient->mIsExplicitSize = false;
+    } else {
       // vertical extent is optional
       bool haveYSize =
         ParseNonNegativeVariant(cssGradient->GetRadiusY(), VARIANT_LP, nullptr);
       if (!haveShape) {
         nsCSSValue shapeValue;
         haveShape = ParseVariant(shapeValue, VARIANT_KEYWORD,
                                  nsCSSProps::kRadialGradientShapeKTable);
-      }
-      int32_t shape =
-        cssGradient->GetRadialShape().GetUnit() == eCSSUnit_Enumerated ?
-        cssGradient->GetRadialShape().GetIntValue() : -1;
+        if (haveShape) {
+          shape = shapeValue.GetIntValue();
+        }
+      }
       if (haveYSize
             ? shape == NS_STYLE_GRADIENT_SHAPE_CIRCULAR
             : cssGradient->GetRadiusX().GetUnit() == eCSSUnit_Percent ||
               shape == NS_STYLE_GRADIENT_SHAPE_ELLIPTICAL) {
         SkipUntil(')');
         return false;
       }
-      cssGradient->mIsExplicitSize = true;
     }
   }
 
   if ((haveShape || haveSize) && ExpectSymbol(',', true)) {
     // [ <shape> || <size> ] ,
     return ParseGradientColorStops(cssGradient, aValue);
   }
 
--- a/layout/style/nsCSSValue.h
+++ b/layout/style/nsCSSValue.h
@@ -1137,24 +1137,56 @@ struct nsCSSValueGradient {
   // line position and angle
   nsCSSValuePair mBgPos;
   nsCSSValue mAngle;
 
   // Only meaningful if mIsRadial is true
 private:
   nsCSSValue mRadialValues[2];
 public:
-  nsCSSValue& GetRadialShape() { return mRadialValues[0]; }
-  const nsCSSValue& GetRadialShape() const { return mRadialValues[0]; }
-  nsCSSValue& GetRadialSize() { return mRadialValues[1]; }
-  const nsCSSValue& GetRadialSize() const { return mRadialValues[1]; }
-  nsCSSValue& GetRadiusX() { return mRadialValues[0]; }
-  const nsCSSValue& GetRadiusX() const { return mRadialValues[0]; }
-  nsCSSValue& GetRadiusY() { return mRadialValues[1]; }
-  const nsCSSValue& GetRadiusY() const { return mRadialValues[1]; }
+  nsCSSValue& GetRadialShape()
+  {
+    MOZ_ASSERT(!mIsExplicitSize);
+    return mRadialValues[0];
+  }
+  const nsCSSValue& GetRadialShape() const
+  {
+    MOZ_ASSERT(!mIsExplicitSize);
+    return mRadialValues[0];
+  }
+  nsCSSValue& GetRadialSize()
+  {
+    MOZ_ASSERT(!mIsExplicitSize);
+    return mRadialValues[1];
+  }
+  const nsCSSValue& GetRadialSize() const
+  {
+    MOZ_ASSERT(!mIsExplicitSize);
+    return mRadialValues[1];
+  }
+  nsCSSValue& GetRadiusX()
+  {
+    MOZ_ASSERT(mIsExplicitSize);
+    return mRadialValues[0];
+  }
+  const nsCSSValue& GetRadiusX() const
+  {
+    MOZ_ASSERT(mIsExplicitSize);
+    return mRadialValues[0];
+  }
+  nsCSSValue& GetRadiusY()
+  {
+    MOZ_ASSERT(mIsExplicitSize);
+    return mRadialValues[1];
+  }
+  const nsCSSValue& GetRadiusY() const
+  {
+    MOZ_ASSERT(mIsExplicitSize);
+    return mRadialValues[1];
+  }
 
   InfallibleTArray<nsCSSValueGradientStop> mStops;
 
   bool operator==(const nsCSSValueGradient& aOther) const
   {
     if (mIsRadial != aOther.mIsRadial ||
         mIsRepeating != aOther.mIsRepeating ||
         mIsLegacySyntax != aOther.mIsLegacySyntax ||
--- a/layout/style/test/property_database.js
+++ b/layout/style/test/property_database.js
@@ -1508,20 +1508,22 @@ var gCSSProperties = {
 		"radial-gradient(closest-corner ellipse, red, blue)",
 
 		"radial-gradient(43px, red, blue)",
 		"radial-gradient(43px 43px, red, blue)",
 		"radial-gradient(50% 50%, red, blue)",
 		"radial-gradient(43px 50%, red, blue)",
 		"radial-gradient(50% 43px, red, blue)",
 		"radial-gradient(circle 43px, red, blue)",
+		"radial-gradient(43px circle, red, blue)",
 		"radial-gradient(ellipse 43px 43px, red, blue)",
 		"radial-gradient(ellipse 50% 50%, red, blue)",
 		"radial-gradient(ellipse 43px 50%, red, blue)",
 		"radial-gradient(ellipse 50% 43px, red, blue)",
+		"radial-gradient(50% 43px ellipse, red, blue)",
 
 		"radial-gradient(farthest-corner at top left, red, blue)",
 		"radial-gradient(ellipse closest-corner at 45px, red, blue)",
 		"radial-gradient(circle farthest-side at 45px, red, blue)",
 		"radial-gradient(closest-side ellipse at 50%, red, blue)",
 		"radial-gradient(farthest-corner circle at 4em, red, blue)",
 
 		"radial-gradient(30% 40% at top left, red, blue)",
@@ -1813,16 +1815,24 @@ var gCSSProperties = {
 			"-moz-repeating-linear-gradient(to bottom bottom, red, blue)",
 			"-moz-repeating-linear-gradient(to left left, red, blue)",
 			"-moz-repeating-linear-gradient(to right right, red, blue)",
 
 			"-moz-repeating-radial-gradient(to top left at 30% 40%, red, blue)",
 			"-moz-repeating-radial-gradient(ellipse at 45px closest-corner, red, blue)",
 			"-moz-repeating-radial-gradient(circle at 45px farthest-side, red, blue)",
 
+			"radial-gradient(circle 175px 20px, black, white)",
+			"radial-gradient(175px 20px circle, black, white)",
+			"radial-gradient(ellipse 175px, black, white)",
+			"radial-gradient(175px ellipse, black, white)",
+			"radial-gradient(50%, red, blue)",
+			"radial-gradient(circle 50%, red, blue)",
+			"radial-gradient(50% circle, red, blue)",
+
 			/* Valid only when prefixed */
 			"linear-gradient(top left, red, blue)",
 			"linear-gradient(0 0, red, blue)",
 			"linear-gradient(20% bottom, red, blue)",
 			"linear-gradient(center 20%, red, blue)",
 			"linear-gradient(left 35px, red, blue)",
 			"linear-gradient(10% 10em, red, blue)",
 			"linear-gradient(44px top, red, blue)",