Bug 1295456 - Use author-specified css color function name for css value serialization. r=dholbert
authorJerryShih <hshih@mozilla.com>
Sun, 16 Oct 2016 03:15:36 +0800
changeset 318185 a29d5cb24af486be4c8d66cdc2a024d6badf787b
parent 318184 dfa15b95bb737d1a242537e5aac3e118226f2631
child 318186 4bfcbf898f1161f10da4fd99860ae6402217f917
push id33211
push usercbook@mozilla.com
push dateMon, 17 Oct 2016 09:38:38 +0000
treeherderautoland@e4ef6fa03aa8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1295456
milestone52.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1295456 - Use author-specified css color function name for css value serialization. r=dholbert If the color is fully opaque, omit the 'a' suffix of color function name (e.g. just use 'rgb' or 'hsl'). Otherwise, show the original author-specified function name for serialization. MozReview-Commit-ID: 4XgkvV2jDRC
layout/style/nsCSSParser.cpp
layout/style/nsCSSValue.cpp
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -6707,64 +6707,70 @@ CSSParserImpl::ParseColor(nsCSSValue& aV
           int32_t value;
           if (nsCSSProps::FindKeyword(keyword, nsCSSProps::kColorKTable, value)) {
             aValue.SetIntValue(value, eCSSUnit_EnumColor);
             return CSSParseResult::Ok;
           }
         }
       }
       break;
-    case eCSSToken_Function:
-      if (mToken.mIdent.LowerCaseEqualsLiteral("rgb") ||
+    case eCSSToken_Function: {
+      bool isRGB;
+      bool isHSL;
+      if ((isRGB = mToken.mIdent.LowerCaseEqualsLiteral("rgb")) ||
           mToken.mIdent.LowerCaseEqualsLiteral("rgba")) {
         // rgb() = rgb( <percentage>{3} [ / <alpha-value> ]? ) |
         //         rgb( <number>{3} [ / <alpha-value> ]? ) |
         //         rgb( <percentage>#{3} , <alpha-value>? ) |
         //         rgb( <number>#{3} , <alpha-value>? )
         // <alpha-value> = <number> | <percentage>
         // rgba is an alias of rgb.
 
         if (GetToken(true)) {
           UngetToken();
         }
         if (mToken.mType == eCSSToken_Number) { // <number>
           uint8_t r, g, b, a;
 
           if (ParseRGBColor(r, g, b, a)) {
-            aValue.SetIntegerColorValue(NS_RGBA(r, g, b, a), eCSSUnit_RGBAColor);
+            aValue.SetIntegerColorValue(NS_RGBA(r, g, b, a),
+                isRGB ? eCSSUnit_RGBColor : eCSSUnit_RGBAColor);
             return CSSParseResult::Ok;
           }
         } else {  // <percentage>
           float r, g, b, a;
 
           if (ParseRGBColor(r, g, b, a)) {
-            aValue.SetFloatColorValue(r, g, b, a, eCSSUnit_PercentageRGBAColor);
+            aValue.SetFloatColorValue(r, g, b, a,
+                isRGB ? eCSSUnit_PercentageRGBColor : eCSSUnit_PercentageRGBAColor);
             return CSSParseResult::Ok;
           }
         }
         SkipUntil(')');
         return CSSParseResult::Error;
       }
-      else if (mToken.mIdent.LowerCaseEqualsLiteral("hsl") ||
+      else if ((isHSL = mToken.mIdent.LowerCaseEqualsLiteral("hsl")) ||
                mToken.mIdent.LowerCaseEqualsLiteral("hsla")) {
         // hsl() = hsl( <hue> <percentage> <percentage> [ / <alpha-value> ]? ) ||
         //         hsl( <hue>, <percentage>, <percentage>, <alpha-value>? )
         // <hue> = <number> | <angle>
         // hsla is an alias of hsl.
 
         float h, s, l, a;
 
         if (ParseHSLColor(h, s, l, a)) {
-          aValue.SetFloatColorValue(h, s, l, a, eCSSUnit_HSLAColor);
+          aValue.SetFloatColorValue(h, s, l, a,
+              isHSL ? eCSSUnit_HSLColor : eCSSUnit_HSLAColor);
           return CSSParseResult::Ok;
         }
         SkipUntil(')');
         return CSSParseResult::Error;
       }
       break;
+    }
     default:
       break;
   }
 
   // try 'xxyyzz' without '#' prefix for compatibility with IE and Nav4x (bug 23236 and 45804)
   if (mHashlessColorQuirk) {
     // - If the string starts with 'a-f', the nsCSSScanner builds the
     //   token as a eCSSToken_Ident and we can parse the string as a
--- a/layout/style/nsCSSValue.cpp
+++ b/layout/style/nsCSSValue.cpp
@@ -1553,22 +1553,24 @@ nsCSSValue::AppendToString(nsCSSProperty
         unit == eCSSUnit_RGBAColor) {
       nscolor color = GetColorValue();
       if (aSerialization == eNormalized &&
           color == NS_RGBA(0, 0, 0, 0)) {
         // Use the strictest match for 'transparent' so we do correct
         // round-tripping of all other rgba() values.
         aResult.AppendLiteral("transparent");
       } else {
+        // For brevity, we omit the alpha component if it's equal to 255 (full
+        // opaque). Also, we try to preserve the author-specified function name,
+        // unless it's rgba() and we're omitting the alpha component - then we
+        // use rgb().
         uint8_t a = NS_GET_A(color);
-        bool showAlpha =
-          (aSerialization == eNormalized && a < 255) ||
-          (aSerialization == eAuthorSpecified &&
-           unit == eCSSUnit_RGBAColor);
-        if (showAlpha) {
+        bool showAlpha = (a != 255);
+
+        if (unit == eCSSUnit_RGBAColor && showAlpha) {
           aResult.AppendLiteral("rgba(");
         } else {
           aResult.AppendLiteral("rgb(");
         }
 
         NS_NAMED_LITERAL_STRING(comma, ", ");
 
         aResult.AppendInt(NS_GET_R(color), 10);
@@ -3045,44 +3047,47 @@ bool
 nsCSSValueFloatColor::IsNonTransparentColor() const
 {
   return mAlpha > 0.0f;
 }
 
 void
 nsCSSValueFloatColor::AppendToString(nsCSSUnit aUnit, nsAString& aResult) const
 {
+  // Similar to the rgb()/rgba() case in nsCSSValue::AppendToString. We omit the
+  // alpha component if it's equal to 1.0f (full opaque). Also, we try to
+  // preserve the author-specified function name, unless it's rgba()/hsla() and
+  // we're omitting the alpha component - then we use rgb()/hsl().
   MOZ_ASSERT(nsCSSValue::IsFloatColorUnit(aUnit), "unexpected unit");
 
-  bool hasAlpha = aUnit == eCSSUnit_PercentageRGBAColor ||
-                  aUnit == eCSSUnit_HSLAColor;
-  bool isHSL = aUnit == eCSSUnit_HSLColor ||
-               aUnit == eCSSUnit_HSLAColor;
+  bool showAlpha = (mAlpha != 1.0f);
+  bool isHSL = (aUnit == eCSSUnit_HSLColor ||
+                aUnit == eCSSUnit_HSLAColor);
 
   if (isHSL) {
     aResult.AppendLiteral("hsl");
   } else {
     aResult.AppendLiteral("rgb");
   }
-  if (hasAlpha) {
+  if (showAlpha && (aUnit == eCSSUnit_HSLAColor || aUnit == eCSSUnit_PercentageRGBAColor)) {
     aResult.AppendLiteral("a(");
   } else {
     aResult.Append('(');
   }
   if (isHSL) {
     aResult.AppendFloat(mComponent1 * 360.0f);
     aResult.AppendLiteral(", ");
   } else {
     aResult.AppendFloat(mComponent1 * 100.0f);
     aResult.AppendLiteral("%, ");
   }
   aResult.AppendFloat(mComponent2 * 100.0f);
   aResult.AppendLiteral("%, ");
   aResult.AppendFloat(mComponent3 * 100.0f);
-  if (hasAlpha) {
+  if (showAlpha) {
     aResult.AppendLiteral("%, ");
     aResult.AppendFloat(mAlpha);
     aResult.Append(')');
   } else {
     aResult.AppendLiteral("%)");
   }
 }