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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
violet 2019-05-20 07:01:29 +00:00
parent abb7207942
commit 94b46be68b
7 changed files with 61 additions and 91 deletions

View File

@ -5199,6 +5199,7 @@ var gCSSProperties = {
"url('badscheme:badurl')",
"blur(3px) url('badscheme:badurl') grayscale(50%)",
"blur()",
"blur(0)",
"blur(0px)",
"blur(0.5px)",
@ -5210,6 +5211,7 @@ var gCSSProperties = {
"blur(calc(5px))",
"blur(calc(2 * 5px))",
"brightness()",
"brightness(0)",
"brightness(50%)",
"brightness(1)",
@ -5218,6 +5220,7 @@ var gCSSProperties = {
"brightness(350%)",
"brightness(4.567)",
"contrast()",
"contrast(0)",
"contrast(50%)",
"contrast(1)",
@ -5244,6 +5247,7 @@ var gCSSProperties = {
"drop-shadow(calc(2px) calc(2px))",
"drop-shadow(calc(2px) calc(2px) calc(2px))",
"grayscale()",
"grayscale(0)",
"grayscale(50%)",
"grayscale(1)",
@ -5252,6 +5256,7 @@ var gCSSProperties = {
"grayscale(350%)",
"grayscale(4.567)",
"hue-rotate()",
"hue-rotate(0)",
"hue-rotate(0deg)",
"hue-rotate(90deg)",
@ -5263,6 +5268,7 @@ var gCSSProperties = {
"hue-rotate(0.5turn)",
"hue-rotate(-2turn)",
"invert()",
"invert(0)",
"invert(50%)",
"invert(1)",
@ -5271,6 +5277,7 @@ var gCSSProperties = {
"invert(350%)",
"invert(4.567)",
"opacity()",
"opacity(0)",
"opacity(50%)",
"opacity(1)",
@ -5279,6 +5286,7 @@ var gCSSProperties = {
"opacity(350%)",
"opacity(4.567)",
"saturate()",
"saturate(0)",
"saturate(50%)",
"saturate(1)",
@ -5287,6 +5295,7 @@ var gCSSProperties = {
"saturate(350%)",
"saturate(4.567)",
"sepia()",
"sepia(0)",
"sepia(50%)",
"sepia(1)",
@ -5314,7 +5323,6 @@ var gCSSProperties = {
// - Comma delimited arguments
// - Wrong argument type
// - Argument value out of range
"blur()",
"blur(3px 5px)",
"blur(3px,)",
"blur(3px, 5px)",
@ -5327,7 +5335,6 @@ var gCSSProperties = {
"blur(calc(20px - 5%))",
"blur(-3px)",
"brightness()",
"brightness(0.5 0.5)",
"brightness(0.5,)",
"brightness(0.5, 0.5)",
@ -5335,7 +5342,6 @@ var gCSSProperties = {
"brightness(10px)",
"brightness(-1)",
"contrast()",
"contrast(0.5 0.5)",
"contrast(0.5,)",
"contrast(0.5, 0.5)",
@ -5361,7 +5367,6 @@ var gCSSProperties = {
"drop-shadow(unset, 2px 2px)",
"drop-shadow(2px 2px, unset)",
"grayscale()",
"grayscale(0.5 0.5)",
"grayscale(0.5,)",
"grayscale(0.5, 0.5)",
@ -5369,7 +5374,6 @@ var gCSSProperties = {
"grayscale(10px)",
"grayscale(-1)",
"hue-rotate()",
"hue-rotate(0.5 0.5)",
"hue-rotate(0.5,)",
"hue-rotate(0.5, 0.5)",
@ -5378,7 +5382,6 @@ var gCSSProperties = {
"hue-rotate(-1)",
"hue-rotate(45deg,)",
"invert()",
"invert(0.5 0.5)",
"invert(0.5,)",
"invert(0.5, 0.5)",
@ -5386,7 +5389,6 @@ var gCSSProperties = {
"invert(10px)",
"invert(-1)",
"opacity()",
"opacity(0.5 0.5)",
"opacity(0.5,)",
"opacity(0.5, 0.5)",
@ -5394,7 +5396,6 @@ var gCSSProperties = {
"opacity(10px)",
"opacity(-1)",
"saturate()",
"saturate(0.5 0.5)",
"saturate(0.5,)",
"saturate(0.5, 0.5)",
@ -5402,7 +5403,6 @@ var gCSSProperties = {
"saturate(10px)",
"saturate(-1)",
"sepia()",
"sepia(0.5 0.5)",
"sepia(0.5,)",
"sepia(0.5, 0.5)",

View File

@ -17,7 +17,7 @@ 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;
@ -62,6 +62,10 @@ impl Factor {
},
}
}
fn one() -> Self {
Factor(NumberOrPercentage::Number(Number::new(1.0)))
}
}
impl Parse for Factor {
@ -209,34 +213,61 @@ impl Parse for Filter {
};
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(

View File

@ -1,2 +0,0 @@
[filters-test-brightness-003.html]
expected: FAIL

View File

@ -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

View File

@ -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

View File

@ -25,7 +25,7 @@ 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)");
@ -42,7 +42,7 @@ 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)");

View File

@ -16,15 +16,15 @@ 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)");
@ -36,27 +36,27 @@ 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)']);