From 5adfab6bb92dc4ddb357db70a516c365ba9ec99e Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Wed, 29 Apr 2015 09:11:00 -0400 Subject: [PATCH] Bug 1112014 - Avoid false negatives in CssPropertySupportsType. r=heycam IGNORE IDL --HG-- extra : rebase_source : f2bc30f0fe334823c334f04e42e89971c07e157b --- layout/inspector/inDOMUtils.cpp | 139 ++++++++++++++++++++++--- layout/inspector/inIDOMUtils.idl | 3 - layout/style/nsCSSParser.cpp | 16 ++- layout/style/test/mochitest.ini | 1 + layout/style/test/test_bug1112014.html | 121 +++++++++++++++++++++ 5 files changed, 254 insertions(+), 26 deletions(-) create mode 100644 layout/style/test/test_bug1112014.html diff --git a/layout/inspector/inDOMUtils.cpp b/layout/inspector/inDOMUtils.cpp index 042fb17169ae..88b44556e5f6 100644 --- a/layout/inspector/inDOMUtils.cpp +++ b/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; } diff --git a/layout/inspector/inIDOMUtils.idl b/layout/inspector/inIDOMUtils.idl index 68df23d51dba..8e9aa4bbcf0b 100644 --- a/layout/inspector/inIDOMUtils.idl +++ b/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; diff --git a/layout/style/nsCSSParser.cpp b/layout/style/nsCSSParser.cpp index 095de41f4511..b6be119bdbff 100644 --- a/layout/style/nsCSSParser.cpp +++ b/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 > mGroupStack; @@ -1300,9 +1298,7 @@ CSSParserImpl::CSSParserImpl() mInFailingSupportsRule(false), mSuppressErrors(false), mDidUnprefixWebkitBoxInEarlierDecl(false), -#ifdef DEBUG mSheetPrincipalRequired(true), -#endif mNextFree(nullptr) { } @@ -7611,9 +7607,13 @@ bool CSSParserImpl::SetValueToURL(nsCSSValue& aValue, const nsString& aURL) { if (!mSheetPrincipal) { - NS_ASSERTION(!mSheetPrincipalRequired, - "Codepaths that expect to parse URLs MUST pass in an " - "origin principal"); + 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 autoRestore(mSheetPrincipalRequired); mSheetPrincipalRequired = false; -#endif nsAutoSuppressErrors suppressErrors(this); diff --git a/layout/style/test/mochitest.ini b/layout/style/test/mochitest.ini index c3f994e13218..37faac038bd2 100644 --- a/layout/style/test/mochitest.ini +++ b/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] diff --git a/layout/style/test/test_bug1112014.html b/layout/style/test/test_bug1112014.html new file mode 100644 index 000000000000..ba1d2f017cb5 --- /dev/null +++ b/layout/style/test/test_bug1112014.html @@ -0,0 +1,121 @@ + + + + + + Test for Bug 1112014 + + + + + +Mozilla Bug 1112014 +

+ +
+
+ +