Bug 1520020 - Accept empty argument for some filters r=emilio
authorviolet <violet.bugreport@gmail.com>
Mon, 20 May 2019 07:01:29 +0000
changeset 536359 17d1c1e26e31551fd7baf9b3ea80f8f2382385a0
parent 536358 0a796fa7c16ff7147cd5c570b2cae5afa670edcc
child 536360 97dae745c1b3ef2292127ba1c4e90b1345c8f576
child 537300 f87855b9274cb496c33b37f9c5034a4abb42e95a
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1520020
milestone68.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 1520020 - Accept empty argument for some filters r=emilio Filters blur(), invert(), etc. can omit argument. Computed/specified style serialization is a little tricky w.r.t the shortest serialization principle. Ideally we should serialize `invert(1)` to `invert()`, but that will be a breaking change, so we always serialize them with an argument. Note, Blink/WetKit treat specified (but not computed) style serialization differently when the specified one is originally without argument. Our current behavior is the same as pre-Chromium Edge. Differential Revision: https://phabricator.services.mozilla.com/D31720
layout/style/test/property_database.js
servo/components/style/values/specified/effects.rs
testing/web-platform/meta/css/filter-effects/filters-test-brightness-003.html.ini
testing/web-platform/meta/css/filter-effects/parsing/filter-computed.html.ini
testing/web-platform/meta/css/filter-effects/parsing/filter-parsing-valid.html.ini
testing/web-platform/tests/css/filter-effects/parsing/filter-computed.html
testing/web-platform/tests/css/filter-effects/parsing/filter-parsing-valid.html
--- a/layout/style/test/property_database.js
+++ b/layout/style/test/property_database.js
@@ -5194,35 +5194,38 @@ var gCSSProperties = {
       // Mixed SVG reference filters and filter functions
       "grayscale(1) url(#my-filter-1)",
       "url(#my-filter-1) brightness(50%) contrast(0.9)",
 
       // Bad URLs
       "url('badscheme:badurl')",
       "blur(3px) url('badscheme:badurl') grayscale(50%)",
 
+      "blur()",
       "blur(0)",
       "blur(0px)",
       "blur(0.5px)",
       "blur(3px)",
       "blur(100px)",
       "blur(0.1em)",
       "blur(calc(-1px))", // Parses and becomes blur(0px).
       "blur(calc(0px))",
       "blur(calc(5px))",
       "blur(calc(2 * 5px))",
 
+      "brightness()",
       "brightness(0)",
       "brightness(50%)",
       "brightness(1)",
       "brightness(1.0)",
       "brightness(2)",
       "brightness(350%)",
       "brightness(4.567)",
 
+      "contrast()",
       "contrast(0)",
       "contrast(50%)",
       "contrast(1)",
       "contrast(1.0)",
       "contrast(2)",
       "contrast(350%)",
       "contrast(4.567)",
 
@@ -5239,59 +5242,65 @@ var gCSSProperties = {
       "drop-shadow(2px calc(2px + 0.2em))",
       "drop-shadow(blue 2px calc(2px + 0.2em))",
       "drop-shadow(2px calc(2px + 0.2em) blue)",
       "drop-shadow(calc(-2px) calc(-2px))",
       "drop-shadow(-2px -2px)",
       "drop-shadow(calc(2px) calc(2px))",
       "drop-shadow(calc(2px) calc(2px) calc(2px))",
 
+      "grayscale()",
       "grayscale(0)",
       "grayscale(50%)",
       "grayscale(1)",
       "grayscale(1.0)",
       "grayscale(2)",
       "grayscale(350%)",
       "grayscale(4.567)",
 
+      "hue-rotate()",
       "hue-rotate(0)",
       "hue-rotate(0deg)",
       "hue-rotate(90deg)",
       "hue-rotate(540deg)",
       "hue-rotate(-90deg)",
       "hue-rotate(10grad)",
       "hue-rotate(1.6rad)",
       "hue-rotate(-1.6rad)",
       "hue-rotate(0.5turn)",
       "hue-rotate(-2turn)",
 
+      "invert()",
       "invert(0)",
       "invert(50%)",
       "invert(1)",
       "invert(1.0)",
       "invert(2)",
       "invert(350%)",
       "invert(4.567)",
 
+      "opacity()",
       "opacity(0)",
       "opacity(50%)",
       "opacity(1)",
       "opacity(1.0)",
       "opacity(2)",
       "opacity(350%)",
       "opacity(4.567)",
 
+      "saturate()",
       "saturate(0)",
       "saturate(50%)",
       "saturate(1)",
       "saturate(1.0)",
       "saturate(2)",
       "saturate(350%)",
       "saturate(4.567)",
 
+      "sepia()",
       "sepia(0)",
       "sepia(50%)",
       "sepia(1)",
       "sepia(1.0)",
       "sepia(2)",
       "sepia(350%)",
       "sepia(4.567)",
     ],
@@ -5309,38 +5318,35 @@ var gCSSProperties = {
       "url(#my-filter),",
       "invert(50%), url(#my-filter), brightness(90%)",
 
       // Test the following situations for each filter function:
       // - Invalid number of arguments
       // - Comma delimited arguments
       // - Wrong argument type
       // - Argument value out of range
-      "blur()",
       "blur(3px 5px)",
       "blur(3px,)",
       "blur(3px, 5px)",
       "blur(#my-filter)",
       "blur(0.5)",
       "blur(50%)",
       "blur(calc(0))", // Unitless zero in calc is not a valid length.
       "blur(calc(0.1))",
       "blur(calc(10%))",
       "blur(calc(20px - 5%))",
       "blur(-3px)",
 
-      "brightness()",
       "brightness(0.5 0.5)",
       "brightness(0.5,)",
       "brightness(0.5, 0.5)",
       "brightness(#my-filter)",
       "brightness(10px)",
       "brightness(-1)",
 
-      "contrast()",
       "contrast(0.5 0.5)",
       "contrast(0.5,)",
       "contrast(0.5, 0.5)",
       "contrast(#my-filter)",
       "contrast(10px)",
       "contrast(-1)",
 
       "drop-shadow()",
@@ -5356,58 +5362,52 @@ var gCSSProperties = {
       "drop-shadow(2px 2px 2)",
       "drop-shadow(2px 2px 2px 2)",
       "drop-shadow(calc(2px) calc(2px) calc(2px) calc(2px))",
       "drop-shadow(green 2px 2px, blue 1px 3px 4px)",
       "drop-shadow(blue 2px 2px, currentColor 1px 2px)",
       "drop-shadow(unset, 2px 2px)",
       "drop-shadow(2px 2px, unset)",
 
-      "grayscale()",
       "grayscale(0.5 0.5)",
       "grayscale(0.5,)",
       "grayscale(0.5, 0.5)",
       "grayscale(#my-filter)",
       "grayscale(10px)",
       "grayscale(-1)",
 
-      "hue-rotate()",
       "hue-rotate(0.5 0.5)",
       "hue-rotate(0.5,)",
       "hue-rotate(0.5, 0.5)",
       "hue-rotate(#my-filter)",
       "hue-rotate(10px)",
       "hue-rotate(-1)",
       "hue-rotate(45deg,)",
 
-      "invert()",
       "invert(0.5 0.5)",
       "invert(0.5,)",
       "invert(0.5, 0.5)",
       "invert(#my-filter)",
       "invert(10px)",
       "invert(-1)",
 
-      "opacity()",
       "opacity(0.5 0.5)",
       "opacity(0.5,)",
       "opacity(0.5, 0.5)",
       "opacity(#my-filter)",
       "opacity(10px)",
       "opacity(-1)",
 
-      "saturate()",
       "saturate(0.5 0.5)",
       "saturate(0.5,)",
       "saturate(0.5, 0.5)",
       "saturate(#my-filter)",
       "saturate(10px)",
       "saturate(-1)",
 
-      "sepia()",
       "sepia(0.5 0.5)",
       "sepia(0.5,)",
       "sepia(0.5, 0.5)",
       "sepia(#my-filter)",
       "sepia(10px)",
       "sepia(-1)",
     ]
   },
--- a/servo/components/style/values/specified/effects.rs
+++ b/servo/components/style/values/specified/effects.rs
@@ -12,17 +12,17 @@ use crate::values::computed::{Context, T
 use crate::values::generics::effects::BoxShadow as GenericBoxShadow;
 use crate::values::generics::effects::Filter as GenericFilter;
 use crate::values::generics::effects::SimpleShadow as GenericSimpleShadow;
 use crate::values::generics::NonNegative;
 use crate::values::specified::color::Color;
 use crate::values::specified::length::{Length, NonNegativeLength};
 #[cfg(feature = "gecko")]
 use crate::values::specified::url::SpecifiedUrl;
-use crate::values::specified::{Angle, NumberOrPercentage};
+use crate::values::specified::{Angle, Number, NumberOrPercentage};
 #[cfg(not(feature = "gecko"))]
 use crate::values::Impossible;
 use crate::Zero;
 use cssparser::{self, BasicParseErrorKind, Parser, Token};
 use style_traits::{ParseError, StyleParseErrorKind, ValueParseErrorKind};
 
 /// A specified value for a single shadow of the `box-shadow` property.
 pub type BoxShadow =
@@ -57,16 +57,20 @@ impl Factor {
             NumberOrPercentage::Percentage(percent) => {
                 Factor(NumberOrPercentage::Percentage(percent.clamp_to_hundred()))
             },
             NumberOrPercentage::Number(number) => {
                 Factor(NumberOrPercentage::Number(number.clamp_to_one()))
             },
         }
     }
+
+    fn one() -> Self {
+        Factor(NumberOrPercentage::Number(Number::new(1.0)))
+    }
 }
 
 impl Parse for Factor {
     #[inline]
     fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
@@ -204,44 +208,71 @@ impl Parse for Filter {
             Err(cssparser::BasicParseError {
                 kind: BasicParseErrorKind::UnexpectedToken(t),
                 location,
             }) => return Err(location.new_custom_error(ValueParseErrorKind::InvalidFilter(t))),
             Err(e) => return Err(e.into()),
         };
         input.parse_nested_block(|i| {
             match_ignore_ascii_case! { &*function,
-                "blur" => Ok(GenericFilter::Blur((Length::parse_non_negative(context, i)?).into())),
-                "brightness" => Ok(GenericFilter::Brightness(Factor::parse(context, i)?)),
-                "contrast" => Ok(GenericFilter::Contrast(Factor::parse(context, i)?)),
+                "blur" => Ok(GenericFilter::Blur(
+                    i.try(|i| NonNegativeLength::parse(context, i))
+                     .unwrap_or(Zero::zero()),
+                )),
+                "brightness" => Ok(GenericFilter::Brightness(
+                    i.try(|i| Factor::parse(context, i))
+                     .unwrap_or(Factor::one()),
+                )),
+                "contrast" => Ok(GenericFilter::Contrast(
+                    i.try(|i| Factor::parse(context, i))
+                     .unwrap_or(Factor::one()),
+                )),
                 "grayscale" => {
                     // Values of amount over 100% are allowed but UAs must clamp the values to 1.
                     // https://drafts.fxtf.org/filter-effects/#funcdef-filter-grayscale
-                    Ok(GenericFilter::Grayscale(Factor::parse_with_clamping_to_one(context, i)?))
+                    Ok(GenericFilter::Grayscale(
+                        i.try(|i| Factor::parse_with_clamping_to_one(context, i))
+                         .unwrap_or(Factor::one()),
+                    ))
                 },
                 "hue-rotate" => {
                     // We allow unitless zero here, see:
                     // https://github.com/w3c/fxtf-drafts/issues/228
-                    Ok(GenericFilter::HueRotate(Angle::parse_with_unitless(context, i)?))
+                    Ok(GenericFilter::HueRotate(
+                        i.try(|i| Angle::parse_with_unitless(context, i))
+                         .unwrap_or(Zero::zero()),
+                    ))
                 },
                 "invert" => {
                     // Values of amount over 100% are allowed but UAs must clamp the values to 1.
                     // https://drafts.fxtf.org/filter-effects/#funcdef-filter-invert
-                    Ok(GenericFilter::Invert(Factor::parse_with_clamping_to_one(context, i)?))
+                    Ok(GenericFilter::Invert(
+                        i.try(|i| Factor::parse_with_clamping_to_one(context, i))
+                         .unwrap_or(Factor::one()),
+                    ))
                 },
                 "opacity" => {
                     // Values of amount over 100% are allowed but UAs must clamp the values to 1.
                     // https://drafts.fxtf.org/filter-effects/#funcdef-filter-opacity
-                    Ok(GenericFilter::Opacity(Factor::parse_with_clamping_to_one(context, i)?))
+                    Ok(GenericFilter::Opacity(
+                        i.try(|i| Factor::parse_with_clamping_to_one(context, i))
+                         .unwrap_or(Factor::one()),
+                    ))
                 },
-                "saturate" => Ok(GenericFilter::Saturate(Factor::parse(context, i)?)),
+                "saturate" => Ok(GenericFilter::Saturate(
+                    i.try(|i| Factor::parse(context, i))
+                     .unwrap_or(Factor::one()),
+                )),
                 "sepia" => {
                     // Values of amount over 100% are allowed but UAs must clamp the values to 1.
                     // https://drafts.fxtf.org/filter-effects/#funcdef-filter-sepia
-                    Ok(GenericFilter::Sepia(Factor::parse_with_clamping_to_one(context, i)?))
+                    Ok(GenericFilter::Sepia(
+                        i.try(|i| Factor::parse_with_clamping_to_one(context, i))
+                         .unwrap_or(Factor::one()),
+                    ))
                 },
                 "drop-shadow" => Ok(GenericFilter::DropShadow(Parse::parse(context, i)?)),
                 _ => Err(location.new_custom_error(
                     ValueParseErrorKind::InvalidFilter(Token::Function(function.clone()))
                 )),
             }
         })
     }
deleted file mode 100644
--- a/testing/web-platform/meta/css/filter-effects/filters-test-brightness-003.html.ini
+++ /dev/null
@@ -1,2 +0,0 @@
-[filters-test-brightness-003.html]
-  expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/css/filter-effects/parsing/filter-computed.html.ini
+++ /dev/null
@@ -1,28 +0,0 @@
-[filter-computed.html]
-  [Property filter value 'brightness()' computes to 'brightness(0)']
-    expected: FAIL
-
-  [Property filter value 'sepia()' computes to 'sepia(1)']
-    expected: FAIL
-
-  [Property filter value 'saturate()' computes to 'saturate(1)']
-    expected: FAIL
-
-  [Property filter value 'grayscale()' computes to 'grayscale(1)']
-    expected: FAIL
-
-  [Property filter value 'invert()' computes to 'invert(0)']
-    expected: FAIL
-
-  [Property filter value 'hue-rotate()' computes to 'hue-rotate(0deg)']
-    expected: FAIL
-
-  [Property filter value 'blur()' computes to 'blur(0px)']
-    expected: FAIL
-
-  [Property filter value 'opacity()' computes to 'opacity(1)']
-    expected: FAIL
-
-  [Property filter value 'contrast()' computes to 'contrast(1)']
-    expected: FAIL
-
deleted file mode 100644
--- a/testing/web-platform/meta/css/filter-effects/parsing/filter-parsing-valid.html.ini
+++ /dev/null
@@ -1,31 +0,0 @@
-[filter-parsing-valid.html]
-  [Serialization should round-trip after setting e.style['filter'\] = "drop-shadow(1px 2px 3px rgba(4, 5, 6, 0.75))"]
-    expected: FAIL
-
-  [e.style['filter'\] = "saturate()" should set the property value]
-    expected: FAIL
-
-  [e.style['filter'\] = "grayscale()" should set the property value]
-    expected: FAIL
-
-  [e.style['filter'\] = "sepia()" should set the property value]
-    expected: FAIL
-
-  [e.style['filter'\] = "blur()" should set the property value]
-    expected: FAIL
-
-  [e.style['filter'\] = "contrast()" should set the property value]
-    expected: FAIL
-
-  [e.style['filter'\] = "invert()" should set the property value]
-    expected: FAIL
-
-  [e.style['filter'\] = "brightness()" should set the property value]
-    expected: FAIL
-
-  [e.style['filter'\] = "opacity()" should set the property value]
-    expected: FAIL
-
-  [e.style['filter'\] = "hue-rotate()" should set the property value]
-    expected: FAIL
-
--- a/testing/web-platform/tests/css/filter-effects/parsing/filter-computed.html
+++ b/testing/web-platform/tests/css/filter-effects/parsing/filter-computed.html
@@ -20,34 +20,34 @@
 <script>
 test_computed_value("filter", "none");
 
 test_computed_value("filter", "blur(100px)");
 test_computed_value("filter", "blur()", "blur(0px)");
 
 test_computed_value("filter", "brightness(0)");
 test_computed_value("filter", "brightness(300%)", "brightness(3)");
-test_computed_value("filter", "brightness()", "brightness(0)");
+test_computed_value("filter", "brightness()", "brightness(1)");
 
 test_computed_value("filter", "contrast(0)");
 test_computed_value("filter", "contrast(300%)", "contrast(3)");
 test_computed_value("filter", "contrast()", "contrast(1)");
 
 test_computed_value("filter", "drop-shadow(1px 2px)", "drop-shadow(rgb(0, 255, 0) 1px 2px 0px)");
 test_computed_value("filter", "drop-shadow(rgb(4, 5, 6) 1px 2px 0px)");
 
 test_computed_value("filter", "grayscale(50%)", "grayscale(0.5)");
 test_computed_value("filter", "grayscale()", "grayscale(1)");
 
 test_computed_value("filter", "hue-rotate(90deg)");
 test_computed_value("filter", "hue-rotate()", "hue-rotate(0deg)");
 
 test_computed_value("filter", "invert(0)");
 test_computed_value("filter", "invert(100%)", "invert(1)");
-test_computed_value("filter", "invert()", "invert(0)");
+test_computed_value("filter", "invert()", "invert(1)");
 
 test_computed_value("filter", "opacity(0)");
 test_computed_value("filter", "opacity(100%)", "opacity(1)");
 test_computed_value("filter", "opacity()", "opacity(1)");
 
 test_computed_value("filter", "saturate(0)");
 test_computed_value("filter", "saturate(300%)", "saturate(3)");
 test_computed_value("filter", "saturate()", "saturate(1)");
--- a/testing/web-platform/tests/css/filter-effects/parsing/filter-parsing-valid.html
+++ b/testing/web-platform/tests/css/filter-effects/parsing/filter-parsing-valid.html
@@ -11,57 +11,57 @@
 <script src="/css/support/parsing-testcommon.js"></script>
 </head>
 <body>
 <script>
 test_valid_value("filter", "none");
 
 test_valid_value("filter", "blur(100px)");
 test_valid_value("filter", "blur(0)", "blur(0px)");
-test_valid_value("filter", "blur()");
+test_valid_value("filter", "blur()", ["blur()", "blur(0px)"]);
 
 test_valid_value("filter", "brightness(0)");
 test_valid_value("filter", "brightness(300%)");
-test_valid_value("filter", "brightness()");
+test_valid_value("filter", "brightness()", ["brightness()", "brightness(1)"]);
 
 test_valid_value("filter", "contrast(0)");
 test_valid_value("filter", "contrast(300%)");
-test_valid_value("filter", "contrast()");
+test_valid_value("filter", "contrast()", ["contrast()", "contrast(1)"]);
 
 test_valid_value("filter", "drop-shadow(1px 2px)");
 test_valid_value("filter", "drop-shadow(1px 2px 3px)");
 test_valid_value("filter", "drop-shadow(0 0 0)", "drop-shadow(0px 0px 0px)");
 // https://github.com/w3c/fxtf-drafts/issues/240
 test_valid_value("filter", "drop-shadow(rgb(4, 5, 6) 1px 2px)");
 test_valid_value("filter", "drop-shadow(1px 2px rgb(4, 5, 6))", "drop-shadow(rgb(4, 5, 6) 1px 2px)");
 test_valid_value("filter", "drop-shadow(rgba(4, 5, 6, 0.75) 1px 2px 3px)");
 
 test_valid_value("filter", "grayscale(0)");
 test_valid_value("filter", "grayscale(300%)", "grayscale(100%)");
-test_valid_value("filter", "grayscale()");
+test_valid_value("filter", "grayscale()", ["grayscale()", "grayscale(1)"]);
 
 test_valid_value("filter", "hue-rotate(90deg)");
 test_valid_value("filter", "hue-rotate(0)", "hue-rotate(0deg)"); // https://github.com/w3c/fxtf-drafts/issues/228
-test_valid_value("filter", "hue-rotate()");
+test_valid_value("filter", "hue-rotate()", ["hue-rotate()", "hue-rotate(0deg)"]);
 
 test_valid_value("filter", "invert(0)");
 test_valid_value("filter", "invert(300%)", "invert(100%)");
-test_valid_value("filter", "invert()");
+test_valid_value("filter", "invert()", ["invert()", "invert(1)"]);
 
 test_valid_value("filter", "opacity(0)");
 test_valid_value("filter", "opacity(300%)", "opacity(100%)");
-test_valid_value("filter", "opacity()");
+test_valid_value("filter", "opacity()", ["opacity()", "opacity(1)"]);
 
 test_valid_value("filter", "saturate(0)");
 test_valid_value("filter", "saturate(300%)");
-test_valid_value("filter", "saturate()");
+test_valid_value("filter", "saturate()", ["saturate()", "saturate(1)"]);
 
 test_valid_value("filter", "sepia(0)");
 test_valid_value("filter", "sepia(300%)", "sepia(100%)");
-test_valid_value("filter", "sepia()");
+test_valid_value("filter", "sepia()", ["sepia()", "sepia(1)"]);
 
 // Edge serializes url(...) without quotes. Blink/WebKit and Firefox use quotes.
 test_valid_value("filter", "url(picture.svg#f)", ['url("picture.svg#f")', 'url(picture.svg#f)']);
 
 test_valid_value("filter", 'url("https://www.example.com/picture.svg#f")',
   ['url("https://www.example.com/picture.svg#f")', 'url(https://www.example.com/picture.svg#f)']);
 
 test_valid_value("filter", 'blur(10px) url("picture.svg#f") contrast(20) brightness(30)',