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.

6 files changed, 33 insertions, 35 deletions

diff --git a/layout/inspector/inDOMUtils.cpp b/layout/inspector/inDOMUtils.cpp
index 58afc5f760..a28b35ef8f 100644
--- a/layout/inspector/inDOMUtils.cpp
+++ b/layout/inspector/inDOMUtils.cpp
@@ -842,7 +842,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:
@@ -905,7 +905,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 cdc84d34cb..89a8be5ec0 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 66e22551e2..3873e9d35b 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 aabbac07a7..34d457c087 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 ea0db5b304..a0f65c069c 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 b4904f0912..1dbd57ba92 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": {