Use the current parser location for CSS error

… 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
This commit is contained in:
Simon Sapin 2017-09-21 01:44:53 +02:00
parent bc0903c928
commit 056e599562
10 changed files with 34 additions and 31 deletions

View file

@ -68,7 +68,7 @@ pub fn parse_counter_style_body<'i, 't, R>(name: CustomIdent,
while let Some(declaration) = iter.next() { while let Some(declaration) = iter.next() {
if let Err(err) = declaration { if let Err(err) = declaration {
let error = ContextualParseError::UnsupportedCounterStyleDescriptorDeclaration(err.slice, err.error); 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)
} }
} }
} }

View file

@ -125,7 +125,7 @@ pub fn parse_font_face_block<R>(context: &ParserContext,
while let Some(declaration) = iter.next() { while let Some(declaration) = iter.next() {
if let Err(err) = declaration { if let Err(err) = declaration {
let error = ContextualParseError::UnsupportedFontFaceDescriptor(err.slice, err.error); 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)
} }
} }
} }

View file

@ -257,7 +257,6 @@ where
let mut media_queries = vec![]; let mut media_queries = vec![];
loop { loop {
let start_position = input.position(); let start_position = input.position();
let start_location = input.current_source_location();
match input.parse_until_before(Delimiter::Comma, |i| MediaQuery::parse(context, i)) { match input.parse_until_before(Delimiter::Comma, |i| MediaQuery::parse(context, i)) {
Ok(mq) => { Ok(mq) => {
media_queries.push(mq); media_queries.push(mq);
@ -267,7 +266,7 @@ where
let error = ContextualParseError::InvalidMediaRule( let error = ContextualParseError::InvalidMediaRule(
input.slice_from(start_position), err); input.slice_from(start_position), err);
let error_context = ParserErrorContext { error_reporter }; 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);
}, },
} }

View file

@ -1035,7 +1035,6 @@ pub fn parse_one_declaration_into<R>(declarations: &mut SourcePropertyDeclaratio
let mut input = ParserInput::new(input); let mut input = ParserInput::new(input);
let mut parser = Parser::new(&mut input); let mut parser = Parser::new(&mut input);
let start_position = parser.position(); let start_position = parser.position();
let start_location = parser.current_source_location();
parser.parse_entirely(|parser| { parser.parse_entirely(|parser| {
let name = id.name().into(); let name = id.name().into();
PropertyDeclaration::parse_into(declarations, id, name, &context, parser) PropertyDeclaration::parse_into(declarations, id, name, &context, parser)
@ -1044,7 +1043,7 @@ pub fn parse_one_declaration_into<R>(declarations: &mut SourcePropertyDeclaratio
let error = ContextualParseError::UnsupportedPropertyDeclaration( let error = ContextualParseError::UnsupportedPropertyDeclaration(
parser.slice_from(start_position), err); parser.slice_from(start_position), err);
let error_context = ParserErrorContext { error_reporter: error_reporter }; 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<R>(context: &ParserContext,
} }
let error = ContextualParseError::UnsupportedPropertyDeclaration(err.slice, err.error); 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);
} }
} }
} }

View file

@ -273,7 +273,8 @@ macro_rules! font_feature_values_blocks {
while let Some(result) = iter.next() { while let Some(result) = iter.next() {
if let Err(err) = result { if let Err(err) = result {
let error = ContextualParseError::UnsupportedRule(err.slice, err.error); 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 { if let Err(err) = declaration {
let error = ContextualParseError::UnsupportedKeyframePropertyDeclaration( let error = ContextualParseError::UnsupportedKeyframePropertyDeclaration(
err.slice, err.error); 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);
} }
} }
}, },

View file

@ -526,7 +526,7 @@ impl<'a, 'i, R: ParseErrorReporter> QualifiedRuleParser<'i> for KeyframeListPars
}, },
Err(e) => { Err(e) => {
let error = ContextualParseError::InvalidKeyframeRule(input.slice_from(start_position), e.clone()); 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) Err(e)
} }
} }
@ -555,7 +555,7 @@ impl<'a, 'i, R: ParseErrorReporter> QualifiedRuleParser<'i> for KeyframeListPars
Err(err) => { Err(err) => {
iter.parser.declarations.clear(); iter.parser.declarations.clear();
let error = ContextualParseError::UnsupportedKeyframePropertyDeclaration(err.slice, err.error); 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. // `parse_important` is not called here, `!important` is not allowed in keyframe blocks.

View file

@ -333,7 +333,8 @@ impl<'a, 'b, R: ParseErrorReporter> NestedRuleParser<'a, 'b, R> {
Ok(rule) => rules.push(rule), Ok(rule) => rules.push(rule),
Err(err) => { Err(err) => {
let error = ContextualParseError::UnsupportedRule(err.slice, err.error); 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);
} }
} }
} }

View file

@ -396,8 +396,9 @@ impl Stylesheet {
}, },
Err(err) => { Err(err) => {
let error = ContextualParseError::InvalidRule(err.slice, err.error); 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, iter.parser.context.log_css_error(&iter.parser.error_context,
err.location, error); location, error);
} }
} }
} }

View file

@ -370,7 +370,7 @@ impl ViewportRule {
} }
Err(err) => { Err(err) => {
let error = ContextualParseError::UnsupportedViewportDescriptorDeclaration(err.slice, err.error); 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);
} }
} }
} }

View file

@ -324,13 +324,13 @@ fn test_report_error_stylesheet() {
invalid: true; invalid: true;
} }
@media (min-width: invalid 1000px) {} @media (min-width: invalid 1000px) {}
@font-face { src: url(), invalid, url() } @font-face { src: url(), invalid, url(); }
@counter-style foo { symbols: a 0invalid b } @counter-style foo { symbols: a 0invalid b }
@font-feature-values Sans Sans { @foo {} @swash { foo: 1 invalid 2 } } @font-feature-values Sans Sans { @foo {} @swash { foo: 1 invalid 2 } }
@invalid; @invalid;
@media screen { @invalid; } @media screen { @invalid; }
@supports (color: green) and invalid and (margin: 0) {} @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; } @viewport { width: 320px invalid auto; }
"; ";
let url = ServoUrl::parse("about::test").unwrap(); let url = ServoUrl::parse("about::test").unwrap();
@ -342,26 +342,26 @@ fn test_report_error_stylesheet() {
None, &error_reporter, QuirksMode::NoQuirks, 5); None, &error_reporter, QuirksMode::NoQuirks, 5);
error_reporter.assert_messages_contain(&[ error_reporter.assert_messages_contain(&[
(8, 9, "Unsupported property declaration: 'display: invalid;'"), (8, 26, "Unsupported property declaration: 'display: invalid;'"),
(9, 9, "Unsupported property declaration: 'background-image:"), (9, 78, "Unsupported property declaration: 'background-image:"),
(10, 9, "Unsupported property declaration: 'invalid: true;'"), (10, 23, "Unsupported property declaration: 'invalid: true;'"),
(12, 11, "Invalid media rule"), (12, 40, "Invalid media rule"),
(13, 18, "Unsupported @font-face descriptor declaration"), (13, 45, "Unsupported @font-face descriptor declaration"),
// When @counter-style is supported, this should be replaced with two errors // 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 // 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 // FIXME: the message of these two should be consistent
(16, 5, "Invalid rule: '@invalid'"), (16, 14, "Invalid rule: '@invalid'"),
(17, 21, "Unsupported rule: '@invalid'"), (17, 30, "Unsupported rule: '@invalid'"),
(18, 5, "Invalid rule: '@supports "), (18, 59, "Invalid rule: '@supports "),
(19, 22, "Invalid keyframe rule: 'from invalid '"), (19, 35, "Invalid keyframe rule: 'from invalid '"),
(19, 43, "Unsupported keyframe property declaration: 'margin: 0 invalid 0 '"), (19, 63, "Unsupported keyframe property declaration: 'margin: 0 invalid 0;'"),
(20, 17, "Unsupported @viewport descriptor declaration: 'width: 320px invalid auto;'"), (20, 43, "Unsupported @viewport descriptor declaration: 'width: 320px invalid auto;'"),
]); ]);
assert_eq!(error_reporter.errors.borrow()[0].url, url); 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); None, &error_reporter, QuirksMode::NoQuirks, 0);
error_reporter.assert_messages_contain(&[ error_reporter.assert_messages_contain(&[
(4, 9, "Unsupported property declaration: '-moz-background-color: red;'"), (4, 36, "Unsupported property declaration: '-moz-background-color: red;'"),
]); ]);
} }