From e719df53c0f89958641728737278b197419dc451 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Fri, 15 Sep 2017 11:38:44 +0800 Subject: [PATCH] Clamp some filter functions to one if the values over 100%. For grayscale, invert, opacity, and sepia, "values of amount over 100% are allowed but UAs must clamp the values to 1" [1]-[4], so we clamp its value in the parser. [1] https://drafts.fxtf.org/filter-effects/#funcdef-filter-grayscale [2] https://drafts.fxtf.org/filter-effects/#funcdef-filter-invert [3] https://drafts.fxtf.org/filter-effects/#funcdef-filter-opacity [4] https://drafts.fxtf.org/filter-effects/#funcdef-filter-sepia --- components/style/values/specified/effects.rs | 48 +++++++++++++++++-- components/style/values/specified/mod.rs | 9 ++++ .../style/values/specified/percentage.rs | 9 ++++ .../html/filter-grayscale-005.htm.ini | 3 -- 4 files changed, 62 insertions(+), 7 deletions(-) delete mode 100644 tests/wpt/metadata-css/filters-1_dev/html/filter-grayscale-005.htm.ini diff --git a/components/style/values/specified/effects.rs b/components/style/values/specified/effects.rs index 7b9ea0c7bb9..7394a27b6ca 100644 --- a/components/style/values/specified/effects.rs +++ b/components/style/values/specified/effects.rs @@ -39,6 +39,30 @@ pub type Filter = GenericFilter; #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub struct Factor(NumberOrPercentage); +impl Factor { + /// Parse this factor but clamp to one if the value is over 100%. + #[inline] + pub fn parse_with_clamping_to_one<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't> + ) -> Result> { + Factor::parse(context, input).map(|v| v.clamp_to_one()) + } + + /// Clamp the value to 1 if the value is over 100%. + #[inline] + fn clamp_to_one(self) -> Self { + match self.0 { + NumberOrPercentage::Percentage(percent) => { + Factor(NumberOrPercentage::Percentage(percent.clamp_to_hundred())) + }, + NumberOrPercentage::Number(number) => { + Factor(NumberOrPercentage::Number(number.clamp_to_one())) + } + } + } +} + impl Parse for Factor { #[inline] fn parse<'i, 't>( @@ -172,12 +196,28 @@ impl Parse for Filter { "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)?)), - "grayscale" => Ok(GenericFilter::Grayscale(Factor::parse(context, i)?)), + "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)?)) + }, "hue-rotate" => Ok(GenericFilter::HueRotate(Angle::parse(context, i)?)), - "invert" => Ok(GenericFilter::Invert(Factor::parse(context, i)?)), - "opacity" => Ok(GenericFilter::Opacity(Factor::parse(context, i)?)), + "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)?)) + }, + "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)?)) + }, "saturate" => Ok(GenericFilter::Saturate(Factor::parse(context, i)?)), - "sepia" => Ok(GenericFilter::Sepia(Factor::parse(context, i)?)), + "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)?)) + }, "drop-shadow" => Ok(GenericFilter::DropShadow(Parse::parse(context, i)?)), _ => Err(ValueParseError::InvalidFilter(Token::Function(function.clone())).into()), } diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 2b08912f73e..92af8e01414 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -216,6 +216,15 @@ impl Number { -> Result> { parse_number_with_clamping_mode(context, input, AllowedNumericType::AtLeastOne) } + + /// Clamp to 1.0 if the value is over 1.0. + #[inline] + pub fn clamp_to_one(self) -> Self { + Number { + value: self.value.min(1.), + calc_clamping_mode: self.calc_clamping_mode, + } + } } impl ToComputedValue for Number { diff --git a/components/style/values/specified/percentage.rs b/components/style/values/specified/percentage.rs index 0373d29016e..f3736b8c860 100644 --- a/components/style/values/specified/percentage.rs +++ b/components/style/values/specified/percentage.rs @@ -124,6 +124,15 @@ impl Percentage { ) -> Result> { Self::parse_with_clamping_mode(context, input, AllowedNumericType::NonNegative) } + + /// Clamp to 100% if the value is over 100%. + #[inline] + pub fn clamp_to_hundred(self) -> Self { + Percentage { + value: self.value.min(1.), + calc_clamping_mode: self.calc_clamping_mode, + } + } } impl Parse for Percentage { diff --git a/tests/wpt/metadata-css/filters-1_dev/html/filter-grayscale-005.htm.ini b/tests/wpt/metadata-css/filters-1_dev/html/filter-grayscale-005.htm.ini deleted file mode 100644 index e6be95b7ac1..00000000000 --- a/tests/wpt/metadata-css/filters-1_dev/html/filter-grayscale-005.htm.ini +++ /dev/null @@ -1,3 +0,0 @@ -[filter-grayscale-005.htm] - type: reftest - expected: FAIL