Bug 1112014 - Avoid false negatives in CssPropertySupportsType. r=heycam

IGNORE IDL

--HG--
extra : rebase_source : f2bc30f0fe334823c334f04e42e89971c07e157b

layout/inspector/inDOMUtils.cpp

@@ -653,6 +653,125 @@ inDOMUtils::CssPropertyIsShorthand(const nsAString& aProperty, bool *_retval)
   return NS_OK;
 }
 
+// A helper function that determines whether the given property
+// supports the given type.
+static bool
+PropertySupportsVariant(nsCSSProperty aPropertyID, uint32_t aVariant)
+{
+  if (nsCSSProps::IsShorthand(aPropertyID)) {
+    // We need a special case for border here, because while it resets
+    // border-image, it can't actually parse an image.
+    if (aPropertyID == eCSSProperty_border) {
+      return (aVariant & (VARIANT_COLOR | VARIANT_LENGTH)) != 0;
+    }
+
+    for (const nsCSSProperty* props = nsCSSProps::SubpropertyEntryFor(aPropertyID);
+         *props != eCSSProperty_UNKNOWN; ++props) {
+      if (PropertySupportsVariant(*props, aVariant)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  // Properties that are parsed by functions must have their
+  // attributes hand-maintained here.
+  if (nsCSSProps::PropHasFlags(aPropertyID, CSS_PROPERTY_VALUE_PARSER_FUNCTION) ||
+      nsCSSProps::PropertyParseType(aPropertyID) == CSS_PROPERTY_PARSE_FUNCTION) {
+    // These must all be special-cased.
+    uint32_t supported;
+    switch (aPropertyID) {
+      case eCSSProperty_border_image_slice:
+      case eCSSProperty_grid_template:
+      case eCSSProperty_grid:
+        supported = VARIANT_PN;
+        break;
+
+      case eCSSProperty_border_image_outset:
+        supported = VARIANT_LN;
+        break;
+
+      case eCSSProperty_border_image_width:
+      case eCSSProperty_stroke_dasharray:
+        supported = VARIANT_LPN;
+        break;
+
+      case eCSSProperty_border_top_left_radius:
+      case eCSSProperty_border_top_right_radius:
+      case eCSSProperty_border_bottom_left_radius:
+      case eCSSProperty_border_bottom_right_radius:
+      case eCSSProperty_background_position:
+      case eCSSProperty_background_size:
+      case eCSSProperty_grid_auto_columns:
+      case eCSSProperty_grid_auto_rows:
+      case eCSSProperty_grid_template_columns:
+      case eCSSProperty_grid_template_rows:
+      case eCSSProperty_object_position:
+      case eCSSProperty_scroll_snap_coordinate:
+      case eCSSProperty_scroll_snap_destination:
+      case eCSSProperty_transform_origin:
+      case eCSSProperty_perspective_origin:
+      case eCSSProperty__moz_outline_radius_topLeft:
+      case eCSSProperty__moz_outline_radius_topRight:
+      case eCSSProperty__moz_outline_radius_bottomLeft:
+      case eCSSProperty__moz_outline_radius_bottomRight:
+        supported = VARIANT_LP;
+        break;
+
+      case eCSSProperty_border_bottom_colors:
+      case eCSSProperty_border_left_colors:
+      case eCSSProperty_border_right_colors:
+      case eCSSProperty_border_top_colors:
+        supported = VARIANT_COLOR;
+        break;
+
+      case eCSSProperty_text_shadow:
+      case eCSSProperty_box_shadow:
+        supported = VARIANT_LENGTH | VARIANT_COLOR;
+        break;
+
+      case eCSSProperty_border_spacing:
+        supported = VARIANT_LENGTH;
+        break;
+
+      case eCSSProperty_content:
+      case eCSSProperty_cursor:
+      case eCSSProperty_clip_path:
+        supported = VARIANT_URL;
+        break;
+
+      case eCSSProperty_fill:
+      case eCSSProperty_stroke:
+        supported = VARIANT_COLOR | VARIANT_URL;
+        break;
+
+      case eCSSProperty_image_orientation:
+        supported = VARIANT_ANGLE;
+        break;
+
+      case eCSSProperty_filter:
+        supported = VARIANT_URL;
+        break;
+
+      case eCSSProperty_grid_column_start:
+      case eCSSProperty_grid_column_end:
+      case eCSSProperty_grid_row_start:
+      case eCSSProperty_grid_row_end:
+      case eCSSProperty_font_weight:
+        supported = VARIANT_NUMBER;
+        break;
+
+      default:
+        supported = 0;
+        break;
+    }
+
+    return (supported & aVariant) != 0;
+  }
+
+  return (nsCSSProps::ParserVariant(aPropertyID) & aVariant) != 0;
+}
+
 NS_IMETHODIMP
 inDOMUtils::CssPropertySupportsType(const nsAString& aProperty, uint32_t aType,
                                     bool *_retval)
@@ -663,6 +782,11 @@ inDOMUtils::CssPropertySupportsType(const nsAString& aProperty, uint32_t aType,
     return NS_ERROR_FAILURE;
   }
 
+  if (propertyID >= eCSSProperty_COUNT) {
+    *_retval = false;
+    return NS_OK;
+  }
+
   uint32_t variant;
   switch (aType) {
   case TYPE_LENGTH:
@@ -704,20 +828,7 @@ inDOMUtils::CssPropertySupportsType(const nsAString& aProperty, uint32_t aType,
     return NS_ERROR_NOT_AVAILABLE;
   }
 
-  if (!nsCSSProps::IsShorthand(propertyID)) {
-    *_retval = nsCSSProps::ParserVariant(propertyID) & variant;
-    return NS_OK;
-  }
-
-  for (const nsCSSProperty* props = nsCSSProps::SubpropertyEntryFor(propertyID);
-       *props != eCSSProperty_UNKNOWN; ++props) {
-    if (nsCSSProps::ParserVariant(*props) & variant) {
-      *_retval = true;
-      return NS_OK;
-    }
-  }
-
-  *_retval = false;
+  *_retval = PropertySupportsVariant(propertyID, variant);
   return NS_OK;
 }
 

layout/inspector/inIDOMUtils.idl

@@ -103,9 +103,6 @@ interface inIDOMUtils : nsISupports
   // For shorthands, checks whether there's a corresponding longhand property
   // that accepts values of this type.  Throws on unsupported properties or
   // unknown types.
-  //
-  // This function may incorrectly return false for properties that use custom
-  // parsing functions instead of table-driven parsing.
   const unsigned long TYPE_LENGTH = 0;
   const unsigned long TYPE_PERCENTAGE = 1;
   const unsigned long TYPE_COLOR = 2;

layout/style/nsCSSParser.cpp

@@ -1212,7 +1212,6 @@ protected:
   // sites.
   bool mDidUnprefixWebkitBoxInEarlierDecl; // not :1 so we can use AutoRestore
 
-#ifdef DEBUG
   // True if any parsing of URL values requires a sheet principal to have
   // been passed in the nsCSSScanner constructor.  This is usually the case.
   // It can be set to false, for example, when we create an nsCSSParser solely
@@ -1221,7 +1220,6 @@ protected:
   // not be set to false if any nsCSSValues created during parsing can escape
   // out of the parser.
   bool mSheetPrincipalRequired;
-#endif
 
   // Stack of rule groups; used for @media and such.
   InfallibleTArray<nsRefPtr<css::GroupRule> > mGroupStack;
@@ -1300,9 +1298,7 @@ CSSParserImpl::CSSParserImpl()
     mInFailingSupportsRule(false),
     mSuppressErrors(false),
     mDidUnprefixWebkitBoxInEarlierDecl(false),
-#ifdef DEBUG
     mSheetPrincipalRequired(true),
-#endif
     mNextFree(nullptr)
 {
 }
@@ -7611,8 +7607,12 @@ bool
 CSSParserImpl::SetValueToURL(nsCSSValue& aValue, const nsString& aURL)
 {
   if (!mSheetPrincipal) {
-    NS_ASSERTION(!mSheetPrincipalRequired,
-                 "Codepaths that expect to parse URLs MUST pass in an "
+    if (!mSheetPrincipalRequired) {
+      /* Pretend to succeed.  */
+      return true;
+    }
+
+    NS_NOTREACHED("Codepaths that expect to parse URLs MUST pass in an "
                   "origin principal");
     return false;
   }
@@ -15379,7 +15379,6 @@ CSSParserImpl::IsValueValidForProperty(const nsCSSProperty aPropID,
   css::ErrorReporter reporter(scanner, mSheet, mChildLoader, nullptr);
   InitScanner(scanner, reporter, nullptr, nullptr, nullptr);
 
-#ifdef DEBUG
   // We normally would need to pass in a sheet principal to InitScanner,
   // because we might parse a URL value.  However, we will never use the
   // parsed nsCSSValue (and so whether we have a sheet principal or not
@@ -15388,7 +15387,6 @@ CSSParserImpl::IsValueValidForProperty(const nsCSSProperty aPropID,
   // that it's safe to skip the assertion.
   AutoRestore<bool> autoRestore(mSheetPrincipalRequired);
   mSheetPrincipalRequired = false;
-#endif
 
   nsAutoSuppressErrors suppressErrors(this);
 

layout/style/test/mochitest.ini

@@ -110,6 +110,7 @@ support-files = file_bug829816.css
 support-files = file_bug1055933_circle-xxl.png
 [test_bug1089417.html]
 support-files = file_bug1089417_iframe.html
+[test_bug1112014.html]
 [test_cascade.html]
 [test_ch_ex_no_infloops.html]
 [test_compute_data_with_start_struct.html]

layout/style/test/test_bug1112014.html

@@ -0,0 +1,121 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=1112014
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 1112014</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script src="property_database.js"></script>
+  <script type="application/javascript;version=1.7">
+
+  let utils = SpecialPowers.Cc["@mozilla.org/inspector/dom-utils;1"]
+    .getService(SpecialPowers.Ci.inIDOMUtils);
+
+  // This holds a canonical test value for each TYPE_ constant.
+  let testValues = {
+    TYPE_LENGTH: "10px",
+    TYPE_PERCENTAGE: "50%",
+    TYPE_COLOR: "rgb(3,3,3)",
+    TYPE_URL: "url(mozilla.org)",
+    TYPE_ANGLE: "90deg",
+    TYPE_FREQUENCY: "10kHz",
+    TYPE_TIME: "1000ms",
+    TYPE_GRADIENT: "linear-gradient( 45deg, blue, red )",
+    TYPE_TIMING_FUNCTION: "cubic-bezier(0.1, 0.7, 1.0, 0.1)",
+    TYPE_IMAGE_RECT: "-moz-image-rect(url(firefox.jpg), 5%, 5%, 10%, 10%)",
+    TYPE_NUMBER: "42"
+  };
+
+  // The canonical test values don't work for all properties, in
+  // particular some shorthand properties.  For these cases we have
+  // override values.
+  let overrideValues = {
+    "font": {
+      TYPE_LENGTH: "10px san-serif",
+      TYPE_PERCENTAGE: "50% san-serif",
+      TYPE_NUMBER: "24px/1.5 san-serif"
+    },
+    "border-image": {
+      TYPE_LENGTH: "url(/somewhere) 30% / 30px stretch",
+      TYPE_IMAGE_RECT: testValues.TYPE_IMAGE_RECT + " 30 30 stretch"
+    },
+    "-moz-border-image": {
+      TYPE_LENGTH: "url(/somewhere) 30% / 30px stretch",
+      TYPE_IMAGE_RECT: testValues.TYPE_IMAGE_RECT + " 30 30 stretch"
+    },
+    "box-shadow": {
+      TYPE_LENGTH: "2px 2px",
+      TYPE_COLOR: testValues.TYPE_COLOR + " 2px 2px"
+    },
+    "text-shadow": {
+      TYPE_LENGTH: "2px 2px",
+      TYPE_COLOR: testValues.TYPE_COLOR + " 2px 2px"
+    },
+    "font-weight": {
+      TYPE_NUMBER: "400"
+    },
+    "grid-template": {
+      TYPE_LENGTH: "'something' 23px",
+      TYPE_PERCENTAGE: "'something' 23%"
+    },
+    "grid": {
+      TYPE_LENGTH: "'something' 23px",
+      TYPE_PERCENTAGE: "'something' 23%"
+    },
+  };
+
+
+  // Ensure that all the TYPE_ constants have a representative
+  // test value, to try to ensure that this test is updated
+  // whenever a new type is added.
+  let reps = [];
+  for (let tc in utils) {
+    if (/TYPE_/.test(tc)) {
+      if (!(tc in testValues)) {
+	reps.push(tc);
+      }
+    }
+  }
+  is(reps.join(","), "", "all types have representative test value");
+
+  for (let propertyName in gCSSProperties) {
+    let prop = gCSSProperties[propertyName];
+    if (prop.backend_only) {
+      // These aren't interesting to us.
+      continue;
+    }
+
+    for (let iter in testValues) {
+      let testValue = testValues[iter];
+      if (propertyName in overrideValues &&
+	  iter in overrideValues[propertyName]) {
+	testValue = overrideValues[propertyName][iter];
+      }
+
+      let supported =
+        utils.cssPropertySupportsType(propertyName, utils[iter]);
+      let parsed = utils.cssPropertyIsValid(propertyName, testValue);
+      is(supported, parsed, propertyName + " supports " + iter);
+    }
+  }
+
+  // Regression test for an assertion failure in an earlier version of
+  // the code.  Note that cssPropertySupportsType returns false for
+  // all types for a variable.
+  ok(!utils.cssPropertySupportsType("--variable", utils.TYPE_COLOR),
+     "cssPropertySupportsType returns false for variable");
+
+  </script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1112014">Mozilla Bug 1112014</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>