Auto merge of #17739 - jdm:no-vendor-prefixed-errors, r=emilio

Suppress CSS parser errors for vendor-prefixed properties.

This matches the behaviour of Gecko's CSS parser.

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #17736
- [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/17739)
<!-- Reviewable:end -->

components/layout_thread/lib.rs

@@ -133,7 +133,6 @@ use style::stylist::{ExtraStyleData, Stylist};
 use style::thread_state;
 use style::timer::Timer;
 use style::traversal::{DomTraversal, TraversalDriver, TraversalFlags};
-use style::values::CompactCowStr;
 
 /// Information needed by the layout thread.
 pub struct LayoutThread {
@@ -714,7 +713,7 @@ impl LayoutThread {
             Msg::RegisterPaint(name, mut properties, painter) => {
                 debug!("Registering the painter");
                 let properties = properties.drain(..)
-                    .filter_map(|name| PropertyId::parse(CompactCowStr::from(&*name)).ok().map(|id| (name.clone(), id)))
+                    .filter_map(|name| PropertyId::parse(&*name).ok().map(|id| (name.clone(), id)))
                     .filter(|&(_, ref id)| id.as_shorthand().is_err())
                     .collect();
                 let registered_painter = RegisteredPainter {

components/script/dom/cssstyledeclaration.rs

@@ -296,7 +296,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertyvalue
     fn GetPropertyValue(&self, property: DOMString) -> DOMString {
-        let id = if let Ok(id) = PropertyId::parse(property.into()) {
+        let id = if let Ok(id) = PropertyId::parse(&property) {
             id
         } else {
             // Unkwown property
@@ -307,7 +307,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
 
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertypriority
     fn GetPropertyPriority(&self, property: DOMString) -> DOMString {
-        let id = if let Ok(id) = PropertyId::parse(property.into()) {
+        let id = if let Ok(id) = PropertyId::parse(&property) {
             id
         } else {
             // Unkwown property
@@ -331,7 +331,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
                    priority: DOMString)
                    -> ErrorResult {
         // Step 3
-        let id = if let Ok(id) = PropertyId::parse(property.into()) {
+        let id = if let Ok(id) = PropertyId::parse(&property) {
             id
         } else {
             // Unknown property
@@ -348,7 +348,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         }
 
         // Step 2 & 3
-        let id = match PropertyId::parse(property.into()) {
+        let id = match PropertyId::parse(&property) {
             Ok(id) => id,
             Err(..) => return Ok(()), // Unkwown property
         };
@@ -380,7 +380,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
             return Err(Error::NoModificationAllowed);
         }
 
-        let id = if let Ok(id) = PropertyId::parse(property.into()) {
+        let id = if let Ok(id) = PropertyId::parse(&property) {
             id
         } else {
             // Unkwown property, cannot be there to remove.

components/style/properties/declaration_block.rs

@@ -8,7 +8,7 @@
 
 use context::QuirksMode;
 use cssparser::{DeclarationListParser, parse_important, ParserInput, CompactCowStr};
-use cssparser::{Parser, AtRuleParser, DeclarationParser, Delimiter};
+use cssparser::{Parser, AtRuleParser, DeclarationParser, Delimiter, ParseError as CssParseError};
 use error_reporting::{ParseErrorReporter, ContextualParseError};
 use parser::{ParserContext, log_css_error};
 use properties::animated_properties::AnimationValue;
@@ -935,13 +935,28 @@ impl<'a, 'b, 'i> AtRuleParser<'i> for PropertyDeclarationParser<'a, 'b> {
     type Error = SelectorParseError<'i, StyleParseError<'i>>;
 }
 
+/// Based on NonMozillaVendorIdentifier from Gecko's CSS parser.
+fn is_non_mozilla_vendor_identifier(name: &str) -> bool {
+    (name.starts_with("-") && !name.starts_with("-moz-")) ||
+        name.starts_with("_")
+}
+
 impl<'a, 'b, 'i> DeclarationParser<'i> for PropertyDeclarationParser<'a, 'b> {
     type Declaration = Importance;
     type Error = SelectorParseError<'i, StyleParseError<'i>>;
 
     fn parse_value<'t>(&mut self, name: CompactCowStr<'i>, input: &mut Parser<'i, 't>)
                        -> Result<Importance, ParseError<'i>> {
-        let id = PropertyId::parse(name)?;
+        let id = match PropertyId::parse(&name) {
+            Ok(id) => id,
+            Err(()) => {
+                return Err(if is_non_mozilla_vendor_identifier(&name) {
+                    PropertyDeclarationParseError::UnknownVendorProperty
+                } else {
+                    PropertyDeclarationParseError::UnknownProperty(name)
+                }.into());
+            }
+        };
         input.parse_until_before(Delimiter::Bang, |input| {
             PropertyDeclaration::parse_into(self.declarations, id, self.context, input)
                 .map_err(|e| e.into())
@@ -976,6 +991,15 @@ pub fn parse_property_declaration_list(context: &ParserContext,
             }
             Err(err) => {
                 iter.parser.declarations.clear();
+
+                // If the unrecognized property looks like a vendor-specific property,
+                // silently ignore it instead of polluting the error output.
+                if let CssParseError::Custom(SelectorParseError::Custom(
+                    StyleParseError::PropertyDeclaration(
+                        PropertyDeclarationParseError::UnknownVendorProperty))) = err.error {
+                    continue;
+                }
+
                 let pos = err.span.start;
                 let error = ContextualParseError::UnsupportedPropertyDeclaration(
                     iter.input.slice(err.span), err.error);

components/style/properties/gecko.mako.rs

@@ -2871,7 +2871,7 @@ fn static_assert() {
                         Gecko_AppendWillChange(&mut self.gecko, feature.0.as_ptr());
                     }
 
-                    if let Ok(prop_id) = PropertyId::parse(feature.0.to_string().into()) {
+                    if let Ok(prop_id) = PropertyId::parse(&feature.0.to_string()) {
                         match prop_id.as_shorthand() {
                             Ok(shorthand) => {
                                 for longhand in shorthand.longhands() {

components/style/properties/properties.mako.rs

@@ -20,7 +20,7 @@ use stylearc::{Arc, UniqueArc};
 use app_units::Au;
 #[cfg(feature = "servo")] use cssparser::RGBA;
 use cssparser::{Parser, TokenSerializationType, serialize_identifier};
-use cssparser::{ParserInput, CompactCowStr};
+use cssparser::ParserInput;
 #[cfg(feature = "servo")] use euclid::SideOffsets2D;
 use computed_values;
 use context::QuirksMode;
@@ -510,7 +510,7 @@ impl LonghandId {
                     % if not property.derived_from:
                         longhands::${property.ident}::parse_declared(context, input)
                     % else:
-                        Err(PropertyDeclarationParseError::UnknownProperty.into())
+                        Err(PropertyDeclarationParseError::UnknownProperty("${property.ident}".into()).into())
                     % endif
                 }
             % endfor
@@ -992,8 +992,8 @@ impl PropertyId {
     /// Returns a given property from the string `s`.
     ///
     /// Returns Err(()) for unknown non-custom properties
-    pub fn parse<'i>(property_name: CompactCowStr<'i>) -> Result<Self, ParseError<'i>> {
+    pub fn parse(property_name: &str) -> Result<Self, ()> {
-        if let Ok(name) = ::custom_properties::parse_name(&property_name) {
+        if let Ok(name) = ::custom_properties::parse_name(property_name) {
             return Ok(PropertyId::Custom(::custom_properties::Name::from(name)))
         }
 
@@ -1014,10 +1014,10 @@ impl PropertyId {
                 % endfor
             }
         }
-        match static_id(&property_name) {
+        match static_id(property_name) {
             Some(&StaticId::Longhand(id)) => Ok(PropertyId::Longhand(id)),
             Some(&StaticId::Shorthand(id)) => Ok(PropertyId::Shorthand(id)),
-            None => Err(StyleParseError::UnknownProperty(property_name).into()),
+            None => Err(()),
         }
     }
 
@@ -1101,7 +1101,7 @@ impl PropertyId {
     }
 
     fn check_allowed_in(&self, rule_type: CssRuleType, stylesheet_origin: Origin)
-                        -> Result<(), PropertyDeclarationParseError> {
+                        -> Result<(), PropertyDeclarationParseError<'static>> {
         let id: NonCustomPropertyId;
         match *self {
             // Custom properties are allowed everywhere
@@ -1186,7 +1186,7 @@ impl PropertyId {
                         return Err(PropertyDeclarationParseError::ExperimentalProperty);
                     }
                 } else {
-                    return Err(PropertyDeclarationParseError::UnknownProperty);
+                    return Err(PropertyDeclarationParseError::UnknownProperty(self.name().into()));
                 }
             }
         } else {
@@ -1462,9 +1462,9 @@ impl PropertyDeclaration {
     /// This will not actually parse Importance values, and will always set things
     /// to Importance::Normal. Parsing Importance values is the job of PropertyDeclarationParser,
     /// we only set them here so that we don't have to reallocate
-    pub fn parse_into(declarations: &mut SourcePropertyDeclaration,
+    pub fn parse_into<'i, 't>(declarations: &mut SourcePropertyDeclaration,
-                      id: PropertyId, context: &ParserContext, input: &mut Parser)
+                              id: PropertyId, context: &ParserContext, input: &mut Parser<'i, 't>)
-                      -> Result<(), PropertyDeclarationParseError> {
+                              -> Result<(), PropertyDeclarationParseError<'i>> {
         assert!(declarations.is_empty());
         let rule_type = context.rule_type();
         debug_assert!(rule_type == CssRuleType::Keyframe ||

components/style/stylesheets/keyframes_rule.rs

@@ -17,6 +17,7 @@ use selectors::parser::SelectorParseError;
 use shared_lock::{DeepCloneParams, DeepCloneWithLock, SharedRwLock, SharedRwLockReadGuard, Locked, ToCssWithGuard};
 use std::fmt;
 use style_traits::{PARSING_MODE_DEFAULT, ToCss, ParseError, StyleParseError};
+use style_traits::PropertyDeclarationParseError;
 use stylearc::Arc;
 use stylesheets::{CssRuleType, StylesheetContents};
 use stylesheets::rule_parser::VendorPrefix;
@@ -532,7 +533,8 @@ impl<'a, 'b, 'i> DeclarationParser<'i> for KeyframeDeclarationParser<'a, 'b> {
 
     fn parse_value<'t>(&mut self, name: CompactCowStr<'i>, input: &mut Parser<'i, 't>)
                        -> Result<(), ParseError<'i>> {
-        let id = PropertyId::parse(name.into())?;
+        let id = PropertyId::parse(&name)
+            .map_err(|()| PropertyDeclarationParseError::UnknownProperty(name))?;
         match PropertyDeclaration::parse_into(self.declarations, id, self.context, input) {
             Ok(()) => {
                 // In case there is still unparsed text in the declaration, we should roll back.

components/style_traits/lib.rs

@@ -91,7 +91,7 @@ pub enum StyleParseError<'i> {
     /// Unexpected closing curly bracket in a DVB.
     UnbalancedCloseCurlyBracketInDeclarationValueBlock,
     /// A property declaration parsing error.
-    PropertyDeclaration(PropertyDeclarationParseError),
+    PropertyDeclaration(PropertyDeclarationParseError<'i>),
     /// A property declaration value had input remaining after successfully parsing.
     PropertyDeclarationValueNotExhausted,
     /// An unexpected dimension token was encountered.
@@ -112,15 +112,15 @@ pub enum StyleParseError<'i> {
     UnspecifiedError,
     /// An unexpected token was found within a namespace rule.
     UnexpectedTokenWithinNamespace(Token<'i>),
-    /// An unknown CSS property was encountered.
-    UnknownProperty(CompactCowStr<'i>),
 }
 
 /// The result of parsing a property declaration.
 #[derive(Eq, PartialEq, Clone, Debug)]
-pub enum PropertyDeclarationParseError {
+pub enum PropertyDeclarationParseError<'i> {
     /// The property declaration was for an unknown property.
-    UnknownProperty,
+    UnknownProperty(CompactCowStr<'i>),
+    /// An unknown vendor-specific identifier was encountered.
+    UnknownVendorProperty,
     /// The property declaration was for a disabled experimental property.
     ExperimentalProperty,
     /// The property declaration contained an invalid value.
@@ -140,8 +140,8 @@ impl<'a> From<StyleParseError<'a>> for ParseError<'a> {
     }
 }
 
-impl<'a> From<PropertyDeclarationParseError> for ParseError<'a> {
+impl<'a> From<PropertyDeclarationParseError<'a>> for ParseError<'a> {
-    fn from(this: PropertyDeclarationParseError) -> Self {
+    fn from(this: PropertyDeclarationParseError<'a>) -> Self {
         cssparser::ParseError::Custom(SelectorParseError::Custom(StyleParseError::PropertyDeclaration(this)))
     }
 }

ports/geckolib/error_reporter.rs

@@ -233,7 +233,8 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> {
                 ErrorString::Ident(namespace),
 
             (_, CssParseError::Custom(SelectorParseError::Custom(
-                StyleParseError::UnknownProperty(property)))) =>
+                StyleParseError::PropertyDeclaration(
+                    PropertyDeclarationParseError::UnknownProperty(property))))) =>
                 ErrorString::Ident(property),
 
             (_, CssParseError::Custom(SelectorParseError::Custom(

tests/unit/style/stylesheets.rs

@@ -316,7 +316,7 @@ fn test_report_error_stylesheet() {
 
     let error = errors.pop().unwrap();
     assert_eq!("Unsupported property declaration: 'invalid: true;', \
-                Custom(UnknownProperty(\"invalid\"))", error.message);
+                Custom(PropertyDeclaration(UnknownProperty(\"invalid\")))", error.message);
     assert_eq!(9, error.line);
     assert_eq!(8, error.column);
 
@@ -329,3 +329,30 @@ fn test_report_error_stylesheet() {
     // testing for the url
     assert_eq!(url, error.url);
 }
+
+#[test]
+fn test_no_report_unrecognized_vendor_properties() {
+    let css = r"
+    div {
+        -o-background-color: red;
+        _background-color: red;
+        -moz-background-color: red;
+    }
+    ";
+    let url = ServoUrl::parse("about::test").unwrap();
+    let error_reporter = CSSInvalidErrorReporterTest::new();
+
+    let errors = error_reporter.errors.clone();
+
+    let lock = SharedRwLock::new();
+    let media = Arc::new(lock.wrap(MediaList::empty()));
+    Stylesheet::from_str(css, url, Origin::UserAgent, media, lock,
+                         None, &error_reporter, QuirksMode::NoQuirks, 0u64);
+
+    let mut errors = errors.lock().unwrap();
+    let error = errors.pop().unwrap();
+    assert_eq!("Unsupported property declaration: '-moz-background-color: red;', \
+                Custom(PropertyDeclaration(UnknownProperty(\"-moz-background-color\")))",
+               error.message);
+    assert!(errors.is_empty());
+}