Bug 1384275 - style: Don't remain in an invalid state when encountering an at-rule in the wrong place. r?emilio
Currently, attempting to parse an at-rule that is out of place, such as
an @import rule after a regular style rule, will cause the parser state
to be set to Invalid. This will cause any following at-rule to be
rejected until we encounter a regular style rule, at which point we'll
go back to the Body state. There's nothing in the CSS specs about
needing to reject all following at-rules (or, as the comment above
Invalid says, ignoring the entire rest of the style sheet).
MozReview-Commit-ID: LS3Vw0DNeq6
--- a/servo/components/style/stylesheets/mod.rs
+++ b/servo/components/style/stylesheets/mod.rs
@@ -250,28 +250,29 @@ impl CssRule {
// nested rules are in the body state
let state = state.unwrap_or(State::Body);
let mut rule_parser = TopLevelRuleParser {
stylesheet_origin: parent_stylesheet_contents.origin,
context: context,
shared_lock: &shared_lock,
loader: loader,
state: state,
+ had_hierarchy_error: false,
namespaces: Some(&mut *guard),
};
- match parse_one_rule(&mut input, &mut rule_parser) {
- Ok(result) => Ok((result, rule_parser.state)),
- Err(_) => {
- Err(match rule_parser.state {
- State::Invalid => SingleRuleParseError::Hierarchy,
- _ => SingleRuleParseError::Syntax,
- })
- }
- }
+ parse_one_rule(&mut input, &mut rule_parser)
+ .map(|result| (result, rule_parser.state))
+ .map_err(|_| {
+ if rule_parser.get_and_reset_had_hierarchy_error() {
+ SingleRuleParseError::Hierarchy
+ } else {
+ SingleRuleParseError::Syntax
+ }
+ })
}
}
impl DeepCloneWithLock for CssRule {
/// Deep clones this CssRule.
fn deep_clone_with_lock(
&self,
lock: &SharedRwLock,
--- a/servo/components/style/stylesheets/rule_parser.rs
+++ b/servo/components/style/stylesheets/rule_parser.rs
@@ -42,16 +42,20 @@ pub struct TopLevelRuleParser<'a> {
pub shared_lock: &'a SharedRwLock,
/// A reference to a stylesheet loader if applicable, for `@import` rules.
pub loader: Option<&'a StylesheetLoader>,
/// The parser context. This initially won't contain any namespaces, but
/// will be populated after parsing namespace rules, if any.
pub context: ParserContext<'a>,
/// The current state of the parser.
pub state: State,
+ /// Whether we have tried to parse was invalid due to being in the wrong
+ /// place (e.g. an @import rule was found while in the `Body` state). Reset
+ /// to `false` when `get_and_reset_had_hierarchy_error` is called.
+ pub had_hierarchy_error: bool,
/// The namespace map we use for parsing. Needs to start as `Some()`, and
/// will be taken out after parsing namespace rules, and that reference will
/// be moved to `ParserContext`.
pub namespaces: Option<&'a mut Namespaces>,
}
impl<'b> TopLevelRuleParser<'b> {
fn nested<'a: 'b>(&'a self) -> NestedRuleParser<'a, 'b> {
@@ -66,32 +70,39 @@ impl<'b> TopLevelRuleParser<'b> {
pub fn context(&self) -> &ParserContext {
&self.context
}
/// Returns the current state of the parser.
pub fn state(&self) -> State {
self.state
}
+
+ /// Returns whether we previously tried to parse a rule that was invalid
+ /// due to being in the wrong place (e.g. an @import rule was found after
+ /// a regular style rule). The state of this flag is reset when this
+ /// function is called.
+ pub fn get_and_reset_had_hierarchy_error(&mut self) -> bool {
+ let had_hierarchy_error = self.had_hierarchy_error;
+ self.had_hierarchy_error = false;
+ had_hierarchy_error
+ }
}
/// The current state of the parser.
#[derive(Eq, PartialEq, Ord, PartialOrd, Copy, Clone)]
pub enum State {
/// We haven't started parsing rules.
Start = 1,
/// We're parsing `@import` rules.
Imports = 2,
/// We're parsing `@namespace` rules.
Namespaces = 3,
/// We're parsing the main body of the stylesheet.
Body = 4,
- /// We've found an invalid state (as, a namespace rule after style rules),
- /// and the rest of the stylesheet should be ignored.
- Invalid = 5,
}
#[derive(Clone, Debug)]
#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
/// Vendor prefix.
pub enum VendorPrefix {
/// -moz prefix.
Moz,
@@ -148,18 +159,18 @@ impl<'a, 'i> AtRuleParser<'i> for TopLev
name: CowRcStr<'i>,
input: &mut Parser<'i, 't>
) -> Result<AtRuleType<AtRulePrelude, CssRule>, ParseError<'i>> {
let location = get_location_with_offset(input.current_source_location(),
self.context.line_number_offset);
match_ignore_ascii_case! { &*name,
"import" => {
if self.state > State::Imports {
- self.state = State::Invalid;
// "@import must be before any rule but @charset"
+ self.had_hierarchy_error = true;
return Err(StyleParseError::UnexpectedImportRule.into())
}
self.state = State::Imports;
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);
@@ -175,18 +186,18 @@ impl<'a, 'i> AtRuleParser<'i> for TopLev
&self.shared_lock,
media,
);
return Ok(AtRuleType::WithoutBlock(CssRule::Import(import_rule)))
},
"namespace" => {
if self.state > State::Namespaces {
- self.state = State::Invalid;
// "@namespace must be before any rule but @charset and @import"
+ self.had_hierarchy_error = true;
return Err(StyleParseError::UnexpectedNamespaceRule.into())
}
self.state = State::Namespaces;
let prefix_result = input.try(|i| i.expect_ident_cloned());
let maybe_namespace = match input.expect_url_or_string() {
Ok(url_or_string) => url_or_string,
Err(BasicParseError::UnexpectedToken(t)) =>
@@ -216,24 +227,22 @@ impl<'a, 'i> AtRuleParser<'i> for TopLev
prefix: opt_prefix,
url: url,
source_location: location,
})
))))
},
// @charset is removed by rust-cssparser if it’s the first rule in the stylesheet
// anything left is invalid.
- "charset" => return Err(StyleParseError::UnexpectedCharsetRule.into()),
+ "charset" => {
+ self.had_hierarchy_error = true;
+ return Err(StyleParseError::UnexpectedCharsetRule.into())
+ }
_ => {}
}
- // Don't allow starting with an invalid state
- if self.state > State::Body {
- self.state = State::Invalid;
- return Err(StyleParseError::UnspecifiedError.into());
- }
self.state = State::Body;
// "Freeze" the namespace map (no more namespace rules can be parsed
// after this point), and stick it in the context.
if self.namespaces.is_some() {
let namespaces = &*self.namespaces.take().unwrap();
self.context.namespaces = Some(namespaces);
}
--- a/servo/components/style/stylesheets/stylesheet.rs
+++ b/servo/components/style/stylesheets/stylesheet.rs
@@ -332,16 +332,17 @@ impl Stylesheet {
);
let rule_parser = TopLevelRuleParser {
stylesheet_origin: origin,
shared_lock: shared_lock,
loader: stylesheet_loader,
context: context,
state: State::Start,
+ had_hierarchy_error: false,
namespaces: Some(namespaces),
};
input.look_for_viewport_percentages();
{
let mut iter =
RuleListParser::new_for_stylesheet(&mut input, rule_parser);