Bug 1357117 part 3: Serialize -webkit-linear-gradient() expressions using the (less-expressive) -webkit-linear-gradient syntax, instead of -moz-linear-gradient. r=heycam draft
authorDaniel Holbert <dholbert@cs.stanford.edu>
Tue, 25 Apr 2017 11:48:41 -0700
changeset 568074 b15218d9cd97d3371e89365fc472d0bcd2672861
parent 568073 cabd4a140d575794a8a656c47692bb9d3a02de88
child 625814 7a6d755e461215f4f79f2f6eec0aa748e196d97b
push id55750
push userdholbert@mozilla.com
push dateTue, 25 Apr 2017 18:48:48 +0000
reviewersheycam
bugs1357117
milestone55.0a1
Bug 1357117 part 3: Serialize -webkit-linear-gradient() expressions using the (less-expressive) -webkit-linear-gradient syntax, instead of -moz-linear-gradient. r=heycam Specifically: * This patch uses a flag added in a prior patch to let us use the author's chosen prefix (-webkit or -moz) when serializing. (We treat the -moz version as a special case, because that makes it more straightforward to unsupport -moz if/when we can.) * This patch makes us share the linear-gradient() side-or-corner serialization codepath when serializing points for -webkit-linear-gradient. (The alternative is the -moz-linear-gradient codepath, which defaults to serializing with percent values 0%/100% for sides & corners -- and raw percentages are invalid in -webkit-linear-gradient(), so we can't share that codepath.) Notably, we have to skip the "to " token that the linear-gradient() codepath would normally print out -- that was a late addition to the spec and so it only exists in the unprefixed modern syntax. (Instead, -webkit-linear-gradient syntax is implicitly "from" the given point). MozReview-Commit-ID: 9Oqo8nG1XDU
layout/style/nsCSSValue.cpp
layout/style/nsComputedDOMStyle.cpp
layout/style/test/test_computed_style.html
layout/style/test/test_specified_value_serialization.html
--- a/layout/style/nsCSSValue.cpp
+++ b/layout/style/nsCSSValue.cpp
@@ -1772,17 +1772,21 @@ nsCSSValue::AppendToString(nsCSSProperty
   }
   else if (eCSSUnit_Percent < unit) {  // length unit
     aResult.AppendFloat(GetFloatValue());
   }
   else if (eCSSUnit_Gradient == unit) {
     nsCSSValueGradient* gradient = GetGradientValue();
 
     if (gradient->mIsLegacySyntax) {
-      aResult.AppendLiteral("-moz-");
+      if (gradient->mIsMozLegacySyntax) {
+        aResult.AppendLiteral("-moz-");
+      } else {
+        aResult.AppendLiteral("-webkit-");
+      }
     }
     if (gradient->mIsRepeating) {
       aResult.AppendLiteral("repeating-");
     }
     if (gradient->mIsRadial) {
       aResult.AppendLiteral("radial-gradient(");
     } else {
       aResult.AppendLiteral("linear-gradient(");
@@ -1824,26 +1828,27 @@ nsCSSValue::AppendToString(nsCSSProperty
         if (gradient->GetRadiusY().GetUnit() != eCSSUnit_None) {
           aResult.Append(' ');
           gradient->GetRadiusY().AppendToString(aProperty, aResult,
                                                 aSerialization);
         }
         needSep = true;
       }
     }
-    if (!gradient->mIsRadial && !gradient->mIsLegacySyntax) {
+    if (!gradient->mIsRadial &&
+        !(gradient->mIsLegacySyntax && gradient->mIsMozLegacySyntax)) {
       if (gradient->mBgPos.mXValue.GetUnit() != eCSSUnit_None ||
           gradient->mBgPos.mYValue.GetUnit() != eCSSUnit_None) {
         MOZ_ASSERT(gradient->mAngle.GetUnit() == eCSSUnit_None);
         MOZ_ASSERT(gradient->mBgPos.mXValue.GetUnit() == eCSSUnit_Enumerated &&
                    gradient->mBgPos.mYValue.GetUnit() == eCSSUnit_Enumerated,
                    "unexpected unit");
-        // XXXdholbert Note: this AppendLiteral call will become conditional
-        // in a later patch in this series.
-        aResult.AppendLiteral("to ");
+        if (!gradient->mIsLegacySyntax) {
+          aResult.AppendLiteral("to ");
+        }
         bool didAppendX = false;
         if (!(gradient->mBgPos.mXValue.GetIntValue() & NS_STYLE_IMAGELAYER_POSITION_CENTER)) {
           gradient->mBgPos.mXValue.AppendToString(eCSSProperty_background_position_x,
                                                   aResult, aSerialization);
           didAppendX = true;
         }
         if (!(gradient->mBgPos.mYValue.GetIntValue() & NS_STYLE_IMAGELAYER_POSITION_CENTER)) {
           if (didAppendX) {
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -2014,28 +2014,37 @@ AppendCSSGradientLength(const nsStyleCoo
   aString.Append(tokenString);
 }
 
 static void
 AppendCSSGradientToBoxPosition(const nsStyleGradient* aGradient,
                                nsAString&             aString,
                                bool&                  aNeedSep)
 {
+  // This function only supports box position keywords. Make sure we're not
+  // calling it with inputs that would have coordinates that aren't
+  // representable with box-position keywords.
+  MOZ_ASSERT(aGradient->mShape == NS_STYLE_GRADIENT_SHAPE_LINEAR &&
+             !(aGradient->mLegacySyntax && aGradient->mMozLegacySyntax),
+             "Only call me for linear-gradient and -webkit-linear-gradient");
+
   float xValue = aGradient->mBgPosX.GetPercentValue();
   float yValue = aGradient->mBgPosY.GetPercentValue();
 
   if (yValue == 1.0f && xValue == 0.5f) {
     // omit "to bottom"
     return;
   }
   NS_ASSERTION(yValue != 0.5f || xValue != 0.5f, "invalid box position");
 
-  // XXXdholbert Note: this AppendLiteral call will become conditional in a
-  // later patch in this series.
-  aString.AppendLiteral("to ");
+  if (!aGradient->mLegacySyntax) {
+    // Modern syntax explicitly includes the word "to". Old syntax does not
+    // (and is implicitly "from" the given position instead).
+    aString.AppendLiteral("to ");
+  }
 
   if (yValue == 0.0f) {
     aString.AppendLiteral("top");
   } else if (yValue == 1.0f) {
     aString.AppendLiteral("bottom");
   } else if (yValue != 0.5f) { // do not write "center" keyword
     NS_NOTREACHED("invalid box position");
   }
@@ -2059,17 +2068,21 @@ AppendCSSGradientToBoxPosition(const nsS
 
 void
 nsComputedDOMStyle::GetCSSGradientString(const nsStyleGradient* aGradient,
                                          nsAString& aString)
 {
   if (!aGradient->mLegacySyntax) {
     aString.Truncate();
   } else {
-    aString.AssignLiteral("-moz-");
+    if (aGradient->mMozLegacySyntax) {
+      aString.AssignLiteral("-moz-");
+    } else {
+      aString.AssignLiteral("-webkit-");
+    }
   }
   if (aGradient->mRepeating) {
     aString.AppendLiteral("repeating-");
   }
   bool isRadial = aGradient->mShape != NS_STYLE_GRADIENT_SHAPE_LINEAR;
   if (isRadial) {
     aString.AppendLiteral("radial-gradient(");
   } else {
@@ -2102,17 +2115,19 @@ nsComputedDOMStyle::GetCSSGradientString
         aString.Append(' ');
         AppendCSSGradientLength(aGradient->mRadiusY, tmpVal, aString);
       }
       needSep = true;
     }
   }
   if (aGradient->mBgPosX.GetUnit() != eStyleUnit_None) {
     MOZ_ASSERT(aGradient->mBgPosY.GetUnit() != eStyleUnit_None);
-    if (!isRadial && !aGradient->mLegacySyntax) {
+    if (!isRadial &&
+        !(aGradient->mLegacySyntax && aGradient->mMozLegacySyntax)) {
+      // linear-gradient() or -webkit-linear-gradient()
       AppendCSSGradientToBoxPosition(aGradient, aString, needSep);
     } else if (aGradient->mBgPosX.GetUnit() != eStyleUnit_Percent ||
                aGradient->mBgPosX.GetPercentValue() != 0.5f ||
                aGradient->mBgPosY.GetUnit() != eStyleUnit_Percent ||
                aGradient->mBgPosY.GetPercentValue() != (isRadial ? 0.5f : 1.0f)) {
       if (isRadial && !aGradient->mLegacySyntax) {
         if (needSep) {
           aString.Append(' ');
--- a/layout/style/test/test_computed_style.html
+++ b/layout/style/test/test_computed_style.html
@@ -409,12 +409,58 @@ var noframe_container = document.getElem
     var test = css_color_4[i];
     p.style.color = test[0];
     is(cs.color, test[1], "css-color-4 computed value of " + test[0]);
   }
 
   p.remove();
 })();
 
+(function test_bug_1357117() {
+  // Test that vendor-prefixed gradient styles round-trip with the same prefix,
+  // or with no prefix.
+  var backgroundImages = [
+    // [ specified style,
+    //   expected computed style,
+    //   descriptionOfTestcase ],
+    // Linear gradient with legacy-gradient-line (needs prefixed syntax):
+    [ "-moz-linear-gradient(10deg, red, blue)",
+      "-moz-linear-gradient(10deg, rgb(255, 0, 0), rgb(0, 0, 255))",
+      "-moz-linear-gradient with angled legacy-gradient-line" ],
+    [ "-webkit-linear-gradient(10deg, red, blue)",
+      "-webkit-linear-gradient(10deg, rgb(255, 0, 0), rgb(0, 0, 255))",
+      "-webkit-linear-gradient with angled legacy-gradient-line" ],
+
+    // Linear gradient with box corner (needs prefixed syntax):
+    [ "-moz-linear-gradient(top left, red, blue)",
+      "-moz-linear-gradient(0% 0%, rgb(255, 0, 0), rgb(0, 0, 255))",
+      "-moz-linear-gradient with box corner" ],
+    [ "-webkit-linear-gradient(top left, red, blue)",
+      "-webkit-linear-gradient(top left, rgb(255, 0, 0), rgb(0, 0, 255))",
+      "-webkit-linear-gradient with box corner" ],
+
+    // Radial gradients (should be serialized using modern unprefixed style):
+    [ "-moz-radial-gradient(contain, red, blue)",
+      "radial-gradient(closest-side, rgb(255, 0, 0), rgb(0, 0, 255))",
+      "-moz-radial-gradient with legacy 'contain' keyword" ],
+    [ "-webkit-radial-gradient(contain, red, blue)",
+      "radial-gradient(closest-side, rgb(255, 0, 0), rgb(0, 0, 255))",
+      "-webkit-radial-gradient with legacy 'contain' keyword" ],
+  ];
+
+  var p = document.createElement("p");
+  var cs = getComputedStyle(p, "");
+  frame_container.appendChild(p);
+
+  for (var i = 0; i < backgroundImages.length; ++i) {
+    var test = backgroundImages[i];
+    p.style.backgroundImage = test[0];
+    is(cs.backgroundImage, test[1],
+       "computed value of prefixed gradient expression (" + test[2] + ")");
+  }
+
+  p.remove();
+})();
+
 </script>
 </pre>
 </body>
 </html>
--- a/layout/style/test/test_specified_value_serialization.html
+++ b/layout/style/test/test_specified_value_serialization.html
@@ -101,12 +101,62 @@
     var test = css_color_4[i];
     p.style.color = test[0];
     is(p.style.color, test[1], "css-color-4 serialization value of " + test[0]);
   }
 
   p.remove();
 })();
 
+(function test_bug_1357117() {
+  // Test that vendor-prefixed gradient styles round-trip with the same prefix,
+  // or with no prefix.
+  var backgroundImages = [
+    // [ specified style,
+    //   expected serialization,
+    //   descriptionOfTestcase ],
+    // Linear gradient with legacy-gradient-line (needs prefixed syntax):
+    [ "-moz-linear-gradient(10deg, red, blue)",
+      "-moz-linear-gradient(10deg, red, blue)",
+      "-moz-linear-gradient with angled legacy-gradient-line" ],
+    [ "-webkit-linear-gradient(10deg, red, blue)",
+      "-webkit-linear-gradient(10deg, red, blue)",
+      "-webkit-linear-gradient with angled legacy-gradient-line" ],
+
+    // Linear gradient with box corner (needs prefixed syntax):
+    [ "-moz-linear-gradient(top left, red, blue)",
+      "-moz-linear-gradient(left top , red, blue)",
+      //                            ^
+      // (NOTE: our -moz-linear-gradient serialization inserts an extra space
+      // before the first comma in some cases. This is ugly but fine,
+      // particularly given bug 1337655).
+      "-moz-linear-gradient with box corner" ],
+    [ "-webkit-linear-gradient(top left, red, blue)",
+      "-webkit-linear-gradient(left top, red, blue)",
+      "-webkit-linear-gradient with box corner" ],
+
+    // Radial gradients (should be serialized using modern unprefixed style):
+    [ "-moz-radial-gradient(contain, red, blue)",
+      "radial-gradient(closest-side, red, blue)",
+      "-moz-radial-gradient with legacy 'contain' keyword" ],
+    [ "-webkit-radial-gradient(contain, red, blue)",
+      "radial-gradient(closest-side, red, blue)",
+      "-webkit-radial-gradient with legacy 'contain' keyword" ],
+  ];
+
+  var frame_container = document.getElementById("display");
+  var p = document.createElement("p");
+  frame_container.appendChild(p);
+
+  for (var i = 0; i < backgroundImages.length; ++i) {
+    var test = backgroundImages[i];
+    p.style.backgroundImage = test[0];
+    is(p.style.backgroundImage, test[1],
+       "serialization value of prefixed gradient expression (" + test[2] + ")");
+  }
+
+  p.remove();
+})();
+
 </script>
 </pre>
 </body>
 </html>