From f2866a5420b4645d888fc50b22f7cbdd8a178042 Mon Sep 17 00:00:00 2001 From: athenian200 Date: Thu, 17 Sep 2020 16:01:38 -0500 Subject: Issue #1647 - Part 2: Implement VARIANT_OPACITY to correctly serialize. Even though percentages are already treated as floats internally by the style system for computation purposes, you have to go out of your way to stop them from being read back out as percentages. What I do here amounts to storing the percentage token in the "wrong" container, the one normally used for floats. This allows a value that was read in as a percentage to be read back out as something else, which is normally prevented by the design of the style system. --- layout/inspector/inDOMUtils.cpp | 4 ++-- layout/style/nsCSSParser.cpp | 30 +++++++++++++++++++++--------- layout/style/nsCSSPropList.h | 10 +++++----- layout/style/nsCSSProps.h | 1 + layout/style/nsRuleNode.cpp | 15 --------------- layout/style/test/property_database.js | 8 ++++---- 6 files changed, 33 insertions(+), 35 deletions(-) diff --git a/layout/inspector/inDOMUtils.cpp b/layout/inspector/inDOMUtils.cpp index e212e20df..2095fb775 100644 --- a/layout/inspector/inDOMUtils.cpp +++ b/layout/inspector/inDOMUtils.cpp @@ -846,7 +846,7 @@ PropertySupportsVariant(nsCSSPropertyID aPropertyID, uint32_t aVariant) case eCSSProperty_grid_row_end: case eCSSProperty_font_weight: case eCSSProperty_initial_letter: - supported = VARIANT_NUMBER; + supported = VARIANT_NUMBER | VARIANT_OPACITY; break; default: @@ -909,7 +909,7 @@ inDOMUtils::CssPropertySupportsType(const nsAString& aProperty, uint32_t aType, break; case TYPE_NUMBER: // Include integers under "number"? - variant = VARIANT_NUMBER | VARIANT_INTEGER; + variant = VARIANT_NUMBER | VARIANT_INTEGER | VARIANT_OPACITY; break; default: // Unknown type diff --git a/layout/style/nsCSSParser.cpp b/layout/style/nsCSSParser.cpp index f6b84bfac..1525c5f9a 100644 --- a/layout/style/nsCSSParser.cpp +++ b/layout/style/nsCSSParser.cpp @@ -1300,7 +1300,7 @@ protected: } bool ParseNonNegativeNumber(nsCSSValue& aValue) { - return ParseSingleTokenNonNegativeVariant(aValue, VARIANT_NUMBER, nullptr); + return ParseSingleTokenNonNegativeVariant(aValue, VARIANT_NUMBER | VARIANT_OPACITY, nullptr); } // Helpers for some common ParseSingleTokenOneOrLargerVariant calls. @@ -1310,7 +1310,7 @@ protected: } bool ParseOneOrLargerNumber(nsCSSValue& aValue) { - return ParseSingleTokenOneOrLargerVariant(aValue, VARIANT_NUMBER, nullptr); + return ParseSingleTokenOneOrLargerVariant(aValue, VARIANT_NUMBER | VARIANT_OPACITY, nullptr); } // http://dev.w3.org/csswg/css-values/#custom-idents @@ -7789,6 +7789,7 @@ CSSParserImpl::ParseNonNegativeVariant(nsCSSValue& aValue, VARIANT_NUMBER | VARIANT_LENGTH | VARIANT_PERCENT | + VARIANT_OPACITY | VARIANT_INTEGER)) == 0, "need to update code below to handle additional variants"); @@ -7829,6 +7830,7 @@ CSSParserImpl::ParseOneOrLargerVariant(nsCSSValue& aValue, // that we specifically handle. MOZ_ASSERT((aVariantMask & ~(VARIANT_ALL_NONNUMERIC | VARIANT_NUMBER | + VARIANT_OPACITY | VARIANT_INTEGER)) == 0, "need to update code below to handle additional variants"); @@ -7957,9 +7959,9 @@ CSSParserImpl::ParseVariant(nsCSSValue& aValue, } } } - // Check VARIANT_NUMBER and VARIANT_INTEGER before VARIANT_LENGTH or - // VARIANT_ZERO_ANGLE. - if (((aVariantMask & VARIANT_NUMBER) != 0) && + // Check VARIANT_NUMBER, number tokens for VARIANT_OPACITY, and + // VARIANT_INTEGER before VARIANT_LENGTH or VARIANT_ZERO_ANGLE. + if (((aVariantMask & (VARIANT_NUMBER | VARIANT_OPACITY)) != 0) && (eCSSToken_Number == tk->mType)) { aValue.SetFloatValue(tk->mNumber, eCSSUnit_Number); return CSSParseResult::Ok; @@ -7969,6 +7971,7 @@ CSSParserImpl::ParseVariant(nsCSSValue& aValue, aValue.SetIntValue(tk->mInteger, eCSSUnit_Integer); return CSSParseResult::Ok; } + if (((aVariantMask & (VARIANT_LENGTH | VARIANT_ANGLE | VARIANT_FREQUENCY | VARIANT_TIME)) != 0 && eCSSToken_Dimension == tk->mType) || @@ -7994,6 +7997,15 @@ CSSParserImpl::ParseVariant(nsCSSValue& aValue, aValue.SetPercentValue(tk->mNumber); return CSSParseResult::Ok; } + // We need to store eCSSToken_Percentage in eCSSUnit_Number in order to + // serialize opacity according to spec. All percentage tokens are stored + // as floats, so no type conversion is needed to make this possible. + // Percentage tokens have to be evaluated later than number tokens. + if (((aVariantMask & VARIANT_OPACITY) != 0) && + (eCSSToken_Percentage == tk->mType)) { + aValue.SetFloatValue(tk->mNumber, eCSSUnit_Number); + return CSSParseResult::Ok; + } if (mUnitlessLengthQuirk) { // NONSTANDARD: Nav interprets unitless numbers as px if (((aVariantMask & VARIANT_LENGTH) != 0) && (eCSSToken_Number == tk->mType)) { @@ -8472,7 +8484,7 @@ CSSParserImpl::ParseImageRect(nsCSSValue& aImage) break; } - static const int32_t VARIANT_SIDE = VARIANT_NUMBER | VARIANT_PERCENT; + static const int32_t VARIANT_SIDE = VARIANT_NUMBER | VARIANT_PERCENT | VARIANT_OPACITY; if (!ParseSingleTokenNonNegativeVariant(top, VARIANT_SIDE, nullptr) || !ExpectSymbol(',', true) || !ParseSingleTokenNonNegativeVariant(right, VARIANT_SIDE, nullptr) || @@ -10881,7 +10893,7 @@ CSSParserImpl::ParseWebkitGradientColorStop(nsCSSValueGradient* aGradient) if (mToken.mIdent.LowerCaseEqualsLiteral("color-stop")) { // Parse stop location, followed by comma. if (!ParseSingleTokenVariant(stop->mLocation, - VARIANT_NUMBER | VARIANT_PERCENT, + VARIANT_NUMBER | VARIANT_PERCENT | VARIANT_OPACITY, nullptr) || !ExpectSymbol(',', true)) { SkipUntil(')'); // Skip to end of color-stop(...) expression. @@ -16044,7 +16056,7 @@ static bool GetFunctionParseInformation(nsCSSKeyword aToken, {VARIANT_LBCALC, VARIANT_LBCALC, VARIANT_LBCALC}, {VARIANT_ANGLE_OR_ZERO}, {VARIANT_ANGLE_OR_ZERO, VARIANT_ANGLE_OR_ZERO}, - {VARIANT_NUMBER}, + {VARIANT_NUMBER|VARIANT_OPACITY}, {VARIANT_LENGTH|VARIANT_NONNEGATIVE_DIMENSION}, {VARIANT_LB|VARIANT_NONNEGATIVE_DIMENSION}, {VARIANT_NUMBER, VARIANT_NUMBER}, @@ -17626,7 +17638,7 @@ CSSParserImpl::ParseScrollSnapPoints(nsCSSValue& aValue, nsCSSPropertyID aPropID nsCSSKeywords::LookupKeyword(mToken.mIdent) == eCSSKeyword_repeat) { nsCSSValue lengthValue; if (ParseNonNegativeVariant(lengthValue, - VARIANT_LENGTH | VARIANT_PERCENT | VARIANT_CALC, + VARIANT_LENGTH | VARIANT_PERCENT | VARIANT_OPACITY | VARIANT_CALC, nullptr) != CSSParseResult::Ok) { REPORT_UNEXPECTED(PEExpectedNonnegativeNP); SkipUntil(')'); diff --git a/layout/style/nsCSSPropList.h b/layout/style/nsCSSPropList.h index 906ed7ee0..890019245 100644 --- a/layout/style/nsCSSPropList.h +++ b/layout/style/nsCSSPropList.h @@ -1703,7 +1703,7 @@ CSS_PROP_SVG( FillOpacity, CSS_PROPERTY_PARSE_VALUE, "", - VARIANT_HPN | VARIANT_OPENTYPE_SVG_KEYWORD, + VARIANT_INHERIT | VARIANT_OPACITY | VARIANT_OPENTYPE_SVG_KEYWORD, kContextOpacityKTable, offsetof(nsStyleSVG, mFillOpacity), eStyleAnimType_float) @@ -1841,7 +1841,7 @@ CSS_PROP_SVGRESET( FloodOpacity, CSS_PROPERTY_PARSE_VALUE, "", - VARIANT_HPN, + VARIANT_INHERIT | VARIANT_OPACITY, nullptr, offsetof(nsStyleSVGReset, mFloodOpacity), eStyleAnimType_float) @@ -3070,7 +3070,7 @@ CSS_PROP_EFFECTS( CSS_PROPERTY_CAN_ANIMATE_ON_COMPOSITOR | CSS_PROPERTY_CREATES_STACKING_CONTEXT, "", - VARIANT_HPN, + VARIANT_INHERIT | VARIANT_OPACITY, nullptr, offsetof(nsStyleEffects, mOpacity), eStyleAnimType_float) @@ -3761,7 +3761,7 @@ CSS_PROP_SVGRESET( StopOpacity, CSS_PROPERTY_PARSE_VALUE, "", - VARIANT_HPN, + VARIANT_INHERIT | VARIANT_OPACITY, nullptr, offsetof(nsStyleSVGReset, mStopOpacity), eStyleAnimType_float) @@ -3836,7 +3836,7 @@ CSS_PROP_SVG( StrokeOpacity, CSS_PROPERTY_PARSE_VALUE, "", - VARIANT_HPN | VARIANT_OPENTYPE_SVG_KEYWORD, + VARIANT_INHERIT | VARIANT_OPACITY | VARIANT_OPENTYPE_SVG_KEYWORD, kContextOpacityKTable, offsetof(nsStyleSVG, mStrokeOpacity), eStyleAnimType_float) diff --git a/layout/style/nsCSSProps.h b/layout/style/nsCSSProps.h index aabbac07a..34d457c08 100644 --- a/layout/style/nsCSSProps.h +++ b/layout/style/nsCSSProps.h @@ -43,6 +43,7 @@ #define VARIANT_IDENTIFIER 0x002000 // D #define VARIANT_IDENTIFIER_NO_INHERIT 0x004000 // like above, but excluding // 'inherit' and 'initial' +#define VARIANT_OPACITY 0x008000 // Take floats and percents as input, output float. #define VARIANT_AUTO 0x010000 // A #define VARIANT_INHERIT 0x020000 // H eCSSUnit_Initial, eCSSUnit_Inherit, eCSSUnit_Unset #define VARIANT_NONE 0x040000 // O diff --git a/layout/style/nsRuleNode.cpp b/layout/style/nsRuleNode.cpp index 3863ec292..036d97f86 100644 --- a/layout/style/nsRuleNode.cpp +++ b/layout/style/nsRuleNode.cpp @@ -1579,21 +1579,6 @@ SetFactor(const nsCSSValue& aValue, float& aField, RuleNodeCacheConditions& aCon case eCSSUnit_Null: return; - case eCSSUnit_Percent: - aField = aValue.GetPercentValue(); - if (aFlags & SETFCT_POSITIVE) { - NS_ASSERTION(aField >= 0.0f, "negative value for positive-only property"); - if (aField < 0.0f) - aField = 0.0f; - } - if (aFlags & SETFCT_OPACITY) { - if (aField < 0.0f) - aField = 0.0f; - if (aField > 1.0f) - aField = 1.0f; - } - return; - case eCSSUnit_Number: aField = aValue.GetFloatValue(); if (aFlags & SETFCT_POSITIVE) { diff --git a/layout/style/test/property_database.js b/layout/style/test/property_database.js index 2fc50a539..bc4383630 100644 --- a/layout/style/test/property_database.js +++ b/layout/style/test/property_database.js @@ -3540,7 +3540,7 @@ var gCSSProperties = { inherited: false, type: CSS_TYPE_LONGHAND, initial_values: [ "1", "17", "397.376", "3e1", "3e+1", "3e0", "3e+0", "3e-0" ], - other_values: [ "0", "0.4", "0.0000", "-3", "3e-1" "-100%", "50%"], + other_values: [ "0", "0.4", "0.0000", "-3", "3e-1" "-100%", "50%" ], invalid_values: [ "0px", "1px" ] }, "-moz-orient": { @@ -4273,7 +4273,7 @@ var gCSSProperties = { inherited: true, type: CSS_TYPE_LONGHAND, initial_values: [ "1", "2.8", "1.000", "300%", "context-fill-opacity", "context-stroke-opacity" ], - other_values: [ "0", "0.3", "-7.3", "-100%", "50%"], + other_values: [ "0", "0.3", "-7.3", "-100%", "50%" ], invalid_values: [] }, "fill-rule": { @@ -4305,8 +4305,8 @@ var gCSSProperties = { domProp: "floodOpacity", inherited: false, type: CSS_TYPE_LONGHAND, - initial_values: [ "1", "2.8", "1.000", "300%"], - other_values: [ "0", "0.3", "-7.3", "-100%", "50%"], + initial_values: [ "1", "2.8", "1.000", "300%" ], + other_values: [ "0", "0.3", "-7.3", "-100%", "50%" ], invalid_values: [] }, "image-rendering": { -- cgit v1.2.3