diff --git a/components/script/dom/cssmediarule.rs b/components/script/dom/cssmediarule.rs index ff3c32939c2d..dd921d9f787b 100644 --- a/components/script/dom/cssmediarule.rs +++ b/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. diff --git a/components/script/dom/htmllinkelement.rs b/components/script/dom/htmllinkelement.rs index 62e95fbcb12e..963f570700a4 100644 --- a/components/script/dom/htmllinkelement.rs +++ b/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()); diff --git a/components/script/dom/htmlstyleelement.rs b/components/script/dom/htmlstyleelement.rs index ac5df3d9689e..330ee738466b 100644 --- a/components/script/dom/htmlstyleelement.rs +++ b/components/script/dom/htmlstyleelement.rs @@ -74,7 +74,7 @@ impl HTMLStyleElement { let element = self.upcast::(); 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); diff --git a/components/script/dom/medialist.rs b/components/script/dom/medialist.rs index 937347764b85..f737ecc9d2ed 100644 --- a/components/script/dom/medialist.rs +++ b/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 diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 9f80203b4f61..114d5ca3a0df 100644 --- a/components/script/dom/window.rs +++ b/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); diff --git a/components/style/error_reporting.rs b/components/style/error_reporting.rs index bfc9df177620..718306323a30 100644 --- a/components/style/error_reporting.rs +++ b/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) + } } } } diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index 067375bef40b..c1637b3c2838 100644 --- a/components/style/gecko/media_queries.rs +++ b/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(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> { + 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> { - 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)) }) diff --git a/components/style/media_queries.rs b/components/style/media_queries.rs index dd90a30088c4..83b3f580922d 100644 --- a/components/style/media_queries.rs +++ b/components/style/media_queries.rs @@ -8,8 +8,10 @@ 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; @@ -240,19 +242,32 @@ impl MediaQuery { /// 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( + 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); }, } diff --git a/components/style/stylesheets/rule_parser.rs b/components/style/stylesheets/rule_parser.rs index 470acbbc581a..2dc1c3060e62 100644 --- a/components/style/stylesheets/rule_parser.rs +++ b/components/style/stylesheets/rule_parser.rs @@ -178,7 +178,8 @@ impl<'a, 'i, R: ParseErrorReporter> AtRuleParser<'i> for TopLevelRuleParser<'a, 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); @@ -354,7 +355,8 @@ impl<'a, 'b, 'i, R: ParseErrorReporter> AtRuleParser<'i> for NestedRuleParser<'a 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))) }, diff --git a/components/style_traits/lib.rs b/components/style_traits/lib.rs index 5ceab72427d5..921d6158660a 100644 --- a/components/style_traits/lib.rs +++ b/components/style_traits/lib.rs @@ -107,7 +107,13 @@ pub enum StyleParseError<'i> { 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>), diff --git a/ports/geckolib/error_reporter.rs b/ports/geckolib/error_reporter.rs index 4b0a1ed088ac..b8aa1f091b60 100644 --- a/ports/geckolib/error_reporter.rs +++ b/ports/geckolib/error_reporter.rs @@ -141,6 +141,14 @@ fn extract_error_params<'a>(err: ParseError<'a>) -> Option> { 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)) | @@ -189,7 +197,8 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> { 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) => @@ -281,6 +290,30 @@ impl<'a> ErrorHelpers<'a> for ContextualParseError<'a> { }; 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(..) | diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index c615e082c793..96b89c829dc0 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -2452,7 +2452,7 @@ pub extern "C" fn Servo_MediaList_SetText(list: RawServoMediaListBorrowed, text: 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); }) }