Auto merge of #18394 - jdm:filtererr, r=xidorn

Report more specific error for invalid filter properties

Reviewed by xidorn in https://bugzilla.mozilla.org/show_bug.cgi?id=1386900.

---
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix [bug 1386900](https://bugzilla.mozilla.org/show_bug.cgi?id=1386900).
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/18394)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-09-06 12:18:41 -05:00 committed by GitHub
commit a7ea6741d3
3 changed files with 21 additions and 5 deletions

View file

@ -4,9 +4,9 @@
//! Specified types for CSS values related to effects. //! Specified types for CSS values related to effects.
use cssparser::Parser; use cssparser::{Parser, Token, BasicParseError};
use parser::{Parse, ParserContext}; use parser::{Parse, ParserContext};
use style_traits::{ParseError, StyleParseError}; use style_traits::{ParseError, StyleParseError, ValueParseError};
#[cfg(not(feature = "gecko"))] #[cfg(not(feature = "gecko"))]
use values::Impossible; use values::Impossible;
use values::computed::{Context, NonNegativeNumber as ComputedNonNegativeNumber, ToComputedValue}; use values::computed::{Context, NonNegativeNumber as ComputedNonNegativeNumber, ToComputedValue};
@ -161,9 +161,14 @@ impl Parse for Filter {
return Ok(GenericFilter::Url(url)); return Ok(GenericFilter::Url(url));
} }
} }
let function = input.expect_function()?.clone(); let function = match input.expect_function() {
Ok(f) => f.clone(),
Err(BasicParseError::UnexpectedToken(t)) =>
return Err(ValueParseError::InvalidFilter(t.clone()).into()),
Err(e) => return Err(e.into()),
};
input.parse_nested_block(|i| { input.parse_nested_block(|i| {
try_match_ident_ignore_ascii_case! { function, match_ignore_ascii_case! { &*function,
"blur" => Ok(GenericFilter::Blur((Length::parse_non_negative(context, i)?).into())), "blur" => Ok(GenericFilter::Blur((Length::parse_non_negative(context, i)?).into())),
"brightness" => Ok(GenericFilter::Brightness(Factor::parse(context, i)?)), "brightness" => Ok(GenericFilter::Brightness(Factor::parse(context, i)?)),
"contrast" => Ok(GenericFilter::Contrast(Factor::parse(context, i)?)), "contrast" => Ok(GenericFilter::Contrast(Factor::parse(context, i)?)),
@ -174,6 +179,7 @@ impl Parse for Filter {
"saturate" => Ok(GenericFilter::Saturate(Factor::parse(context, i)?)), "saturate" => Ok(GenericFilter::Saturate(Factor::parse(context, i)?)),
"sepia" => Ok(GenericFilter::Sepia(Factor::parse(context, i)?)), "sepia" => Ok(GenericFilter::Sepia(Factor::parse(context, i)?)),
"drop-shadow" => Ok(GenericFilter::DropShadow(Parse::parse(context, i)?)), "drop-shadow" => Ok(GenericFilter::DropShadow(Parse::parse(context, i)?)),
_ => Err(ValueParseError::InvalidFilter(Token::Function(function.clone())).into()),
} }
}) })
} }

View file

@ -132,6 +132,14 @@ pub enum StyleParseError<'i> {
pub enum ValueParseError<'i> { pub enum ValueParseError<'i> {
/// An invalid token was encountered while parsing a color value. /// An invalid token was encountered while parsing a color value.
InvalidColor(Token<'i>), InvalidColor(Token<'i>),
/// An invalid filter value was encountered.
InvalidFilter(Token<'i>),
}
impl<'a> From<ValueParseError<'a>> for ParseError<'a> {
fn from(this: ValueParseError<'a>) -> Self {
StyleParseError::ValueError(this).into()
}
} }
impl<'i> ValueParseError<'i> { impl<'i> ValueParseError<'i> {

View file

@ -122,7 +122,8 @@ fn extract_error_param<'a>(err: ParseError<'a>) -> Option<ErrorString<'a>> {
fn extract_value_error_param<'a>(err: ValueParseError<'a>) -> ErrorString<'a> { fn extract_value_error_param<'a>(err: ValueParseError<'a>) -> ErrorString<'a> {
match err { match err {
ValueParseError::InvalidColor(t) => ErrorString::UnexpectedToken(t), ValueParseError::InvalidColor(t) |
ValueParseError::InvalidFilter(t) => ErrorString::UnexpectedToken(t),
} }
} }
@ -221,6 +222,7 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> {
PropertyDeclarationParseError::InvalidValue(_, ref err))))) => { PropertyDeclarationParseError::InvalidValue(_, ref err))))) => {
let prefix = match *err { let prefix = match *err {
Some(ValueParseError::InvalidColor(_)) => Some(&b"PEColorNotColor\0"[..]), Some(ValueParseError::InvalidColor(_)) => Some(&b"PEColorNotColor\0"[..]),
Some(ValueParseError::InvalidFilter(_)) => Some(&b"PEExpectedNoneOrURLOrFilterFunction\0"[..]),
_ => None, _ => None,
}; };
return (prefix, b"PEValueParsingError\0", Action::Drop); return (prefix, b"PEValueParsingError\0", Action::Drop);