stylo: Error reporting for unknown media features

components/script/dom/cssmediarule.rs

@@ -72,13 +72,15 @@ impl CSSMediaRule {
         let mut input = ParserInput::new(&text);
         let mut input = Parser::new(&mut input);
         let global = self.global();
-        let win = global.as_window();
-        let url = win.get_url();
-        let quirks_mode = win.Document().quirks_mode();
+        let window = global.as_window();
+        let url = window.get_url();
+        let quirks_mode = window.Document().quirks_mode();
         let context = ParserContext::new_for_cssom(&url, Some(CssRuleType::Media),
                                                    PARSING_MODE_DEFAULT,
                                                    quirks_mode);
-        let new_medialist = parse_media_query_list(&context, &mut input);
+
+        let new_medialist = parse_media_query_list(&context, &mut input,
+                                                   window.css_error_reporter());
         let mut guard = self.cssconditionrule.shared_lock().write();
 
         // Clone an Arc because we can’t borrow `guard` twice at the same time.

components/script/dom/htmllinkelement.rs

@@ -289,7 +289,9 @@ impl HTMLLinkElement {
         let context = CssParserContext::new_for_cssom(&doc_url, Some(CssRuleType::Media),
                                                       PARSING_MODE_DEFAULT,
                                                       document.quirks_mode());
-        let media = parse_media_query_list(&context, &mut css_parser);
+        let window = document.window();
+        let media = parse_media_query_list(&context, &mut css_parser,
+                                           window.css_error_reporter());
 
         let im_attribute = element.get_attribute(&ns!(), &local_name!("integrity"));
         let integrity_val = im_attribute.r().map(|a| a.value());

components/script/dom/htmlstyleelement.rs

@@ -74,7 +74,7 @@ impl HTMLStyleElement {
         let element = self.upcast::<Element>();
         assert!(node.is_in_doc());
 
-        let win = window_from_node(node);
+        let window = window_from_node(node);
         let doc = document_from_node(self);
 
         let mq_attribute = element.get_attribute(&ns!(), &local_name!("media"));
@@ -84,19 +84,22 @@ impl HTMLStyleElement {
         };
 
         let data = node.GetTextContent().expect("Element.textContent must be a string");
-        let url = win.get_url();
+        let url = window.get_url();
         let context = CssParserContext::new_for_cssom(&url,
                                                       Some(CssRuleType::Media),
                                                       PARSING_MODE_DEFAULT,
                                                       doc.quirks_mode());
         let shared_lock = node.owner_doc().style_shared_lock().clone();
         let mut input = ParserInput::new(&mq_str);
-        let mq = Arc::new(shared_lock.wrap(
-                    parse_media_query_list(&context, &mut CssParser::new(&mut input))));
+        let css_error_reporter = window.css_error_reporter();
+        let mq = Arc::new(shared_lock.wrap(parse_media_query_list(&context,
+                                                                  &mut CssParser::new(&mut input),
+                                                                  css_error_reporter)));
         let loader = StylesheetLoader::for_element(self.upcast());
-        let sheet = Stylesheet::from_str(&data, win.get_url(), Origin::Author, mq,
+        let sheet = Stylesheet::from_str(&data, window.get_url(),
+                                         Origin::Author, mq,
                                          shared_lock, Some(&loader),
-                                         win.css_error_reporter(),
+                                         css_error_reporter,
                                          doc.quirks_mode(),
                                          self.line_number as u32);
 

components/script/dom/medialist.rs

@@ -74,13 +74,14 @@ impl MediaListMethods for MediaList {
         let mut input = ParserInput::new(&value);
         let mut parser = Parser::new(&mut input);
         let global = self.global();
-        let win = global.as_window();
-        let url = win.get_url();
-        let quirks_mode = win.Document().quirks_mode();
+        let window = global.as_window();
+        let url = window.get_url();
+        let quirks_mode = window.Document().quirks_mode();
         let context = ParserContext::new_for_cssom(&url, Some(CssRuleType::Media),
                                                    PARSING_MODE_DEFAULT,
                                                    quirks_mode);
-        *media_queries = parse_media_query_list(&context, &mut parser);
+        *media_queries = parse_media_query_list(&context, &mut parser,
+                                                window.css_error_reporter());
     }
 
     // https://drafts.csswg.org/cssom/#dom-medialist-length

components/script/dom/window.rs

@@ -1012,7 +1012,8 @@ impl WindowMethods for Window {
         let context = CssParserContext::new_for_cssom(&url, Some(CssRuleType::Media),
                                                       PARSING_MODE_DEFAULT,
                                                       quirks_mode);
-        let media_query_list = media_queries::parse_media_query_list(&context, &mut parser);
+        let media_query_list = media_queries::parse_media_query_list(&context, &mut parser,
+                                                                     self.css_error_reporter());
         let document = self.Document();
         let mql = MediaQueryList::new(&document, media_query_list);
         self.media_query_lists.push(&*mql);

components/style/error_reporting.rs

@@ -19,7 +19,7 @@ pub enum ContextualParseError<'a> {
     UnsupportedPropertyDeclaration(&'a str, ParseError<'a>),
     /// A font face descriptor was not recognized.
     UnsupportedFontFaceDescriptor(&'a str, ParseError<'a>),
-    /// A font feature values descroptor was not recognized.
+    /// A font feature values descriptor was not recognized.
     UnsupportedFontFeatureValuesDescriptor(&'a str, ParseError<'a>),
     /// A keyframe rule was not valid.
     InvalidKeyframeRule(&'a str, ParseError<'a>),
@@ -44,7 +44,9 @@ pub enum ContextualParseError<'a> {
     /// A counter style rule had extends with symbols.
     InvalidCounterStyleExtendsWithSymbols,
     /// A counter style rule had extends with additive-symbols.
-    InvalidCounterStyleExtendsWithAdditiveSymbols
+    InvalidCounterStyleExtendsWithAdditiveSymbols,
+    /// A media rule was invalid for some reason.
+    InvalidMediaRule(&'a str, ParseError<'a>),
 }
 
 impl<'a> fmt::Display for ContextualParseError<'a> {
@@ -168,6 +170,10 @@ impl<'a> fmt::Display for ContextualParseError<'a> {
             ContextualParseError::InvalidCounterStyleExtendsWithAdditiveSymbols => {
                 write!(f, "Invalid @counter-style rule: 'system: extends …' with 'additive-symbols'")
             }
+            ContextualParseError::InvalidMediaRule(media_rule, ref err) => {
+                write!(f, "Invalid media rule: {}, ", media_rule)?;
+                parse_error_to_str(err, f)
+            }
         }
     }
 }

components/style/gecko/media_queries.rs

@@ -23,7 +23,6 @@ use media_queries::MediaType;
 use parser::ParserContext;
 use properties::{ComputedValues, StyleBuilder};
 use properties::longhands::font_size;
-use selectors::parser::SelectorParseError;
 use servo_arc::Arc;
 use std::fmt::{self, Write};
 use std::sync::atomic::{AtomicBool, AtomicIsize, Ordering};
@@ -469,6 +468,89 @@ unsafe fn find_in_table<F>(mut current_entry: *const nsCSSProps_KTableEntry,
     }
 }
 
+fn parse_feature_value<'i, 't>(feature: &nsMediaFeature,
+                               feature_value_type: nsMediaFeature_ValueType,
+                               context: &ParserContext,
+                               input: &mut Parser<'i, 't>)
+                               -> Result<MediaExpressionValue, ParseError<'i>> {
+    let value = match feature_value_type {
+        nsMediaFeature_ValueType::eLength => {
+           let length = Length::parse_non_negative(context, input)?;
+           // FIXME(canaltinova): See bug 1396057. Gecko doesn't support calc
+           // inside media queries. This check is for temporarily remove it
+           // for parity with gecko. We should remove this check when we want
+           // to support it.
+           if let Length::Calc(_) = length {
+               return Err(StyleParseError::UnspecifiedError.into())
+           }
+           MediaExpressionValue::Length(length)
+        },
+        nsMediaFeature_ValueType::eInteger => {
+           // FIXME(emilio): We should use `Integer::parse` to handle `calc`
+           // properly in integer expressions. Note that calc is still not
+           // supported in media queries per FIXME above.
+           let i = input.expect_integer()?;
+           if i < 0 {
+               return Err(StyleParseError::UnspecifiedError.into())
+           }
+           MediaExpressionValue::Integer(i as u32)
+        }
+        nsMediaFeature_ValueType::eBoolInteger => {
+           let i = input.expect_integer()?;
+           if i < 0 || i > 1 {
+               return Err(StyleParseError::UnspecifiedError.into())
+           }
+           MediaExpressionValue::BoolInteger(i == 1)
+        }
+        nsMediaFeature_ValueType::eFloat => {
+           MediaExpressionValue::Float(input.expect_number()?)
+        }
+        nsMediaFeature_ValueType::eIntRatio => {
+           let a = input.expect_integer()?;
+           if a <= 0 {
+               return Err(StyleParseError::UnspecifiedError.into())
+           }
+
+           input.expect_delim('/')?;
+
+           let b = input.expect_integer()?;
+           if b <= 0 {
+               return Err(StyleParseError::UnspecifiedError.into())
+           }
+           MediaExpressionValue::IntRatio(a as u32, b as u32)
+        }
+        nsMediaFeature_ValueType::eResolution => {
+           MediaExpressionValue::Resolution(Resolution::parse(input)?)
+        }
+        nsMediaFeature_ValueType::eEnumerated => {
+           let keyword = input.expect_ident()?;
+           let keyword = unsafe {
+               bindings::Gecko_LookupCSSKeyword(keyword.as_bytes().as_ptr(),
+               keyword.len() as u32)
+           };
+
+           let first_table_entry: *const nsCSSProps_KTableEntry = unsafe {
+               *feature.mData.mKeywordTable.as_ref()
+           };
+
+           let value =
+               match unsafe { find_in_table(first_table_entry, |kw, _| kw == keyword) } {
+                   Some((_kw, value)) => {
+                       value
+                   }
+                   None => return Err(StyleParseError::UnspecifiedError.into()),
+               };
+
+           MediaExpressionValue::Enumerated(value)
+        }
+        nsMediaFeature_ValueType::eIdent => {
+           MediaExpressionValue::Ident(input.expect_ident()?.as_ref().to_owned())
+        }
+    };
+
+    Ok(value)
+}
+
 impl Expression {
     /// Trivially construct a new expression.
     fn new(feature: &'static nsMediaFeature,
@@ -488,13 +570,24 @@ impl Expression {
     /// ```
     pub fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
                          -> Result<Self, ParseError<'i>> {
-        input.expect_parenthesis_block()?;
+        input.expect_parenthesis_block().map_err(|err|
+            match err {
+                BasicParseError::UnexpectedToken(t) => StyleParseError::ExpectedIdentifier(t),
+                _ => StyleParseError::UnspecifiedError,
+            }
+        )?;
+
         input.parse_nested_block(|input| {
             // FIXME: remove extra indented block when lifetimes are non-lexical
             let feature;
             let range;
             {
-                let ident = input.expect_ident()?;
+                let ident = input.expect_ident().map_err(|err|
+                    match err {
+                        BasicParseError::UnexpectedToken(t) => StyleParseError::ExpectedIdentifier(t),
+                        _ => StyleParseError::UnspecifiedError,
+                    }
+                )?;
 
                 let mut flags = 0;
                 let result = {
@@ -530,17 +623,19 @@ impl Expression {
                     Ok((f, r)) => {
                         feature = f;
                         range = r;
-                    }
-                    Err(()) => return Err(SelectorParseError::UnexpectedIdent(ident.clone()).into()),
+                    },
+                    Err(()) => {
+                        return Err(StyleParseError::MediaQueryExpectedFeatureName(ident.clone()).into())
+                    },
                 }
 
                 if (feature.mReqFlags & !flags) != 0 {
-                    return Err(SelectorParseError::UnexpectedIdent(ident.clone()).into());
+                    return Err(StyleParseError::MediaQueryExpectedFeatureName(ident.clone()).into());
                 }
 
                 if range != nsMediaExpression_Range::eEqual &&
-                    feature.mRangeType != nsMediaFeature_RangeType::eMinMaxAllowed {
-                    return Err(SelectorParseError::UnexpectedIdent(ident.clone()).into());
+                   feature.mRangeType != nsMediaFeature_RangeType::eMinMaxAllowed {
+                    return Err(StyleParseError::MediaQueryExpectedFeatureName(ident.clone()).into());
                 }
             }
 
@@ -556,80 +651,11 @@ impl Expression {
                 return Ok(Expression::new(feature, None, range));
             }
 
-            let value = match feature.mValueType {
-                nsMediaFeature_ValueType::eLength => {
-                    let length = Length::parse_non_negative(context, input)?;
-                    // FIXME(canaltinova): See bug 1396057. Gecko doesn't support calc
-                    // inside media queries. This check is for temporarily remove it
-                    // for parity with gecko. We should remove this check when we want
-                    // to support it.
-                    if let Length::Calc(_) = length {
-                        return Err(StyleParseError::UnspecifiedError.into())
-                    }
-                    MediaExpressionValue::Length(length)
-                },
-                nsMediaFeature_ValueType::eInteger => {
-                    // FIXME(emilio): We should use `Integer::parse` to handle `calc`
-                    // properly in integer expressions. Note that calc is still not
-                    // supported in media queries per FIXME above.
-                    let i = input.expect_integer()?;
-                    if i < 0 {
-                        return Err(StyleParseError::UnspecifiedError.into())
-                    }
-                    MediaExpressionValue::Integer(i as u32)
-                }
-                nsMediaFeature_ValueType::eBoolInteger => {
-                    let i = input.expect_integer()?;
-                    if i < 0 || i > 1 {
-                        return Err(StyleParseError::UnspecifiedError.into())
-                    }
-                    MediaExpressionValue::BoolInteger(i == 1)
-                }
-                nsMediaFeature_ValueType::eFloat => {
-                    MediaExpressionValue::Float(input.expect_number()?)
-                }
-                nsMediaFeature_ValueType::eIntRatio => {
-                    let a = input.expect_integer()?;
-                    if a <= 0 {
-                        return Err(StyleParseError::UnspecifiedError.into())
-                    }
-
-                    input.expect_delim('/')?;
-
-                    let b = input.expect_integer()?;
-                    if b <= 0 {
-                        return Err(StyleParseError::UnspecifiedError.into())
-                    }
-                    MediaExpressionValue::IntRatio(a as u32, b as u32)
-                }
-                nsMediaFeature_ValueType::eResolution => {
-                    MediaExpressionValue::Resolution(Resolution::parse(input)?)
-                }
-                nsMediaFeature_ValueType::eEnumerated => {
-                    let keyword = input.expect_ident()?;
-                    let keyword = unsafe {
-                        bindings::Gecko_LookupCSSKeyword(keyword.as_bytes().as_ptr(),
-                                                         keyword.len() as u32)
-                    };
-
-                    let first_table_entry: *const nsCSSProps_KTableEntry = unsafe {
-                        *feature.mData.mKeywordTable.as_ref()
-                    };
-
-                    let value =
-                        match unsafe { find_in_table(first_table_entry, |kw, _| kw == keyword) } {
-                            Some((_kw, value)) => {
-                                value
-                            }
-                            None => return Err(StyleParseError::UnspecifiedError.into()),
-                        };
-
-                    MediaExpressionValue::Enumerated(value)
-                }
-                nsMediaFeature_ValueType::eIdent => {
-                    MediaExpressionValue::Ident(input.expect_ident()?.as_ref().to_owned())
-                }
-            };
+            let value = parse_feature_value(feature,
+                                            feature.mValueType,
+                                            context, input).map_err(|_|
+                StyleParseError::MediaQueryExpectedFeatureValue
+            )?;
 
             Ok(Expression::new(feature, Some(value), range))
         })