From dd6252e34f208eede34125facd067c4601870de4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 8 May 2019 12:44:46 +0000 Subject: [PATCH] style: Don't report errors for properties for which we've parsed another value in the same declaration block. I thought a bit about how to test it and it's not particularly great. test_css_parse_error_smoketest.html is great to assert that something _gets_ reported, but not that it doesn't :) Differential Revision: https://phabricator.services.mozilla.com/D30201 --- .../style/properties/declaration_block.rs | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index 29e6ce1fd24..5ce80955a48 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -1269,7 +1269,7 @@ pub fn parse_one_declaration_into( struct PropertyDeclarationParser<'a, 'b: 'a> { context: &'a ParserContext<'b>, declarations: &'a mut SourcePropertyDeclaration, - /// The last parsed property id if non-custom, and if any. + /// The last parsed property id if any. last_parsed_property_id: Option, } @@ -1300,6 +1300,7 @@ impl<'a, 'b, 'i> DeclarationParser<'i> for PropertyDeclarationParser<'a, 'b> { let id = match PropertyId::parse(&name, self.context) { Ok(id) => id, Err(..) => { + self.last_parsed_property_id = None; return Err(input.new_custom_error(if is_non_mozilla_vendor_identifier(&name) { StyleParseErrorKind::UnknownVendorProperty } else { @@ -1328,7 +1329,7 @@ type SmallParseErrorVec<'i> = SmallVec<[(ParseError<'i>, &'i str, Option( context: &ParserContext, - _block: Option<&PropertyDeclarationBlock>, + block: Option<&PropertyDeclarationBlock>, selectors: Option<&SelectorList>, mut error: ParseError<'i>, slice: &str, @@ -1336,6 +1337,21 @@ fn report_one_css_error<'i>( ) { debug_assert!(context.error_reporting_enabled()); + fn all_properties_in_block(block: &PropertyDeclarationBlock, property: &PropertyId) -> bool { + match *property { + PropertyId::LonghandAlias(id, _) | + PropertyId::Longhand(id) => block.contains(id), + PropertyId::ShorthandAlias(id, _) | + PropertyId::Shorthand(id) => { + id.longhands().all(|longhand| block.contains(longhand)) + }, + // NOTE(emilio): We could do this, but it seems of limited utility, + // and it's linear on the size of the declaration block, so let's + // not. + PropertyId::Custom(..) => false, + } + } + // If the unrecognized property looks like a vendor-specific property, // silently ignore it instead of polluting the error output. if let ParseErrorKind::Custom(StyleParseErrorKind::UnknownVendorProperty) = error.kind { @@ -1343,6 +1359,11 @@ fn report_one_css_error<'i>( } if let Some(ref property) = property { + if let Some(block) = block { + if all_properties_in_block(block, property) { + return; + } + } error = match *property { PropertyId::Custom(ref c) => StyleParseErrorKind::new_invalid(format!("--{}", c), error), _ => StyleParseErrorKind::new_invalid(property.non_custom_id().unwrap().name(), error),