Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the current parser location for CSS error #18808

Merged
merged 9 commits into from Oct 10, 2017

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
  • Loading branch information
SimonSapin committed Oct 10, 2017
commit 056e5995624598575ace9c6647967c7a3b2c7287
@@ -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)
}
}
}
@@ -125,7 +125,7 @@ pub fn parse_font_face_block<R>(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)
}
}
}
@@ -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);
},
}

@@ -1035,7 +1035,6 @@ pub fn parse_one_declaration_into<R>(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<R>(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<R>(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);
}
}
}
@@ -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);
}
}
},
@@ -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.
@@ -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);
}
}
}
@@ -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);
}
}
}
@@ -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);
}
}
}
@@ -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;'"),
]);
}

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.