Bug 1538101 - Don't report errors for properties for which we've parsed another value in the same declaration block. r=jdm
☠☠ backed out by 616fc2abbbec ☠ ☠
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 08 May 2019 12:44:46 +0000
changeset 473059 eee17e1b043502d063b1d609bd98750e91c1bc00
parent 473058 ebcb2c853b9ce80debb2a093c3855a16a2f15492
child 473060 0ad4726584ec4bdb1786371815c814b3c7b66559
push id113065
push useropoprus@mozilla.com
push dateThu, 09 May 2019 03:46:59 +0000
treeherdermozilla-inbound@34a824c75b7b [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
@@ -1323,31 +1323,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);