Bug 790903: Accept unitless 0 as 'flex-basis' component in 'flex' sho…

…rthand. r=dbaron

layout/style/nsCSSParser.cpp

@@ -5197,16 +5197,21 @@ CSSParserImpl::ParseFlex()
   //
   // More details in each section below.
 
-  // (a) Parse first component. It's either 'flex-basis' or 'flex-grow', so
-  // we allow anything that would be legal to specify for the 'flex-basis'
-  // property (except for "inherit") and we also allow VARIANT_NUMBER so that
-  // we'll accept 'flex-grow' values (and importantly, so that we'll treat
-  // unitless 0 as a number instead of a length, since the flexbox spec
-  // disallows unitless 0 as a flex-basis value in the shorthand).
-  uint32_t variantMask = VARIANT_NUMBER |
+  uint32_t flexBasisVariantMask =
     (nsCSSProps::ParserVariant(eCSSProperty_flex_basis) & ~(VARIANT_INHERIT));
 
-  if (!ParseNonNegativeVariant(tmpVal, variantMask, nsCSSProps::kWidthKTable)) {
+  // (a) Parse first component. It can be either be a 'flex-basis' value or a
+  // 'flex-grow' value, so we use the flex-basis-specific variant mask, along
+  //  with VARIANT_NUMBER to accept 'flex-grow' values.
+  //
+  // NOTE: if we encounter unitless 0 here, we *must* interpret it as a
+  // 'flex-grow' value (a number), *not* as a 'flex-basis' value (a length).
+  // Conveniently, that's the behavior this combined variant-mask gives us --
+  // it'll treat unitless 0 as a number. The flexbox spec requires this:
+  // "a unitless zero that is not already preceded by two flex factors must be
+  //  interpreted as a flex factor.
+  if (!ParseNonNegativeVariant(tmpVal, flexBasisVariantMask | VARIANT_NUMBER,
+                               nsCSSProps::kWidthKTable)) {
     // First component was not a valid flex-basis or flex-grow value. Fail.
     return false;
   }
@@ -5237,20 +5242,24 @@ CSSParserImpl::ParseFlex()
     // d) Finally: If we didn't get flex-basis at the beginning, try to parse
     //    it now, at the end.
     //
-    // NOTE: Even though we're looking for a (length-ish) flex-basis value, we
-    // *do* need to pass VARIANT_NUMBER as part of |variantMask| here.  This
-    // ensures that we'll parse unitless '0' as a number, rather than as a
-    // length, so that we can reject it (along with any other number) below.
-    // (The flexbox spec disallows unitless '0' as a flex-basis value in the
-    // 'flex' shorthand.)
+    // NOTE: If we encounter unitless 0 in this final position, we'll parse it
+    // as a 'flex-basis' value.  That's OK, because we know it must have
+    // been "preceded by 2 flex factors" (justification below), which gets us
+    // out of the spec's requirement of otherwise having to treat unitless 0
+    // as a flex factor.
+    //
+    // JUSTIFICATION: How do we know that a unitless 0 here must have been
+    // preceded by 2 flex factors? Well, suppose we had a unitless 0 that
+    // was preceded by only 1 flex factor.  Then, we would have already
+    // accepted this unitless 0 as the 'flex-shrink' value, up above (since
+    // it's a valid flex-shrink value), and we'd have moved on to the next
+    // token (if any). And of course, if we instead had a unitless 0 preceded
+    // by *no* flex factors (if it were the first token), we would've already
+    // parsed it in our very first call to ParseNonNegativeVariant().  So, any
+    // unitless 0 encountered here *must* have been preceded by 2 flex factors.
     if (!wasFirstComponentFlexBasis &&
-        ParseNonNegativeVariant(tmpVal, variantMask,
+        ParseNonNegativeVariant(tmpVal, flexBasisVariantMask,
                                 nsCSSProps::kWidthKTable)) {
-      if (tmpVal.GetUnit() == eCSSUnit_Number) {
-        // This is where we reject "0 0 0" (which the spec says is invalid,
-        // because we must reject unitless 0 as a flex-basis value).
-        return false;
-      }
       flexBasis = tmpVal;
     }
   }

layout/style/test/file_flexbox_flex_shorthand.html

@@ -203,6 +203,12 @@
 
   // Three Values --> Sets all three subproperties
   // ---------------------------------------------
+  {
+    "flex":        "0 0 0",
+    "flex-grow":   "0",
+    "flex-shrink": "0",
+    "flex-basis":  "0px",
+  },
   {
     "flex":        "0.0 0.00 0px",
     "flex-grow":   "0",

layout/style/test/property_database.js

@@ -4106,6 +4106,7 @@ if (SpecialPowers.getBoolPref("layout.css.flexbox.enabled")) {
 			"0 1",
 			"0.5",
 			"1.2 3.4",
+			"0 0 0",
 			"0 0 0px",
 			"0px 0 0",
 			"5px 0 0",
@@ -4120,7 +4121,6 @@ if (SpecialPowers.getBoolPref("layout.css.flexbox.enabled")) {
 			"-0"
 		],
 		invalid_values: [
-			"0 0 0",
 			"1 2px 3",
 			"1 auto 3",
 			"1px 2 3px",