servo: Merge #18341 - stylo: Error reporting for unknown media features (from ferjm:bug1384225.media.errors); r=jdm
authorFernando Jiménez Moreno <ferjmoreno@gmail.com>
Fri, 08 Sep 2017 11:14:51 -0500
changeset 429286 b5aaeb1c1879acc3f3565fa5dfba3692972600e7
parent 429285 24c7ddf09f52daff961ba4b21c2cb49f7bd06626
child 429287 d57e40a922ee924bdf07609fbdb3328cea308bdd
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm
bugs18341, 1384225
milestone57.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
servo: Merge #18341 - stylo: Error reporting for unknown media features (from ferjm:bug1384225.media.errors); r=jdm - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix [bug 1384225](https://bugzilla.mozilla.org/show_bug.cgi?id=1384225) Source-Repo: https://github.com/servo/servo Source-Revision: fef2cfde8cc6688c63cde1966b081a4698a5d977
servo/components/script/dom/cssmediarule.rs
servo/components/script/dom/htmllinkelement.rs
servo/components/script/dom/htmlstyleelement.rs
servo/components/script/dom/medialist.rs
servo/components/script/dom/window.rs
servo/components/style/error_reporting.rs
servo/components/style/gecko/media_queries.rs
servo/components/style/media_queries.rs
servo/components/style/stylesheets/rule_parser.rs
servo/components/style_traits/lib.rs
servo/ports/geckolib/error_reporter.rs
servo/ports/geckolib/glue.rs
--- a/servo/components/script/dom/cssmediarule.rs
+++ b/servo/components/script/dom/cssmediarule.rs
@@ -67,23 +67,25 @@ impl CSSMediaRule {
         list.to_css_string().into()
     }
 
     /// https://drafts.csswg.org/css-conditional-3/#the-cssmediarule-interface
     pub fn set_condition_text(&self, text: DOMString) {
         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.
 
         // FIXME(SimonSapin): allow access to multiple objects with one write guard?
         // Would need a set of usize pointer addresses or something,
         // the same object is not accessed more than once.
         let mqs = Arc::clone(&self.mediarule.write_with(&mut guard).media_queries);
--- a/servo/components/script/dom/htmllinkelement.rs
+++ b/servo/components/script/dom/htmllinkelement.rs
@@ -284,17 +284,19 @@ impl HTMLLinkElement {
         };
 
         let mut input = ParserInput::new(&mq_str);
         let mut css_parser = CssParser::new(&mut input);
         let doc_url = document.url();
         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());
         let integrity_metadata = match integrity_val {
             Some(ref value) => &***value,
             None => "",
         };
 
--- a/servo/components/script/dom/htmlstyleelement.rs
+++ b/servo/components/script/dom/htmlstyleelement.rs
@@ -69,39 +69,42 @@ impl HTMLStyleElement {
                            HTMLStyleElementBinding::Wrap)
     }
 
     pub fn parse_own_css(&self) {
         let node = self.upcast::<Node>();
         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"));
         let mq_str = match mq_attribute {
             Some(a) => String::from(&**a.value()),
             None => String::new(),
         };
 
         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);
 
         let sheet = Arc::new(sheet);
 
         // No subresource loads were triggered, just fire the load event now.
         if self.pending_loads.get() == 0 {
             self.upcast::<EventTarget>().fire_event(atom!("load"));
--- a/servo/components/script/dom/medialist.rs
+++ b/servo/components/script/dom/medialist.rs
@@ -69,23 +69,24 @@ impl MediaListMethods for MediaList {
             // Step 1
             *media_queries = StyleMediaList::empty();
             return;
         }
         // Step 3
         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
     fn Length(&self) -> u32 {
         let guard = self.shared_lock().read();
         self.media_queries.read_with(&guard).media_queries.len() as u32
     }
 
--- a/servo/components/script/dom/window.rs
+++ b/servo/components/script/dom/window.rs
@@ -1007,17 +1007,18 @@ impl WindowMethods for Window {
     fn MatchMedia(&self, query: DOMString) -> Root<MediaQueryList> {
         let mut input = ParserInput::new(&query);
         let mut parser = Parser::new(&mut input);
         let url = self.get_url();
         let quirks_mode = self.Document().quirks_mode();
         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);
         mql
     }
 
     #[allow(unrooted_must_root)]
     // https://fetch.spec.whatwg.org/#fetch-method
--- a/servo/components/style/error_reporting.rs
+++ b/servo/components/style/error_reporting.rs
@@ -14,17 +14,17 @@ use style_traits::ParseError;
 use stylesheets::UrlExtraData;
 
 /// Errors that can be encountered while parsing CSS.
 pub enum ContextualParseError<'a> {
     /// A property declaration was not recognized.
     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>),
     /// A font feature values rule was not valid.
     InvalidFontFeatureValuesRule(&'a str, ParseError<'a>),
     /// A keyframe property declaration was not recognized.
     UnsupportedKeyframePropertyDeclaration(&'a str, ParseError<'a>),
     /// A rule was invalid for some reason.
@@ -39,17 +39,19 @@ pub enum ContextualParseError<'a> {
     InvalidCounterStyleWithoutSymbols(String),
     /// A counter style rule had less than two symbols.
     InvalidCounterStyleNotEnoughSymbols(String),
     /// A counter style rule did not have additive-symbols.
     InvalidCounterStyleWithoutAdditiveSymbols,
     /// 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> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         fn token_to_str(t: &Token, f: &mut fmt::Formatter) -> fmt::Result {
             match *t {
                 Token::Ident(ref i) => write!(f, "identifier {}", i),
                 Token::AtKeyword(ref kw) => write!(f, "keyword @{}", kw),
@@ -163,16 +165,20 @@ impl<'a> fmt::Display for ContextualPars
                 write!(f, "Invalid @counter-style rule: 'system: additive' without 'additive-symbols'")
             }
             ContextualParseError::InvalidCounterStyleExtendsWithSymbols => {
                 write!(f, "Invalid @counter-style rule: 'system: extends …' with 'symbols'")
             }
             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)
+            }
         }
     }
 }
 
 /// A generic trait for an error reporter.
 pub trait ParseErrorReporter {
     /// Called when the style engine detects an error.
     ///
--- a/servo/components/style/gecko/media_queries.rs
+++ b/servo/components/style/gecko/media_queries.rs
@@ -18,17 +18,16 @@ use gecko_bindings::structs::{nsCSSKeywo
 use gecko_bindings::structs::{nsMediaExpression_Range, nsMediaFeature};
 use gecko_bindings::structs::{nsMediaFeature_ValueType, nsMediaFeature_RangeType, nsMediaFeature_RequirementFlags};
 use gecko_bindings::structs::{nsPresContext, RawGeckoPresContextOwned};
 use gecko_bindings::structs::nsIAtom;
 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};
 use str::starts_with_ignore_ascii_case;
 use string_cache::Atom;
 use style_traits::{CSSPixel, DevicePixel};
 use style_traits::{ToCss, ParseError, StyleParseError};
 use style_traits::viewport::ViewportConstraints;
@@ -464,16 +463,99 @@ unsafe fn find_in_table<F>(mut current_e
         if f(keyword, value) {
             return Some((keyword, value));
         }
 
         current_entry = current_entry.offset(1);
     }
 }
 
+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,
            value: Option<MediaExpressionValue>,
            range: nsMediaExpression_Range) -> Self {
         Expression {
             feature: feature,
             value: value,
@@ -483,23 +565,34 @@ impl Expression {
 
     /// Parse a media expression of the form:
     ///
     /// ```
     /// (media-feature: media-value)
     /// ```
     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 = {
                     let mut feature_name = &**ident;
 
                     if unsafe { structs::StylePrefs_sWebkitPrefixedAliasesEnabled } &&
                        starts_with_ignore_ascii_case(feature_name, "-webkit-") {
                         feature_name = &feature_name[8..];
@@ -525,116 +618,49 @@ impl Expression {
                         None => Err(()),
                     }
                 };
 
                 match result {
                     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());
                 }
             }
 
             // If there's no colon, this is a media query of the form
             // '(<feature>)', that is, there's no value specified.
             //
             // Gecko doesn't allow ranged expressions without a value, so just
             // reject them here too.
             if input.try(|i| i.expect_colon()).is_err() {
                 if range != nsMediaExpression_Range::eEqual {
                     return Err(StyleParseError::RangedExpressionWithNoValue.into())
                 }
                 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))
         })
     }
 
     /// Returns whether this media query evaluates to true for the given device.
     pub fn matches(&self, device: &Device, quirks_mode: QuirksMode) -> bool {
         let mut css_value = nsCSSValue::null();
--- a/servo/components/style/media_queries.rs
+++ b/servo/components/style/media_queries.rs
@@ -3,18 +3,20 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! [Media queries][mq].
 //!
 //! [mq]: https://drafts.csswg.org/mediaqueries/
 
 use Atom;
 use context::QuirksMode;
-use cssparser::{Delimiter, Parser, Token, ParserInput};
-use parser::ParserContext;
+use cssparser::{Delimiter, Parser};
+use cssparser::{Token, ParserInput};
+use error_reporting::{ContextualParseError, ParseErrorReporter};
+use parser::{ParserContext, ParserErrorContext};
 use selectors::parser::SelectorParseError;
 use serialize_comma_separated_list;
 use std::fmt;
 use str::string_as_ascii_lowercase;
 use style_traits::{ToCss, ParseError, StyleParseError};
 use values::CustomIdent;
 
 #[cfg(feature = "servo")]
@@ -235,29 +237,42 @@ impl MediaQuery {
 }
 
 /// Parse a media query list from CSS.
 ///
 /// Always returns a media query list. If any invalid media query is found, the
 /// media query list is only filled with the equivalent of "not all", see:
 ///
 /// https://drafts.csswg.org/mediaqueries/#error-handling
-pub fn parse_media_query_list(context: &ParserContext, input: &mut Parser) -> MediaList {
+pub fn parse_media_query_list<R>(
+    context: &ParserContext,
+    input: &mut Parser,
+    error_reporter: &R,
+) -> MediaList
+where
+    R: ParseErrorReporter,
+{
     if input.is_exhausted() {
         return MediaList::empty()
     }
 
     let mut media_queries = vec![];
     loop {
+        let start_position = input.position();
+        let start_location = input.current_source_location();
         match input.parse_until_before(Delimiter::Comma, |i| MediaQuery::parse(context, i)) {
             Ok(mq) => {
                 media_queries.push(mq);
             },
-            Err(..) => {
+            Err(err) => {
                 media_queries.push(MediaQuery::never_matching());
+                let error = ContextualParseError::InvalidMediaRule(
+                    input.slice_from(start_position), err);
+                let error_context = ParserErrorContext { error_reporter };
+                context.log_css_error(&error_context, start_location, error);
             },
         }
 
         match input.next() {
             Ok(&Token::Comma) => {},
             Ok(_) => unreachable!(),
             Err(_) => break,
         }
--- a/servo/components/style/stylesheets/rule_parser.rs
+++ b/servo/components/style/stylesheets/rule_parser.rs
@@ -173,17 +173,18 @@ impl<'a, 'i, R: ParseErrorReporter> AtRu
                     // "@import must be before any rule but @charset"
                     self.had_hierarchy_error = true;
                     return Err(StyleParseError::UnexpectedImportRule.into())
                 }
 
                 let url_string = input.expect_url_or_string()?.as_ref().to_owned();
                 let specified_url = SpecifiedUrl::parse_from_string(url_string, &self.context)?;
 
-                let media = parse_media_query_list(&self.context, input);
+                let media = parse_media_query_list(&self.context, input,
+                                                   self.error_context.error_reporter);
                 let media = Arc::new(self.shared_lock.wrap(media));
 
                 let prelude = AtRuleNonBlockPrelude::Import(specified_url, media, location);
                 return Ok(AtRuleType::WithoutBlock(prelude));
             },
             "namespace" => {
                 if self.state > State::Namespaces {
                     // "@namespace must be before any rule but @charset and @import"
@@ -349,17 +350,18 @@ impl<'a, 'b, 'i, R: ParseErrorReporter> 
         &mut self,
         name: CowRcStr<'i>,
         input: &mut Parser<'i, 't>
     ) -> Result<AtRuleType<AtRuleNonBlockPrelude, AtRuleBlockPrelude>, ParseError<'i>> {
         let location = get_location_with_offset(input.current_source_location());
 
         match_ignore_ascii_case! { &*name,
             "media" => {
-                let media_queries = parse_media_query_list(self.context, input);
+                let media_queries = parse_media_query_list(self.context, input,
+                                                           self.error_context.error_reporter);
                 let arc = Arc::new(self.shared_lock.wrap(media_queries));
                 Ok(AtRuleType::WithBlock(AtRuleBlockPrelude::Media(arc, location)))
             },
             "supports" => {
                 let cond = SupportsCondition::parse(input)?;
                 Ok(AtRuleType::WithBlock(AtRuleBlockPrelude::Supports(cond, location)))
             },
             "font-face" => {
--- a/servo/components/style_traits/lib.rs
+++ b/servo/components/style_traits/lib.rs
@@ -102,17 +102,23 @@ pub enum StyleParseError<'i> {
     /// Unexpected closing curly bracket in a DVB.
     UnbalancedCloseCurlyBracketInDeclarationValueBlock,
     /// A property declaration parsing error.
     PropertyDeclaration(PropertyDeclarationParseError<'i>),
     /// A property declaration value had input remaining after successfully parsing.
     PropertyDeclarationValueNotExhausted,
     /// An unexpected dimension token was encountered.
     UnexpectedDimension(CowRcStr<'i>),
-    /// A media query using a ranged expression with no value was encountered.
+    /// Expected identifier not found.
+    ExpectedIdentifier(Token<'i>),
+    /// Missing or invalid media feature name.
+    MediaQueryExpectedFeatureName(CowRcStr<'i>),
+    /// Missing or invalid media feature value.
+    MediaQueryExpectedFeatureValue,
+    /// min- or max- properties must have a value.
     RangedExpressionWithNoValue,
     /// A function was encountered that was not expected.
     UnexpectedFunction(CowRcStr<'i>),
     /// @namespace must be before any rule but @charset and @import
     UnexpectedNamespaceRule,
     /// @import must be before any rule but @charset
     UnexpectedImportRule,
     /// Unexpected @charset rule encountered.
--- a/servo/ports/geckolib/error_reporter.rs
+++ b/servo/ports/geckolib/error_reporter.rs
@@ -136,16 +136,24 @@ struct ErrorParams<'a> {
 /// a second parameter if it exists, for use in the prefix for the eventual error message.
 fn extract_error_params<'a>(err: ParseError<'a>) -> Option<ErrorParams<'a>> {
     let (main, prefix) = match err {
         CssParseError::Custom(SelectorParseError::Custom(
             StyleParseError::PropertyDeclaration(
                 PropertyDeclarationParseError::InvalidValue(property, Some(e))))) =>
             (Some(ErrorString::Snippet(property.into())), Some(extract_value_error_param(e))),
 
+        CssParseError::Custom(SelectorParseError::Custom(
+            StyleParseError::MediaQueryExpectedFeatureName(ident))) =>
+            (Some(ErrorString::Ident(ident)), None),
+
+        CssParseError::Custom(SelectorParseError::Custom(
+            StyleParseError::ExpectedIdentifier(token))) =>
+            (Some(ErrorString::UnexpectedToken(token)), None),
+
         CssParseError::Custom(SelectorParseError::UnexpectedTokenInAttributeSelector(t)) |
         CssParseError::Custom(SelectorParseError::BadValueInAttr(t)) |
         CssParseError::Custom(SelectorParseError::ExpectedBarInAttr(t)) |
         CssParseError::Custom(SelectorParseError::NoQualifiedNameInAttributeSelector(t)) |
         CssParseError::Custom(SelectorParseError::InvalidQualNameInAttr(t)) |
         CssParseError::Custom(SelectorParseError::ExplicitNamespaceUnexpectedToken(t)) |
         CssParseError::Custom(SelectorParseError::PseudoElementExpectedIdent(t)) |
         CssParseError::Custom(SelectorParseError::NoIdentForPseudo(t)) |
@@ -184,17 +192,18 @@ impl<'a> ErrorHelpers<'a> for Contextual
             ContextualParseError::UnsupportedFontFaceDescriptor(s, err) |
             ContextualParseError::UnsupportedFontFeatureValuesDescriptor(s, err) |
             ContextualParseError::InvalidKeyframeRule(s, err) |
             ContextualParseError::InvalidFontFeatureValuesRule(s, err) |
             ContextualParseError::UnsupportedKeyframePropertyDeclaration(s, err) |
             ContextualParseError::InvalidRule(s, err) |
             ContextualParseError::UnsupportedRule(s, err) |
             ContextualParseError::UnsupportedViewportDescriptorDeclaration(s, err) |
-            ContextualParseError::UnsupportedCounterStyleDescriptorDeclaration(s, err) =>
+            ContextualParseError::UnsupportedCounterStyleDescriptorDeclaration(s, err) |
+            ContextualParseError::InvalidMediaRule(s, err) =>
                 (s.into(), err),
             ContextualParseError::InvalidCounterStyleWithoutSymbols(s) |
             ContextualParseError::InvalidCounterStyleNotEnoughSymbols(s) =>
                 (s.into(), StyleParseError::UnspecifiedError.into()),
             ContextualParseError::InvalidCounterStyleWithoutAdditiveSymbols |
             ContextualParseError::InvalidCounterStyleExtendsWithSymbols |
             ContextualParseError::InvalidCounterStyleExtendsWithAdditiveSymbols =>
                 ("".into(), StyleParseError::UnspecifiedError.into())
@@ -276,16 +285,40 @@ impl<'a> ErrorHelpers<'a> for Contextual
                     CssParseError::Custom(SelectorParseError::ClassNeedsIdent(_)) =>
                         Some(&b"PEClassSelNotIdent\0"[..]),
                     CssParseError::Custom(SelectorParseError::EmptyNegation) =>
                         Some(&b"PENegationBadArg\0"[..]),
                     _ => None,
                 };
                 return (prefix, b"PEBadSelectorRSIgnored\0", Action::Nothing);
             }
+            ContextualParseError::InvalidMediaRule(_, ref err) => {
+                let err: &[u8] = match *err {
+                    CssParseError::Custom(SelectorParseError::Custom(
+                            StyleParseError::ExpectedIdentifier(..))) => {
+                        b"PEGatherMediaNotIdent\0"
+                    },
+                    CssParseError::Custom(SelectorParseError::Custom(
+                            StyleParseError::MediaQueryExpectedFeatureName(..))) => {
+                        b"PEMQExpectedFeatureName\0"
+                    },
+                    CssParseError::Custom(SelectorParseError::Custom(
+                            StyleParseError::MediaQueryExpectedFeatureValue)) => {
+                        b"PEMQExpectedFeatureValue\0"
+                    },
+                    CssParseError::Custom(SelectorParseError::Custom(
+                            StyleParseError::RangedExpressionWithNoValue)) => {
+                        b"PEMQNoMinMaxWithoutValue\0"
+                    },
+                    _ => {
+                        b"PEDeclDropped\0"
+                    },
+                };
+                (err, Action::Nothing)
+            }
             ContextualParseError::UnsupportedRule(..) =>
                 (b"PEDeclDropped\0", Action::Nothing),
             ContextualParseError::UnsupportedViewportDescriptorDeclaration(..) |
             ContextualParseError::UnsupportedCounterStyleDescriptorDeclaration(..) |
             ContextualParseError::InvalidCounterStyleWithoutSymbols(..) |
             ContextualParseError::InvalidCounterStyleNotEnoughSymbols(..) |
             ContextualParseError::InvalidCounterStyleWithoutAdditiveSymbols |
             ContextualParseError::InvalidCounterStyleExtendsWithSymbols |
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -2486,17 +2486,17 @@ pub extern "C" fn Servo_MediaList_SetTex
     let text = unsafe { text.as_ref().unwrap().as_str_unchecked() };
     let mut input = ParserInput::new(&text);
     let mut parser = Parser::new(&mut input);
     let url_data = unsafe { dummy_url_data() };
     let context = ParserContext::new_for_cssom(url_data, Some(CssRuleType::Media),
                                                PARSING_MODE_DEFAULT,
                                                QuirksMode::NoQuirks);
      write_locked_arc(list, |list: &mut MediaList| {
-        *list = parse_media_query_list(&context, &mut parser);
+        *list = parse_media_query_list(&context, &mut parser, &NullReporter);
     })
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_MediaList_GetLength(list: RawServoMediaListBorrowed) -> u32 {
     read_locked_arc(list, |list: &MediaList| list.media_queries.len() as u32)
 }