Bug 1358586: Require comma after <angle>, in -webkit-linear-gradient() CSS syntax. r?heycam draft
authorDaniel Holbert <dholbert@cs.stanford.edu>
Fri, 21 Apr 2017 14:38:14 -0700
changeset 566603 1d50daed28835587c2db2d9b9d34906e3e95a930
parent 566565 0ea6a7c6f2a511e14b74656d68cab77cd35f46f9
child 625376 6d4936deea379fd236f21a3775c0d2483c7a7214
push id55282
push userdholbert@mozilla.com
push dateFri, 21 Apr 2017 21:38:26 +0000
reviewersheycam
bugs1358586
milestone55.0a1
Bug 1358586: Require comma after <angle>, in -webkit-linear-gradient() CSS syntax. r?heycam The logic around here is a bit tricky, because -moz-linear-gradient() shares this codepath. Here's how things were going wrong: * The -moz-linear-gradient syntax allows an angle (e.g. "30deg") to be followed by an optional box-position (e.g. "top right") before eventually requiring a comma and then some color stops. * In contrast, the -webkit-linear-gradient syntax does NOT allow a box-position there (unless the box-position is by itself, without an angle). * So, for -webkit-linear-gradient, we (correctly) skipped the box-position parsing code, if we've seen an angle already. * BUT: in skipping that code, we *also* inadvertantly skipped the code that enforces that we eventually see a comma before we get to the color stops. So, we incorrectly accept some -webkit-linear-gradient() expressions that are missing a comma that really should be mandatory. So, this patch adds a special case for -webkit-linear-gradient that preemptively bails if we get an angle that's not followed by a comma. (With that, we can also simplify the condition around the box position parsing, too.) MozReview-Commit-ID: B7MQLxqe6D7
layout/style/nsCSSParser.cpp
layout/style/test/property_database.js
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -10230,21 +10230,30 @@ CSSParserImpl::ParseLinearGradient(nsCSS
       VARIANT_ANGLE;
 
     bool haveAngle =
       ParseSingleTokenVariant(cssGradient->mAngle, angleFlags, nullptr);
 
     // If we got an angle, we might now have a comma, ending the gradient-line.
     bool haveAngleComma = haveAngle && ExpectSymbol(',', true);
 
-    // If we're webkit-prefixed & didn't get an angle,
-    // OR if we're moz-prefixed & didn't get an angle+comma,
-    // then proceed to parse a box-position.
-    if (((aFlags & eGradient_WebkitLegacy) && !haveAngle) ||
-        ((aFlags & eGradient_MozLegacy) && !haveAngleComma)) {
+    // And in fact, if we're -webkit-linear-gradient and got an angle, there
+    // *must* be a comma after the angle. (Though -moz-linear-gradient is more
+    // permissive because it will allow box-position before the comma.)
+    if (aFlags & eGradient_WebkitLegacy && haveAngle && !haveAngleComma) {
+      SkipUntil(')');
+      return false;
+    }
+
+    // If we're prefixed & didn't get an angle + comma, then proceed to parse a
+    // box-position. (Note that due to the above webkit-specific early-return,
+    // this will only parse an *initial* box-position inside of
+    // -webkit-linear-gradient, vs. an initial OR post-angle box-position
+    // inside of -moz-linear-gradient.)
+    if (((aFlags & eGradient_AnyLegacy) && !haveAngleComma)) {
       // (Note: 3rd arg controls whether the "center" keyword is allowed.
       // -moz-linear-gradient allows it; -webkit-linear-gradient does not.)
       if (!ParseBoxPositionValues(cssGradient->mBgPos, false,
                                   (aFlags & eGradient_MozLegacy))) {
         SkipUntil(')');
         return false;
       }
 
--- a/layout/style/test/property_database.js
+++ b/layout/style/test/property_database.js
@@ -387,16 +387,23 @@ var invalidGradientAndElementValues = [
   /* no quirks mode lengths */
   "-moz-linear-gradient(10 10px -45deg, red, blue) repeat",
   "-moz-linear-gradient(10px 10 -45deg, red, blue) repeat",
   "linear-gradient(red -99, yellow, green, blue 120%)",
   /* Unitless 0 is invalid as an <angle> */
   "-moz-linear-gradient(top left 0, red, blue)",
   "-moz-linear-gradient(5px 5px 0, red, blue)",
   "linear-gradient(0, red, blue)",
+  /* There must be a comma between gradient-line (e.g. <angle>) and colors */
+  "-moz-linear-gradient(30deg red, blue)",
+  "-moz-linear-gradient(5px 5px 30deg red, blue)",
+  "-moz-linear-gradient(5px 5px red, blue)",
+  "-moz-linear-gradient(top left 30deg red, blue)",
+  "linear-gradient(to top left red, blue)",
+  "linear-gradient(to right red, blue)",
   /* Invalid color, calc() or -moz-image-rect() function */
   "linear-gradient(red, rgb(0, rubbish, 0) 50%, red)",
   "linear-gradient(red, red calc(50% + rubbish), red)",
   "linear-gradient(to top calc(50% + rubbish), red, blue)",
   /* Old syntax */
   "-moz-linear-gradient(10px 10px, 20px, 30px 30px, 40px, from(blue), to(red))",
   "-moz-radial-gradient(20px 20px, 10px 10px, from(green), to(#ff00ff))",
   "-moz-radial-gradient(10px 10px, 20%, 40px 40px, 10px, from(green), to(#ff00ff))",
@@ -702,22 +709,22 @@ if (IsCSSPropertyPrefEnabled("layout.css
 
     // 2011 GRADIENTS: -webkit-linear-gradient(), -webkit-radial -gradient()
     // ---------------------------------------------------------------------
     // Basic linear-gradient syntax (valid when prefixed or unprefixed):
     "-webkit-linear-gradient(red, green, blue)",
 
     // Angled linear-gradients (valid when prefixed or unprefixed):
     "-webkit-linear-gradient(135deg, red, blue)",
+    "-webkit-linear-gradient( 135deg  , red  , blue )",
     "-webkit-linear-gradient(280deg, red 60%, blue)",
 
     // Linear-gradient with unitless-0 <angle> (normally invalid for <angle>
     // but accepted here for better webkit emulation):
     "-webkit-linear-gradient(0, red, blue)",
-    "-webkit-linear-gradient(0 red, blue)",
 
     // Basic radial-gradient syntax (valid when prefixed or unprefixed):
     "-webkit-radial-gradient(circle, white, black)",
     "-webkit-radial-gradient(circle, white, black)",
     "-webkit-radial-gradient(ellipse closest-side, white, black)",
     "-webkit-radial-gradient(circle farthest-corner, white, black)",
 
     // Contain/cover keywords (valid only for -moz/-webkit prefixed):
@@ -853,16 +860,22 @@ if (IsCSSPropertyPrefEnabled("layout.css
     // ---------------------------------------------------------------------
     // Syntax that's invalid for all types of gradients:
     // * empty gradient expressions:
     "-webkit-linear-gradient()",
     "-webkit-radial-gradient()",
     "-webkit-repeating-linear-gradient()",
     "-webkit-repeating-radial-gradient()",
 
+    // * missing comma between <legacy-gradient-line> and color list:
+    "-webkit-linear-gradient(0 red, blue)",
+    "-webkit-linear-gradient(30deg red, blue)",
+    "-webkit-linear-gradient(top right red, blue)",
+    "-webkit-linear-gradient(bottom red, blue)",
+
     // Linear syntax that's invalid for both -webkit & unprefixed, but valid
     // for -moz:
     // * initial <legacy-gradient-line> which includes a length:
     "-webkit-linear-gradient(10px, red, blue)",
     "-webkit-linear-gradient(10px top, red, blue)",
     // * initial <legacy-gradient-line> which includes a side *and* an angle:
     "-webkit-linear-gradient(bottom 30deg, red, blue)",
     "-webkit-linear-gradient(30deg bottom, red, blue)",