Bug 526402: Stop accepting unitless 0 for angles, times, and frequencies. r=zwol
authorL. David Baron <dbaron@dbaron.org>
Wed, 04 Nov 2009 18:36:18 -0800
changeset 34551 0d248c37d1ae8363e91d2c8e6d58289fd58d7c19
parent 34550 6258b513a554c5098972f89716461dd5460f09c2
child 34552 eb2a556deb115cdf7a714b6a9104abfc5ffc3c74
push id10116
push userzweinberg@mozilla.com
push dateThu, 05 Nov 2009 02:38:52 +0000
treeherdermozilla-central@eb2a556deb11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerszwol
bugs526402
milestone1.9.3a1pre
Bug 526402: Stop accepting unitless 0 for angles, times, and frequencies. r=zwol
layout/style/nsCSSParser.cpp
layout/style/nsCSSScanner.h
layout/style/test/property_database.js
layout/style/test/test_shorthand_property_getters.html
layout/style/test/test_transitions_computed_value_combinations.html
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -107,16 +107,18 @@
 #define VARIANT_INHERIT         0x020000  // H eCSSUnit_Initial, eCSSUnit_Inherit
 #define VARIANT_NONE            0x040000  // O
 #define VARIANT_NORMAL          0x080000  // M
 #define VARIANT_SYSFONT         0x100000  // eCSSUnit_System_Font
 #define VARIANT_GRADIENT        0x200000  // eCSSUnit_Gradient
 #define VARIANT_CUBIC_BEZIER    0x400000  // CSS transition timing function
 #define VARIANT_ALL             0x800000  //
 #define VARIANT_IMAGE_RECT    0x01000000  // eCSSUnit_Function
+// This is an extra bit that says that a VARIANT_ANGLE allows unitless zero:
+#define VARIANT_ZERO_ANGLE    0x02000000  // unitless zero for angles
 
 // Common combinations of variants
 #define VARIANT_AL   (VARIANT_AUTO | VARIANT_LENGTH)
 #define VARIANT_LP   (VARIANT_LENGTH | VARIANT_PERCENT)
 #define VARIANT_AH   (VARIANT_AUTO | VARIANT_INHERIT)
 #define VARIANT_AHLP (VARIANT_AH | VARIANT_LP)
 #define VARIANT_AHI  (VARIANT_AH | VARIANT_INTEGER)
 #define VARIANT_AHK  (VARIANT_AH | VARIANT_KEYWORD)
@@ -142,16 +144,17 @@
 #define VARIANT_HUO  (VARIANT_INHERIT | VARIANT_URL | VARIANT_NONE)
 #define VARIANT_AHUO (VARIANT_AUTO | VARIANT_HUO)
 #define VARIANT_HPN  (VARIANT_INHERIT | VARIANT_PERCENT | VARIANT_NUMBER)
 #define VARIANT_HN   (VARIANT_INHERIT | VARIANT_NUMBER)
 #define VARIANT_HON  (VARIANT_HN | VARIANT_NONE)
 #define VARIANT_HOS  (VARIANT_INHERIT | VARIANT_NONE | VARIANT_STRING)
 #define VARIANT_TIMING_FUNCTION (VARIANT_KEYWORD | VARIANT_CUBIC_BEZIER)
 #define VARIANT_UK   (VARIANT_URL | VARIANT_KEYWORD)
+#define VARIANT_ANGLE_OR_ZERO (VARIANT_ANGLE | VARIANT_ZERO_ANGLE)
 
 //----------------------------------------------------------------------
 
 // Your basic top-down recursive descent style parser
 class CSSParserImpl : public nsICSSParser {
 public:
   CSSParserImpl();
   virtual ~CSSParserImpl();
@@ -1801,22 +1804,22 @@ CSSParserImpl::ParseMediaQueryExpression
         rv = ParseVariant(a->Item(0), VARIANT_INTEGER, nsnull) &&
              a->Item(0).GetIntValue() > 0 &&
              ExpectSymbol('/', PR_TRUE) &&
              ParseVariant(a->Item(1), VARIANT_INTEGER, nsnull) &&
              a->Item(1).GetIntValue() > 0;
       }
       break;
     case nsMediaFeature::eResolution:
-      rv = GetToken(PR_TRUE) && mToken.IsDimension() &&
+      rv = GetToken(PR_TRUE) && mToken.mType == eCSSToken_Dimension &&
            mToken.mIntegerValid && mToken.mNumber > 0.0f;
       if (rv) {
         // No worries about whether unitless zero is allowed, since the
         // value must be positive (and we checked that above).
-        NS_ASSERTION(!mToken.mIdent.IsEmpty(), "IsDimension lied");
+        NS_ASSERTION(!mToken.mIdent.IsEmpty(), "unit lied");
         if (mToken.mIdent.LowerCaseEqualsLiteral("dpi")) {
           expr->mValue.SetFloatValue(mToken.mNumber, eCSSUnit_Inch);
         } else if (mToken.mIdent.LowerCaseEqualsLiteral("dpcm")) {
           expr->mValue.SetFloatValue(mToken.mNumber, eCSSUnit_Centimeter);
         } else {
           rv = PR_FALSE;
         }
       }
@@ -4289,27 +4292,21 @@ CSSParserImpl::TranslateDimension(nsCSSV
   } else {
     // Must be a zero number...
     NS_ASSERTION(0 == aNumber, "numbers without units must be 0");
     if ((VARIANT_LENGTH & aVariantMask) != 0) {
       units = eCSSUnit_Point;
       type = VARIANT_LENGTH;
     }
     else if ((VARIANT_ANGLE & aVariantMask) != 0) {
+      NS_ASSERTION(aVariantMask & VARIANT_ZERO_ANGLE,
+                   "must have allowed zero angle");
       units = eCSSUnit_Degree;
       type = VARIANT_ANGLE;
     }
-    else if ((VARIANT_FREQUENCY & aVariantMask) != 0) {
-      units = eCSSUnit_Hertz;
-      type = VARIANT_FREQUENCY;
-    }
-    else if ((VARIANT_TIME & aVariantMask) != 0) {
-      units = eCSSUnit_Seconds;
-      type = VARIANT_TIME;
-    }
     else {
       NS_ERROR("Variant mask does not include dimension; why were we called?");
       return PR_FALSE;
     }
   }
   if ((type & aVariantMask) != 0) {
     aValue.SetFloatValue(aNumber, units);
     return PR_TRUE;
@@ -4461,18 +4458,22 @@ CSSParserImpl::ParseVariant(nsCSSValue& 
         PRInt32 value;
         if (nsCSSProps::FindKeyword(keyword, aKeywordTable, value)) {
           aValue.SetIntValue(value, eCSSUnit_Enumerated);
           return PR_TRUE;
         }
       }
     }
   }
-  if (((aVariantMask & (VARIANT_LENGTH | VARIANT_ANGLE | VARIANT_FREQUENCY | VARIANT_TIME)) != 0) &&
-      tk->IsDimension()) {
+  if (((aVariantMask & (VARIANT_LENGTH | VARIANT_ANGLE |
+                        VARIANT_FREQUENCY | VARIANT_TIME)) != 0 &&
+       eCSSToken_Dimension == tk->mType) ||
+      ((aVariantMask & (VARIANT_LENGTH | VARIANT_ZERO_ANGLE)) != 0 &&
+       eCSSToken_Number == tk->mType &&
+       tk->mNumber == 0.0f)) {
     if (TranslateDimension(aValue, aVariantMask, tk->mNumber, tk->mIdent)) {
       return PR_TRUE;
     }
     // Put the token back; we didn't parse it, so we shouldn't consume it
     UngetToken();
     return PR_FALSE;
   }
   if (((aVariantMask & VARIANT_PERCENT) != 0) &&
@@ -6470,17 +6471,18 @@ CSSParserImpl::ParseBackgroundItem(CSSPa
                 mToken.mIdent.LowerCaseEqualsLiteral("-moz-image-rect"))) {
       if (haveImage)
         return PR_FALSE;
       haveImage = PR_TRUE;
       if (!ParseSingleValueProperty(aItem.mImage,
                                     eCSSProperty_background_image)) {
         return PR_FALSE;
       }
-    } else if (mToken.IsDimension() || tt == eCSSToken_Percentage) {
+    } else if (tt == eCSSToken_Dimension || tt == eCSSToken_Number ||
+               tt == eCSSToken_Percentage) {
       if (havePosition)
         return PR_FALSE;
       havePosition = PR_TRUE;
       if (!ParseBoxPositionValues(aItem.mPosition, PR_FALSE)) {
         return PR_FALSE;
       }
     } else {
       if (haveColor)
@@ -7626,18 +7628,18 @@ static PRBool GetFunctionParseInformatio
          eNumber,
          eTwoNumbers,
          eMatrix,
          eNumVariantMasks };
   static const PRInt32 kMaxElemsPerFunction = 6;
   static const PRInt32 kVariantMasks[eNumVariantMasks][kMaxElemsPerFunction] = {
     {VARIANT_LENGTH | VARIANT_PERCENT},
     {VARIANT_LENGTH | VARIANT_PERCENT, VARIANT_LENGTH | VARIANT_PERCENT},
-    {VARIANT_ANGLE},
-    {VARIANT_ANGLE, VARIANT_ANGLE},
+    {VARIANT_ANGLE_OR_ZERO},
+    {VARIANT_ANGLE_OR_ZERO, VARIANT_ANGLE_OR_ZERO},
     {VARIANT_NUMBER},
     {VARIANT_NUMBER, VARIANT_NUMBER},
     {VARIANT_NUMBER, VARIANT_NUMBER, VARIANT_NUMBER, VARIANT_NUMBER,
      VARIANT_LENGTH | VARIANT_PERCENT, VARIANT_LENGTH | VARIANT_PERCENT}};
 
 #ifdef DEBUG
   static const PRUint8 kVariantMaskLengths[eNumVariantMasks] =
     {1, 2, 1, 2, 1, 2, 6};
--- a/layout/style/nsCSSScanner.h
+++ b/layout/style/nsCSSScanner.h
@@ -115,21 +115,16 @@ struct nsCSSToken {
   PRInt32         mInteger2;
   nsCSSTokenType  mType;
   PRUnichar       mSymbol;
   PRPackedBool    mIntegerValid; // for number, dimension, urange
   PRPackedBool    mHasSign; // for number, percentage, and dimension
 
   nsCSSToken();
 
-  PRBool IsDimension() {
-    return PRBool((eCSSToken_Dimension == mType) ||
-                  ((eCSSToken_Number == mType) && (mNumber == 0.0f)));
-  }
-
   PRBool IsSymbol(PRUnichar aSymbol) {
     return PRBool((eCSSToken_Symbol == mType) && (mSymbol == aSymbol));
   }
 
   void AppendToString(nsString& aBuffer);
 };
 
 // CSS Scanner API. Used to tokenize an input stream using the CSS
--- a/layout/style/test/property_database.js
+++ b/layout/style/test/property_database.js
@@ -1740,37 +1740,37 @@ var gCSSProperties = {
 		invalid_values: []
 	},
 	"pause": {
 		domProp: "pause",
 		inherited: false,
 		backend_only: true,
 		type: CSS_TYPE_TRUE_SHORTHAND,
 		subproperties: [ "pause-before", "pause-after" ],
-		initial_values: [ "0", "0s", "0ms", "0 0", "0s 0ms", "0ms 0" ],
+		initial_values: [ "0s", "0ms", "0s 0ms" ],
 		other_values: [ "1s", "200ms", "-2s", "50%", "-10%", "10% 200ms", "-3s -5%" ],
-		invalid_values: [ "0px" ]
+		invalid_values: [ "0", "0px", "0 0", "0ms 0" ]
 	},
 	"pause-after": {
 		domProp: "pauseAfter",
 		inherited: false,
 		backend_only: true,
 		type: CSS_TYPE_LONGHAND,
-		initial_values: [ "0", "0s", "0ms" ],
+		initial_values: [ "0s", "0ms" ],
 		other_values: [ "1s", "200ms", "-2s", "50%", "-10%" ],
-		invalid_values: [ "0px" ]
+		invalid_values: [ "0", "0px" ]
 	},
 	"pause-before": {
 		domProp: "pauseBefore",
 		inherited: false,
 		backend_only: true,
 		type: CSS_TYPE_LONGHAND,
-		initial_values: [ "0", "0s", "0ms" ],
+		initial_values: [ "0s", "0ms" ],
 		other_values: [ "1s", "200ms", "-2s", "50%", "-10%" ],
-		invalid_values: [ "0px" ]
+		invalid_values: [ "0", "0px" ]
 	},
 	"pitch": {
 		domProp: "pitch",
 		inherited: true,
 		backend_only: true,
 		type: CSS_TYPE_LONGHAND,
 		initial_values: [ "medium" ],
 		other_values: [ "x-low", "low", "high", "x-high" ],
@@ -1967,27 +1967,27 @@ var gCSSProperties = {
 		initial_values: [ "all 0s ease 0s" ],
 		other_values: [ "width 1s linear 2s", "width 1s 2s linear", "width linear 1s 2s", "linear width 1s 2s", "linear 1s width 2s", "linear 1s 2s width", "1s width linear 2s", "1s width 2s linear", "1s 2s width linear", "1s linear width 2s", "1s linear 2s width", "1s 2s linear width", "width linear 1s", "width 1s linear", "linear width 1s", "linear 1s width", "1s width linear", "1s linear width", "1s 2s width", "1s width 2s", "width 1s 2s", "1s 2s linear", "1s linear 2s", "linear 1s 2s", "width 1s", "1s width", "linear 1s", "1s linear", "1s 2s", "2s 1s", "width", "linear", "1s", "height", "2s", "ease-in-out", "2s ease-in", "opacity linear", "ease-out 2s", "2s color, 1s width, 500ms height linear, 1s opacity 4s cubic-bezier(0.0, 0.1, 1.0, 1.0)" ],
 		invalid_values: [ "2s, 1s width", "1s width, 2s", "2s all, 1s width", "1s width, 2s all", "1s width, 2s none", "2s none, 1s width", "2s inherit", "inherit 2s", "2s width, 1s inherit", "2s inherit, 1s width", "2s initial", "2s all, 1s width", "2s width, 1s all" ]
 	},
 	"-moz-transition-delay": {
 		domProp: "MozTransitionDelay",
 		inherited: false,
 		type: CSS_TYPE_LONGHAND,
-		initial_values: [ "0", "0s", "0ms" ],
+		initial_values: [ "0s", "0ms" ],
 		other_values: [ "1s", "250ms", "-100ms", "-1s", "1s, 250ms, 2.3s"],
-		invalid_values: []
+		invalid_values: [ "0", "0px" ]
 	},
 	"-moz-transition-duration": {
 		domProp: "MozTransitionDuration",
 		inherited: false,
 		type: CSS_TYPE_LONGHAND,
-		initial_values: [ "0", "0s", "0ms" ],
+		initial_values: [ "0s", "0ms" ],
 		other_values: [ "1s", "250ms", "-1ms", "-2s", "1s, 250ms, 2.3s"],
-		invalid_values: []
+		invalid_values: [ "0", "0px" ]
 	},
 	"-moz-transition-property": {
 		domProp: "MozTransitionProperty",
 		inherited: false,
 		type: CSS_TYPE_LONGHAND,
 		initial_values: [ "all" ],
 		other_values: [ "none", "left", "top", "color", "width, height, opacity", "foobar", "auto" ],
 		invalid_values: [ "none, none", "all, all", "color, none", "none, color", "all, color", "color, all", "inherit, color", "color, inherit", "initial, color", "color, initial", "none, color", "color, none", "all, color", "color, all" ]
--- a/layout/style/test/test_shorthand_property_getters.html
+++ b/layout/style/test/test_shorthand_property_getters.html
@@ -143,26 +143,26 @@ is(e.style.background, "", "should not h
 e.setAttribute("style", "-moz-background-clip: border, border, border; -moz-background-origin: padding, padding, padding; -moz-background-size: cover, auto, contain; background-color: blue; background-image: url(404.png), none, url(404-2.png); background-attachment: fixed, scroll, scroll; background-position: top left, center, 30px 50px, bottom; background-repeat: repeat-x, repeat, no-repeat");
 is(e.style.background, "", "should not have background shorthand (background-position too long)");
 e.setAttribute("style", "-moz-background-clip: border, border, border; -moz-background-origin: padding, padding, padding; -moz-background-size: cover, auto, contain; background-color: blue; background-image: url(404.png), none, url(404-2.png); background-attachment: fixed, scroll, scroll; background-position: top left, center, 30px 50px; background-repeat: repeat-x, repeat");
 is(e.style.background, "", "should not have background shorthand (background-repeat too short)");
 e.setAttribute("style", "-moz-background-clip: border, border, border; -moz-background-origin: padding, padding, padding; -moz-background-size: cover, auto, contain, cover; background-color: blue; background-image: url(404.png), none, url(404-2.png); background-attachment: fixed, scroll, scroll; background-position: top left, center, 30px 50px; background-repeat: repeat-x, repeat, no-repeat");
 is(e.style.background, "", "should not have background shorthand (-moz-background-size too long)");
 
 // Check that we only serialize transition when the lists are the same length.
-e.setAttribute("style", "-moz-transition-property: color, width; -moz-transition-duration: 1s, 200ms; -moz-transition-timing-function: ease-in, linear; -moz-transition-delay: 0, 1s");
+e.setAttribute("style", "-moz-transition-property: color, width; -moz-transition-duration: 1s, 200ms; -moz-transition-timing-function: ease-in, linear; -moz-transition-delay: 0s, 1s");
 isnot(e.style.MozTransition, "", "should have -moz-transition shorthand (lists same length)");
-e.setAttribute("style", "-moz-transition-property: color, width, left; -moz-transition-duration: 1s, 200ms; -moz-transition-timing-function: ease-in, linear; -moz-transition-delay: 0, 1s");
+e.setAttribute("style", "-moz-transition-property: color, width, left; -moz-transition-duration: 1s, 200ms; -moz-transition-timing-function: ease-in, linear; -moz-transition-delay: 0s, 1s");
 is(e.style.MozTransition, "", "should not have -moz-transition shorthand (lists different lengths)");
-e.setAttribute("style", "-moz-transition-property: all; -moz-transition-duration: 1s, 200ms; -moz-transition-timing-function: ease-in, linear; -moz-transition-delay: 0, 1s");
+e.setAttribute("style", "-moz-transition-property: all; -moz-transition-duration: 1s, 200ms; -moz-transition-timing-function: ease-in, linear; -moz-transition-delay: 0s, 1s");
 is(e.style.MozTransition, "", "should not have -moz-transition shorthand (lists different lengths)");
-e.setAttribute("style", "-moz-transition-property: color, width; -moz-transition-duration: 1s, 200ms, 300ms; -moz-transition-timing-function: ease-in, linear; -moz-transition-delay: 0, 1s");
+e.setAttribute("style", "-moz-transition-property: color, width; -moz-transition-duration: 1s, 200ms, 300ms; -moz-transition-timing-function: ease-in, linear; -moz-transition-delay: 0s, 1s");
 is(e.style.MozTransition, "", "should not have -moz-transition shorthand (lists different lengths)");
-e.setAttribute("style", "-moz-transition-property: color, width; -moz-transition-duration: 1s, 200ms; -moz-transition-timing-function: ease-in, linear, ease-out; -moz-transition-delay: 0, 1s");
+e.setAttribute("style", "-moz-transition-property: color, width; -moz-transition-duration: 1s, 200ms; -moz-transition-timing-function: ease-in, linear, ease-out; -moz-transition-delay: 0s, 1s");
 is(e.style.MozTransition, "", "should not have -moz-transition shorthand (lists different lengths)");
-e.setAttribute("style", "-moz-transition-property: color, width; -moz-transition-duration: 1s, 200ms; -moz-transition-timing-function: ease-in, linear; -moz-transition-delay: 0, 1s, 0");
+e.setAttribute("style", "-moz-transition-property: color, width; -moz-transition-duration: 1s, 200ms; -moz-transition-timing-function: ease-in, linear; -moz-transition-delay: 0s, 1s, 0s");
 is(e.style.MozTransition, "", "should not have -moz-transition shorthand (lists different lengths)");
 
 
 </script>
 </pre>
 </body>
 </html>
--- a/layout/style/test/test_transitions_computed_value_combinations.html
+++ b/layout/style/test/test_transitions_computed_value_combinations.html
@@ -38,17 +38,17 @@ function myrand()
 // We want to test a bunch of values for each property.
 // Each of these values will also have a "computed" property filled in
 // below, so that we ensure it always computes to the same value.
 var values = {
   "-moz-transition-duration":
     [
       { lone: true,  specified: "-moz-initial" },
       { lone: false, specified: "2s" },
-      { lone: false, specified: "0" },
+      { lone: false, specified: "0s" },
       { lone: false, specified: "430ms" },
       { lone: false, specified: "-1s" },
     ],
   "-moz-transition-property":
     [
       { lone: true,  specified: "-moz-initial" },
       { lone: true,  specified: "none" },
       { lone: true,  specified: "all" },
@@ -65,17 +65,17 @@ var values = {
       { lone: false, specified: "ease" },
       { lone: false, specified: "ease-in-out" },
       { lone: false, specified: "cubic-bezier(0, 0, 0.63, 1.00)" },
     ],
   "-moz-transition-delay":
     [
       { lone: true,  specified: "-moz-initial" },
       { lone: false, specified: "2s" },
-      { lone: false, specified: "0" },
+      { lone: false, specified: "0s" },
       { lone: false, specified: "430ms" },
       { lone: false, specified: "-1s" },
     ],
 };
 
 var elt = document.getElementById("content");
 var cs = getComputedStyle(elt, "");