Skip to content

Commit

Permalink
style: Don't report errors for properties for which we've parsed anot…
Browse files Browse the repository at this point in the history
…her 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
  • Loading branch information
emilio committed May 10, 2019
1 parent 0221026 commit dd6252e
Showing 1 changed file with 23 additions and 2 deletions.
25 changes: 23 additions & 2 deletions components/style/properties/declaration_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PropertyId>,
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1328,21 +1329,41 @@ type SmallParseErrorVec<'i> = SmallVec<[(ParseError<'i>, &'i str, Option<Propert
#[cold]
fn report_one_css_error<'i>(
context: &ParserContext,
_block: Option<&PropertyDeclarationBlock>,
block: Option<&PropertyDeclarationBlock>,
selectors: Option<&SelectorList<SelectorImpl>>,
mut error: ParseError<'i>,
slice: &str,
property: Option<PropertyId>,
) {
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 {
return;
}

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),
Expand Down

0 comments on commit dd6252e

Please sign in to comment.