Bug 1538101 - Don't report errors for properties for which we've parsed another value in the same declaration block. r=jdm
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 08 May 2019 12:44:46 +0000
changeset 531903 3429489885f855dc1658504c7d3b91de52446e3f
parent 531902 c49518b4191c5985f6bfb21fab850c3b73313234
child 531904 4e2250bbaed37b7b0a9a2c4b66c0c5455a1551a6
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm
bugs1538101
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1538101 - Don't report errors for properties for which we've parsed another value in the same declaration block. r=jdm 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
servo/components/style/properties/declaration_block.rs
--- a/servo/components/style/properties/declaration_block.rs
+++ b/servo/components/style/properties/declaration_block.rs
@@ -1264,17 +1264,17 @@ pub fn parse_one_declaration_into(
         }
     })
 }
 
 /// A struct to parse property declarations.
 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>,
 }
 
 
 /// Default methods reject all at rules.
 impl<'a, 'b, 'i> AtRuleParser<'i> for PropertyDeclarationParser<'a, 'b> {
     type PreludeNoBlock = ();
     type PreludeBlock = ();
@@ -1295,16 +1295,17 @@ impl<'a, 'b, 'i> DeclarationParser<'i> f
     fn parse_value<'t>(
         &mut self,
         name: CowRcStr<'i>,
         input: &mut Parser<'i, 't>,
     ) -> Result<Importance, ParseError<'i>> {
         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 {
                     StyleParseErrorKind::UnknownProperty(name)
                 }));
             }
         };
         if self.context.error_reporting_enabled() {
@@ -1323,31 +1324,51 @@ impl<'a, 'b, 'i> DeclarationParser<'i> f
     }
 }
 
 type SmallParseErrorVec<'i> = SmallVec<[(ParseError<'i>, &'i str, Option<PropertyId>); 2]>;
 
 #[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),
         };
     }
 
     let location = error.location;
     let error = ContextualParseError::UnsupportedPropertyDeclaration(slice, error, selectors);