Bug 1422225: Error reporting fixes. r?xidorn
Do it so that we always try to evaluate the media expression and the modern
syntax last.
This will regress stuff like:
@media all and (invalid-feature-value) { }
Where instead of reporting the invalid feature value we'd report the unexpected
token 'all'. But I didn't want to complicate the code too much because this
error reporting stuff is going to need to change regardless with
bug 1469173.
MozReview-Commit-ID: 2tqdAsWh6Kh
--- a/dom/locales/en-US/chrome/layout/css.properties
+++ b/dom/locales/en-US/chrome/layout/css.properties
@@ -13,16 +13,18 @@ PEUnknownProperty=Unknown property ‘%1$S’.
PEValueParsingError=Error in parsing value for ‘%1$S’.
PEExpectEndValue=Expected end of value but found ‘%1$S’.
PERuleTrailing=Expected end of rule but found ‘%1$S’.
PESkipAtRuleEOF2=end of at-rule
PEUnknownAtRule=Unrecognized at-rule or error parsing at-rule ‘%1$S’.
PECharsetRuleEOF=charset string in @charset rule
PECharsetRuleNotString=Expected charset string but found ‘%1$S’.
PEGatherMediaEOF=end of media list in @import or @media rule
+PEMQUnexpectedOperator=Unexpected operator in media list.
+PEMQUnexpectedToken=Unexpected token ‘%1$S’ in media list.
PEGatherMediaNotComma=Expected ‘,’ in media list but found ‘%1$S’.
PEGatherMediaNotIdent=Expected identifier in media list but found ‘%1$S’.
PEGatherMediaReservedMediaType=Found reserved keyword ‘%1$S’ when looking for media type.
PEParseSourceSizeListEOF=length value for matched media condition
PEParseSourceSizeListNotComma=Expected ‘,’ after value but found ‘%1$S’
PEImportNotURI=Expected URI in @import rule but found ‘%1$S’.
PEImportBadURI=Invalid URI in @import rule: ‘%1$S’.
PEImportUnexpected=Found unexpected ‘%1$S’ within @import.
--- a/layout/style/test/test_css_parse_error_smoketest.html
+++ b/layout/style/test/test_css_parse_error_smoketest.html
@@ -36,18 +36,19 @@
{ css: "| {}", error: "Expected element name or ‘*’ but found ‘ ’. Ruleset ignored due to bad selector." },
{ css: ". {}", error: "Expected identifier for class selector but found ‘ ’. Ruleset ignored due to bad selector." },
{ css: ":not() {}", error: "Missing argument in negation pseudo-class ‘)’. Ruleset ignored due to bad selector." },
{ css: "* { -webkit-text-size-adjust: 100% }", error: "Error in parsing value for ‘-webkit-text-size-adjust’. Declaration dropped." },
{ css: "@media (totally-unknown-feature) {}", error: "Expected media feature name but found ‘totally-unknown-feature’." },
- { css: "@media \"foo\" {}", error: "Expected identifier in media list but found ‘\"foo\"’." },
+ { css: "@media \"foo\" {}", error: "Unexpected token ‘\"foo\"’ in media list." },
{ css: "@media (min-width) {}", error: "Media features with min- or max- must have a value." },
+ { css: "@media (min-width >= 3px) {}", error: "Unexpected operator in media list." },
{ css: "@media (device-height: -1px) {}", error: "Found invalid value for media feature." },
{ css: "@media (min-width: -1px) {}", error: "Found invalid value for media feature." },
{ css: "@media (min-resolution: 2) {}", error: "Found invalid value for media feature." },
{ css: "@media (min-monochrome: 1.1) {}", error: "Found invalid value for media feature." },
{ css: "@media (min-aspect-ratio: 1) {}", error: "Found invalid value for media feature." },
{ css: "@media (orientation: invalid-orientation-value) {}", error: "Found invalid value for media feature." },
];
--- a/servo/components/style/gecko/media_queries.rs
+++ b/servo/components/style/gecko/media_queries.rs
@@ -2,17 +2,17 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
//! Gecko's media-query device and expression representation.
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};
use gecko_bindings::bindings;
use gecko_bindings::structs;
use gecko_bindings::structs::{nsCSSKTableEntry, nsCSSKeyword, nsCSSUnit, nsCSSValue};
use gecko_bindings::structs::{nsMediaFeature, nsMediaFeature_RangeType};
use gecko_bindings::structs::{nsMediaFeature_ValueType, nsPresContext};
@@ -615,49 +615,34 @@ impl MediaFeatureExpression {
///
/// ```
/// (media-feature: media-value)
/// ```
pub fn parse<'i, 't>(
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
/// parenthesis.
pub fn parse_in_parenthesis_block<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
) -> Result<Self, ParseError<'i>> {
// FIXME: remove extra indented block when lifetimes are non-lexical
let feature;
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;
if context.chrome_rules_enabled() || context.stylesheet_origin == Origin::UserAgent
{
flags |= structs::nsMediaFeature_RequirementFlags_eUserAgentAndChromeOnly;
}
@@ -742,27 +727,27 @@ impl MediaFeatureExpression {
}
Ok(operator) => operator,
};
let range_or_operator = match range {
Some(range) => {
if operator.is_some() {
return Err(input.new_custom_error(
- StyleParseErrorKind::MediaQueryExpectedFeatureValue
+ StyleParseErrorKind::MediaQueryUnexpectedOperator
));
}
Some(RangeOrOperator::Range(range))
}
None => {
match operator {
Some(operator) => {
if !feature_allows_ranges {
return Err(input.new_custom_error(
- StyleParseErrorKind::MediaQueryExpectedFeatureValue
+ StyleParseErrorKind::MediaQueryUnexpectedOperator
));
}
Some(RangeOrOperator::Operator(operator))
}
None => None,
}
}
};
--- a/servo/components/style/media_queries/media_condition.rs
+++ b/servo/components/style/media_queries/media_condition.rs
@@ -158,22 +158,21 @@ impl MediaCondition {
}
fn parse_paren_block<'i, 't>(
context: &ParserContext,
input: &mut Parser<'i, 't>,
) -> 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))
})
}
/// Whether this condition matches the device and quirks mode.
pub fn matches(&self, device: &Device, quirks_mode: QuirksMode) -> bool {
match *self {
MediaCondition::Feature(ref f) => f.matches(device, quirks_mode),
MediaCondition::InParens(ref c) => c.matches(device, quirks_mode),
--- a/servo/components/style/media_queries/media_query.rs
+++ b/servo/components/style/media_queries/media_query.rs
@@ -4,17 +4,16 @@
//! A media query:
//!
//! https://drafts.csswg.org/mediaqueries/#typedef-media-query
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};
use super::media_condition::MediaCondition;
use values::CustomIdent;
/// <https://drafts.csswg.org/mediaqueries/#mq-prefix>
@@ -120,44 +119,33 @@ impl MediaQuery {
}
/// Parse a media query given css input.
///
/// Returns an error if any of the expressions is unknown.
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 })
}
}
/// <http://dev.w3.org/csswg/mediaqueries-3/#media0>
#[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq)]
pub enum MediaQueryType {
/// A media type that matches every device.
--- a/servo/components/style_traits/lib.rs
+++ b/servo/components/style_traits/lib.rs
@@ -101,22 +101,22 @@ pub enum StyleParseErrorKind<'i> {
/// Unexpected closing bracket in a DVB.
UnbalancedCloseSquareBracketInDeclarationValueBlock,
/// Unexpected closing curly bracket in a DVB.
UnbalancedCloseCurlyBracketInDeclarationValueBlock,
/// A property declaration value had input remaining after successfully parsing.
PropertyDeclarationValueNotExhausted,
/// An unexpected dimension token was encountered.
UnexpectedDimension(CowRcStr<'i>),
- /// Expected identifier not found.
- ExpectedIdentifier(Token<'i>),
/// Missing or invalid media feature name.
MediaQueryExpectedFeatureName(CowRcStr<'i>),
/// Missing or invalid media feature value.
MediaQueryExpectedFeatureValue,
+ /// A media feature range operator was not expected.
+ MediaQueryUnexpectedOperator,
/// 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,
--- a/servo/ports/geckolib/error_reporter.rs
+++ b/servo/ports/geckolib/error_reporter.rs
@@ -140,19 +140,17 @@ fn extract_error_params<'a>(err: ErrorKi
}
ParseErrorKind::Custom(
StyleParseErrorKind::MediaQueryExpectedFeatureName(ident)
) => {
(Some(ErrorString::Ident(ident)), None)
}
- ParseErrorKind::Custom(
- StyleParseErrorKind::ExpectedIdentifier(token)
- ) |
+ ParseErrorKind::Basic(BasicParseErrorKind::UnexpectedToken(token)) |
ParseErrorKind::Custom(
StyleParseErrorKind::ValueError(ValueParseErrorKind::InvalidColor(token))
) => {
(Some(ErrorString::UnexpectedToken(token)), None)
}
ParseErrorKind::Custom(StyleParseErrorKind::SelectorError(err)) => match err {
SelectorParseErrorKind::UnexpectedTokenInAttributeSelector(t) |
@@ -337,30 +335,30 @@ impl<'a> ErrorHelpers<'a> for Contextual
_ => None,
},
_ => None,
};
return (prefix, cstr!("PEBadSelectorRSIgnored"), Action::Nothing);
}
ContextualParseError::InvalidMediaRule(_, ref err) => {
let err: &CStr = match err.kind {
- ParseErrorKind::Custom(StyleParseErrorKind::ExpectedIdentifier(..)) => {
- cstr!("PEGatherMediaNotIdent")
- },
ParseErrorKind::Custom(StyleParseErrorKind::MediaQueryExpectedFeatureName(..)) => {
cstr!("PEMQExpectedFeatureName")
},
ParseErrorKind::Custom(StyleParseErrorKind::MediaQueryExpectedFeatureValue) => {
cstr!("PEMQExpectedFeatureValue")
},
+ ParseErrorKind::Custom(StyleParseErrorKind::MediaQueryUnexpectedOperator) => {
+ cstr!("PEMQUnexpectedOperator")
+ },
ParseErrorKind::Custom(StyleParseErrorKind::RangedExpressionWithNoValue) => {
cstr!("PEMQNoMinMaxWithoutValue")
},
_ => {
- cstr!("PEDeclDropped")
+ cstr!("PEMQUnexpectedToken")
},
};
(err, Action::Nothing)
}
ContextualParseError::UnsupportedRule(..) =>
(cstr!("PEDeclDropped"), Action::Nothing),
ContextualParseError::UnsupportedViewportDescriptorDeclaration(..) |
ContextualParseError::UnsupportedCounterStyleDescriptorDeclaration(..) |