style: Error reporting fixes for media queries.

Do it so that we always try to evaluate the media expression and the modern
syntax last, so that the most specific error message comes up.

Bug: 1422225
Reviewed-by: xidorn
MozReview-Commit-ID: 2tqdAsWh6Kh

components/style/gecko/media_queries.rs

@@ -7,7 +7,7 @@
 use app_units::AU_PER_PX;
 use app_units::Au;
 use context::QuirksMode;
-use cssparser::{BasicParseErrorKind, Parser, RGBA, Token};
+use cssparser::{Parser, RGBA, Token};
 use euclid::Size2D;
 use euclid::TypedScale;
 use gecko::values::{convert_nscolor_to_rgba, convert_rgba_to_nscolor};
@@ -292,8 +292,8 @@ enum RangeOrOperator {
     Operator(Operator),
 }
 
-/// A range expression for gecko contains a reference to the media feature, the
-/// value the media query contained, and the range to evaluate.
+/// A feature expression for gecko contains a reference to the media feature,
+/// the value the media query contained, and the range to evaluate.
 #[derive(Clone, Debug, MallocSizeOf)]
 pub struct MediaFeatureExpression {
     feature: &'static nsMediaFeature,
@@ -641,21 +641,13 @@ impl MediaFeatureExpression {
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
-        input.expect_parenthesis_block().map_err(|err| {
-            err.location.new_custom_error(match err.kind {
-                BasicParseErrorKind::UnexpectedToken(t) => {
-                    StyleParseErrorKind::ExpectedIdentifier(t)
-                },
-                _ => StyleParseErrorKind::UnspecifiedError,
-            })
-        })?;
-
+        input.expect_parenthesis_block()?;
         input.parse_nested_block(|input| {
             Self::parse_in_parenthesis_block(context, input)
         })
     }
 
-    /// Parse a media range expression where we've already consumed the
+    /// Parse a media feature expression where we've already consumed the
     /// parenthesis.
     pub fn parse_in_parenthesis_block<'i, 't>(
         context: &ParserContext,
@@ -666,14 +658,7 @@ impl MediaFeatureExpression {
         let range;
         {
             let location = input.current_source_location();
-            let ident = input.expect_ident().map_err(|err| {
-                err.location.new_custom_error(match err.kind {
-                    BasicParseErrorKind::UnexpectedToken(t) => {
-                        StyleParseErrorKind::ExpectedIdentifier(t)
-                    },
-                    _ => StyleParseErrorKind::UnspecifiedError,
-                })
-            })?;
+            let ident = input.expect_ident()?;
 
             let mut flags = 0;
 
@@ -768,7 +753,7 @@ impl MediaFeatureExpression {
             Some(range) => {
                 if operator.is_some() {
                     return Err(input.new_custom_error(
-                        StyleParseErrorKind::MediaQueryExpectedFeatureValue
+                        StyleParseErrorKind::MediaQueryUnexpectedOperator
                     ));
                 }
                 Some(RangeOrOperator::Range(range))
@@ -778,7 +763,7 @@ impl MediaFeatureExpression {
                     Some(operator) => {
                         if !feature_allows_ranges {
                             return Err(input.new_custom_error(
-                                StyleParseErrorKind::MediaQueryExpectedFeatureValue
+                                StyleParseErrorKind::MediaQueryUnexpectedOperator
                             ));
                         }
                         Some(RangeOrOperator::Operator(operator))

components/style/media_queries/media_condition.rs

@@ -163,12 +163,11 @@ impl MediaCondition {
     ) -> Result<Self, ParseError<'i>> {
         input.parse_nested_block(|input| {
             // Base case.
-            if let Ok(expr) = input.try(|i| MediaFeatureExpression::parse_in_parenthesis_block(context, i)) {
-                return Ok(MediaCondition::Feature(expr));
+            if let Ok(inner) = input.try(|i| Self::parse(context, i)) {
+                return Ok(MediaCondition::InParens(Box::new(inner)))
             }
-
-            let inner = Self::parse(context, input)?;
-            Ok(MediaCondition::InParens(Box::new(inner)))
+            let expr = MediaFeatureExpression::parse_in_parenthesis_block(context, input)?;
+            Ok(MediaCondition::Feature(expr))
         })
     }
 

components/style/media_queries/media_query.rs

@@ -9,7 +9,6 @@
 use Atom;
 use cssparser::Parser;
 use parser::ParserContext;
-use selectors::parser::SelectorParseErrorKind;
 use std::fmt::{self, Write};
 use str::string_as_ascii_lowercase;
 use style_traits::{CssWriter, ParseError, ToCss};
@@ -125,34 +124,23 @@ impl MediaQuery {
     pub fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
-    ) -> Result<MediaQuery, ParseError<'i>> {
-        if let Ok(condition) = input.try(|i| MediaCondition::parse(context, i)) {
-            return Ok(Self {
-                qualifier: None,
-                media_type: MediaQueryType::All,
-                condition: Some(condition),
-            })
-        }
+    ) -> Result<Self, ParseError<'i>> {
+        let (qualifier, explicit_media_type) = input.try(|input| -> Result<_, ()> {
+            let qualifier = input.try(Qualifier::parse).ok();
+            let ident = input.expect_ident().map_err(|_| ())?;
+            let media_type = MediaQueryType::parse(&ident)?;
+            Ok((qualifier, Some(media_type)))
+        }).unwrap_or_default();
 
-        let qualifier = input.try(Qualifier::parse).ok();
-        let media_type = {
-            let location = input.current_source_location();
-            let ident = input.expect_ident()?;
-            match MediaQueryType::parse(&ident) {
-                Ok(t) => t,
-                Err(..) => return Err(location.new_custom_error(
-                    SelectorParseErrorKind::UnexpectedIdent(ident.clone())
-                ))
-            }
+        let condition = if explicit_media_type.is_none() {
+            Some(MediaCondition::parse(context, input)?)
+        } else if input.try(|i| i.expect_ident_matching("and")).is_ok() {
+            Some(MediaCondition::parse_disallow_or(context, input)?)
+        } else {
+            None
         };
 
-        let condition =
-            if input.try(|i| i.expect_ident_matching("and")).is_ok() {
-                Some(MediaCondition::parse_disallow_or(context, input)?)
-            } else {
-                None
-            };
-
+        let media_type = explicit_media_type.unwrap_or(MediaQueryType::All);
         Ok(Self { qualifier, media_type, condition })
     }
 }