From 056e5995624598575ace9c6647967c7a3b2c7287 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Thu, 21 Sep 2017 01:44:53 +0200 Subject: [PATCH] Use the current parser location for CSS error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … rather than the start location of the current construct. This likely places the error just *after* of the unexpected token whereas before would be best, but that’s likely a much bigger change. See https://bugzilla.mozilla.org/show_bug.cgi?id=1378861 --- components/style/counter_style/mod.rs | 2 +- components/style/font_face.rs | 2 +- components/style/media_queries.rs | 3 +- .../style/properties/declaration_block.rs | 6 ++-- .../stylesheets/font_feature_values_rule.rs | 6 ++-- .../style/stylesheets/keyframes_rule.rs | 4 +-- components/style/stylesheets/rule_parser.rs | 3 +- components/style/stylesheets/stylesheet.rs | 3 +- components/style/stylesheets/viewport_rule.rs | 2 +- tests/unit/style/stylesheets.rs | 34 +++++++++---------- 10 files changed, 34 insertions(+), 31 deletions(-) diff --git a/components/style/counter_style/mod.rs b/components/style/counter_style/mod.rs index cb0a9f3d8c7..67ad1695c2f 100644 --- a/components/style/counter_style/mod.rs +++ b/components/style/counter_style/mod.rs @@ -68,7 +68,7 @@ pub fn parse_counter_style_body<'i, 't, R>(name: CustomIdent, while let Some(declaration) = iter.next() { if let Err(err) = declaration { let error = ContextualParseError::UnsupportedCounterStyleDescriptorDeclaration(err.slice, err.error); - context.log_css_error(error_context, err.location, error) + context.log_css_error(error_context, iter.input.current_source_location(), error) } } } diff --git a/components/style/font_face.rs b/components/style/font_face.rs index 30eb242a6d4..ce70e3c58d4 100644 --- a/components/style/font_face.rs +++ b/components/style/font_face.rs @@ -125,7 +125,7 @@ pub fn parse_font_face_block(context: &ParserContext, while let Some(declaration) = iter.next() { if let Err(err) = declaration { let error = ContextualParseError::UnsupportedFontFaceDescriptor(err.slice, err.error); - context.log_css_error(error_context, err.location, error) + context.log_css_error(error_context, iter.input.current_source_location(), error) } } } diff --git a/components/style/media_queries.rs b/components/style/media_queries.rs index 83b3f580922..691ab63b4c1 100644 --- a/components/style/media_queries.rs +++ b/components/style/media_queries.rs @@ -257,7 +257,6 @@ where 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); @@ -267,7 +266,7 @@ where 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); + context.log_css_error(&error_context, input.current_source_location(), error); }, } diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index df4840f0dda..d581a832873 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -1035,7 +1035,6 @@ pub fn parse_one_declaration_into(declarations: &mut SourcePropertyDeclaratio let mut input = ParserInput::new(input); let mut parser = Parser::new(&mut input); let start_position = parser.position(); - let start_location = parser.current_source_location(); parser.parse_entirely(|parser| { let name = id.name().into(); PropertyDeclaration::parse_into(declarations, id, name, &context, parser) @@ -1044,7 +1043,7 @@ pub fn parse_one_declaration_into(declarations: &mut SourcePropertyDeclaratio let error = ContextualParseError::UnsupportedPropertyDeclaration( parser.slice_from(start_position), err); let error_context = ParserErrorContext { error_reporter: error_reporter }; - context.log_css_error(&error_context, start_location, error); + context.log_css_error(&error_context, parser.current_source_location(), error); }) } @@ -1133,7 +1132,8 @@ pub fn parse_property_declaration_list(context: &ParserContext, } let error = ContextualParseError::UnsupportedPropertyDeclaration(err.slice, err.error); - context.log_css_error(error_context, err.location, error); + let location = iter.input.current_source_location(); + context.log_css_error(error_context, location, error); } } } diff --git a/components/style/stylesheets/font_feature_values_rule.rs b/components/style/stylesheets/font_feature_values_rule.rs index e2adac1fc8b..5808e3fe42b 100644 --- a/components/style/stylesheets/font_feature_values_rule.rs +++ b/components/style/stylesheets/font_feature_values_rule.rs @@ -273,7 +273,8 @@ macro_rules! font_feature_values_blocks { while let Some(result) = iter.next() { if let Err(err) = result { let error = ContextualParseError::UnsupportedRule(err.slice, err.error); - context.log_css_error(error_context, err.location, error); + let location = iter.input.current_source_location(); + context.log_css_error(error_context, location, error); } } } @@ -427,7 +428,8 @@ macro_rules! font_feature_values_blocks { if let Err(err) = declaration { let error = ContextualParseError::UnsupportedKeyframePropertyDeclaration( err.slice, err.error); - self.context.log_css_error(self.error_context, err.location, error); + let location = iter.input.current_source_location(); + self.context.log_css_error(self.error_context, location, error); } } }, diff --git a/components/style/stylesheets/keyframes_rule.rs b/components/style/stylesheets/keyframes_rule.rs index 8541f58d248..0a9bf30c3ec 100644 --- a/components/style/stylesheets/keyframes_rule.rs +++ b/components/style/stylesheets/keyframes_rule.rs @@ -526,7 +526,7 @@ impl<'a, 'i, R: ParseErrorReporter> QualifiedRuleParser<'i> for KeyframeListPars }, Err(e) => { let error = ContextualParseError::InvalidKeyframeRule(input.slice_from(start_position), e.clone()); - self.context.log_css_error(self.error_context, start_location, error); + self.context.log_css_error(self.error_context, input.current_source_location(), error); Err(e) } } @@ -555,7 +555,7 @@ impl<'a, 'i, R: ParseErrorReporter> QualifiedRuleParser<'i> for KeyframeListPars Err(err) => { iter.parser.declarations.clear(); let error = ContextualParseError::UnsupportedKeyframePropertyDeclaration(err.slice, err.error); - context.log_css_error(self.error_context, err.location, error); + context.log_css_error(self.error_context, iter.input.current_source_location(), error); } } // `parse_important` is not called here, `!important` is not allowed in keyframe blocks. diff --git a/components/style/stylesheets/rule_parser.rs b/components/style/stylesheets/rule_parser.rs index 30d1219dab1..b2779508031 100644 --- a/components/style/stylesheets/rule_parser.rs +++ b/components/style/stylesheets/rule_parser.rs @@ -333,7 +333,8 @@ impl<'a, 'b, R: ParseErrorReporter> NestedRuleParser<'a, 'b, R> { Ok(rule) => rules.push(rule), Err(err) => { let error = ContextualParseError::UnsupportedRule(err.slice, err.error); - self.context.log_css_error(self.error_context, err.location, error); + let location = iter.input.current_source_location(); + self.context.log_css_error(self.error_context, location, error); } } } diff --git a/components/style/stylesheets/stylesheet.rs b/components/style/stylesheets/stylesheet.rs index 1b5d2285af6..c17ba75715b 100644 --- a/components/style/stylesheets/stylesheet.rs +++ b/components/style/stylesheets/stylesheet.rs @@ -396,8 +396,9 @@ impl Stylesheet { }, Err(err) => { let error = ContextualParseError::InvalidRule(err.slice, err.error); + let location = iter.input.current_source_location(); iter.parser.context.log_css_error(&iter.parser.error_context, - err.location, error); + location, error); } } } diff --git a/components/style/stylesheets/viewport_rule.rs b/components/style/stylesheets/viewport_rule.rs index d0263e1366d..05f5e9be786 100644 --- a/components/style/stylesheets/viewport_rule.rs +++ b/components/style/stylesheets/viewport_rule.rs @@ -370,7 +370,7 @@ impl ViewportRule { } Err(err) => { let error = ContextualParseError::UnsupportedViewportDescriptorDeclaration(err.slice, err.error); - context.log_css_error(error_context, err.location, error); + context.log_css_error(error_context, parser.input.current_source_location(), error); } } } diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 4214bc605fa..b3e48809f0b 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -324,13 +324,13 @@ fn test_report_error_stylesheet() { invalid: true; } @media (min-width: invalid 1000px) {} - @font-face { src: url(), invalid, url() } + @font-face { src: url(), invalid, url(); } @counter-style foo { symbols: a 0invalid b } @font-feature-values Sans Sans { @foo {} @swash { foo: 1 invalid 2 } } @invalid; @media screen { @invalid; } @supports (color: green) and invalid and (margin: 0) {} - @keyframes foo { from invalid {} to { margin: 0 invalid 0 } } + @keyframes foo { from invalid {} to { margin: 0 invalid 0; } } @viewport { width: 320px invalid auto; } "; let url = ServoUrl::parse("about::test").unwrap(); @@ -342,26 +342,26 @@ fn test_report_error_stylesheet() { None, &error_reporter, QuirksMode::NoQuirks, 5); error_reporter.assert_messages_contain(&[ - (8, 9, "Unsupported property declaration: 'display: invalid;'"), - (9, 9, "Unsupported property declaration: 'background-image:"), - (10, 9, "Unsupported property declaration: 'invalid: true;'"), - (12, 11, "Invalid media rule"), - (13, 18, "Unsupported @font-face descriptor declaration"), + (8, 26, "Unsupported property declaration: 'display: invalid;'"), + (9, 78, "Unsupported property declaration: 'background-image:"), + (10, 23, "Unsupported property declaration: 'invalid: true;'"), + (12, 40, "Invalid media rule"), + (13, 45, "Unsupported @font-face descriptor declaration"), // When @counter-style is supported, this should be replaced with two errors - (14, 5, "Invalid rule: '@counter-style "), + (14, 25, "Invalid rule: '@counter-style "), // When @font-feature-values is supported, this should be replaced with two errors - (15, 5, "Invalid rule: '@font-feature-values "), + (15, 37, "Invalid rule: '@font-feature-values "), - // FIXME: these two should be consistent - (16, 5, "Invalid rule: '@invalid'"), - (17, 21, "Unsupported rule: '@invalid'"), + // FIXME: the message of these two should be consistent + (16, 14, "Invalid rule: '@invalid'"), + (17, 30, "Unsupported rule: '@invalid'"), - (18, 5, "Invalid rule: '@supports "), - (19, 22, "Invalid keyframe rule: 'from invalid '"), - (19, 43, "Unsupported keyframe property declaration: 'margin: 0 invalid 0 '"), - (20, 17, "Unsupported @viewport descriptor declaration: 'width: 320px invalid auto;'"), + (18, 59, "Invalid rule: '@supports "), + (19, 35, "Invalid keyframe rule: 'from invalid '"), + (19, 63, "Unsupported keyframe property declaration: 'margin: 0 invalid 0;'"), + (20, 43, "Unsupported @viewport descriptor declaration: 'width: 320px invalid auto;'"), ]); assert_eq!(error_reporter.errors.borrow()[0].url, url); @@ -385,7 +385,7 @@ fn test_no_report_unrecognized_vendor_properties() { None, &error_reporter, QuirksMode::NoQuirks, 0); error_reporter.assert_messages_contain(&[ - (4, 9, "Unsupported property declaration: '-moz-background-color: red;'"), + (4, 36, "Unsupported property declaration: '-moz-background-color: red;'"), ]); }