From 51764ac7228e723054bf304a381cc83d9067f10a Mon Sep 17 00:00:00 2001 From: athenian200 Date: Wed, 16 Sep 2020 17:34:03 -0500 Subject: Issue #1647 - Part 1: Implement percentage for CSS opacity keywords This preliminary step allows percentages to be computed and display correctly, but unfortunately it fails a test after changing VARIANT_HN to VARIANT_HPN because that allows values to be serialized as percentages. However, not doing this means percentages are rejected as valid values for the user to input. The way the style system is setup makes it hard to change this for opacity without changing it for everything else, especially since some code-saving speed hacks in Bug 636029 and Bug 441367 that make a lot of assumptions about this stuff very rigid. --- layout/style/nsCSSPropList.h | 10 +++++----- layout/style/nsRuleNode.cpp | 15 +++++++++++++++ layout/style/test/property_database.js | 18 +++++++++--------- 3 files changed, 29 insertions(+), 14 deletions(-) (limited to 'layout/style') diff --git a/layout/style/nsCSSPropList.h b/layout/style/nsCSSPropList.h index 411f982a4..66e22551e 100644 --- a/layout/style/nsCSSPropList.h +++ b/layout/style/nsCSSPropList.h @@ -1690,7 +1690,7 @@ CSS_PROP_SVG( FillOpacity, CSS_PROPERTY_PARSE_VALUE, "", - VARIANT_HN | VARIANT_OPENTYPE_SVG_KEYWORD, + VARIANT_HPN | VARIANT_OPENTYPE_SVG_KEYWORD, kContextOpacityKTable, offsetof(nsStyleSVG, mFillOpacity), eStyleAnimType_float) @@ -1828,7 +1828,7 @@ CSS_PROP_SVGRESET( FloodOpacity, CSS_PROPERTY_PARSE_VALUE, "", - VARIANT_HN, + VARIANT_HPN, nullptr, offsetof(nsStyleSVGReset, mFloodOpacity), eStyleAnimType_float) @@ -3057,7 +3057,7 @@ CSS_PROP_EFFECTS( CSS_PROPERTY_CAN_ANIMATE_ON_COMPOSITOR | CSS_PROPERTY_CREATES_STACKING_CONTEXT, "", - VARIANT_HN, + VARIANT_HPN, nullptr, offsetof(nsStyleEffects, mOpacity), eStyleAnimType_float) @@ -3748,7 +3748,7 @@ CSS_PROP_SVGRESET( StopOpacity, CSS_PROPERTY_PARSE_VALUE, "", - VARIANT_HN, + VARIANT_HPN, nullptr, offsetof(nsStyleSVGReset, mStopOpacity), eStyleAnimType_float) @@ -3823,7 +3823,7 @@ CSS_PROP_SVG( StrokeOpacity, CSS_PROPERTY_PARSE_VALUE, "", - VARIANT_HN | VARIANT_OPENTYPE_SVG_KEYWORD, + VARIANT_HPN | VARIANT_OPENTYPE_SVG_KEYWORD, kContextOpacityKTable, offsetof(nsStyleSVG, mStrokeOpacity), eStyleAnimType_float) diff --git a/layout/style/nsRuleNode.cpp b/layout/style/nsRuleNode.cpp index a0f65c069..ea0db5b30 100644 --- a/layout/style/nsRuleNode.cpp +++ b/layout/style/nsRuleNode.cpp @@ -1577,6 +1577,21 @@ 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 c75f7b498..b4904f091 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" ], + other_values: [ "0", "0.4", "0.0000", "-3", "3e-1" "-100%", "50%"], invalid_values: [ "0px", "1px" ] }, "-moz-orient": { @@ -4272,8 +4272,8 @@ var gCSSProperties = { domProp: "fillOpacity", inherited: true, type: CSS_TYPE_LONGHAND, - initial_values: [ "1", "2.8", "1.000", "context-fill-opacity", "context-stroke-opacity" ], - other_values: [ "0", "0.3", "-7.3" ], + initial_values: [ "1", "2.8", "1.000", "300%", "context-fill-opacity", "context-stroke-opacity" ], + 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" ], - other_values: [ "0", "0.3", "-7.3" ], + initial_values: [ "1", "2.8", "1.000", "300%"], + other_values: [ "0", "0.3", "-7.3", "-100%", "50%"], invalid_values: [] }, "image-rendering": { @@ -4380,8 +4380,8 @@ var gCSSProperties = { domProp: "stopOpacity", inherited: false, type: CSS_TYPE_LONGHAND, - initial_values: [ "1", "2.8", "1.000" ], - other_values: [ "0", "0.3", "-7.3" ], + initial_values: [ "1", "2.8", "1.000", "300%" ], + other_values: [ "0", "0.3", "-7.3", "-100%", "50%" ], invalid_values: [] }, "stroke": { @@ -4436,8 +4436,8 @@ var gCSSProperties = { domProp: "strokeOpacity", inherited: true, type: CSS_TYPE_LONGHAND, - initial_values: [ "1", "2.8", "1.000", "context-fill-opacity", "context-stroke-opacity" ], - other_values: [ "0", "0.3", "-7.3" ], + initial_values: [ "1", "2.8", "1.000", "300%", "context-fill-opacity", "context-stroke-opacity" ], + other_values: [ "0", "0.3", "-7.3", "-100%", "50% ], invalid_values: [] }, "stroke-width": { -- cgit v1.2.3 From b5c9f8e24e6ce368532f893fbd9a517b79096839 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/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 ++++---- 5 files changed, 31 insertions(+), 33 deletions(-) (limited to 'layout/style') diff --git a/layout/style/nsCSSParser.cpp b/layout/style/nsCSSParser.cpp index cdc84d34c..89a8be5ec 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 66e22551e..3873e9d35 100644 --- a/layout/style/nsCSSPropList.h +++ b/layout/style/nsCSSPropList.h @@ -1690,7 +1690,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) @@ -1828,7 +1828,7 @@ CSS_PROP_SVGRESET( FloodOpacity, CSS_PROPERTY_PARSE_VALUE, "", - VARIANT_HPN, + VARIANT_INHERIT | VARIANT_OPACITY, nullptr, offsetof(nsStyleSVGReset, mFloodOpacity), eStyleAnimType_float) @@ -3057,7 +3057,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) @@ -3748,7 +3748,7 @@ CSS_PROP_SVGRESET( StopOpacity, CSS_PROPERTY_PARSE_VALUE, "", - VARIANT_HPN, + VARIANT_INHERIT | VARIANT_OPACITY, nullptr, offsetof(nsStyleSVGReset, mStopOpacity), eStyleAnimType_float) @@ -3823,7 +3823,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 ea0db5b30..a0f65c069 100644 --- a/layout/style/nsRuleNode.cpp +++ b/layout/style/nsRuleNode.cpp @@ -1577,21 +1577,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 b4904f091..1dbd57ba9 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