diff --git a/components/style/stylesheets/mod.rs b/components/style/stylesheets/mod.rs index b3360fa41ab..2d6539c08b4 100644 --- a/components/style/stylesheets/mod.rs +++ b/components/style/stylesheets/mod.rs @@ -46,7 +46,7 @@ pub use self::media_rule::MediaRule; pub use self::namespace_rule::NamespaceRule; pub use self::origin::{Origin, OriginSet, OriginSetIterator, PerOrigin, PerOriginIter}; pub use self::page_rule::PageRule; -pub use self::rule_parser::{State, TopLevelRuleParser}; +pub use self::rule_parser::{State, TopLevelRuleParser, InsertRuleContext}; pub use self::rule_list::{CssRules, CssRulesHelpers}; pub use self::rules_iterator::{AllRules, EffectiveRules}; pub use self::rules_iterator::{NestedRuleIterationCondition, RulesIterator}; @@ -178,12 +178,6 @@ pub enum CssRuleType { Viewport = 15, } -#[allow(missing_docs)] -pub enum SingleRuleParseError { - Syntax, - Hierarchy, -} - #[allow(missing_docs)] pub enum RulesMutateError { Syntax, @@ -192,15 +186,6 @@ pub enum RulesMutateError { InvalidState, } -impl From for RulesMutateError { - fn from(other: SingleRuleParseError) -> Self { - match other { - SingleRuleParseError::Syntax => RulesMutateError::Syntax, - SingleRuleParseError::Hierarchy => RulesMutateError::HierarchyRequest, - } - } -} - impl CssRule { /// Returns the CSSOM rule type of this rule. pub fn rule_type(&self) -> CssRuleType { @@ -236,11 +221,12 @@ impl CssRule { /// Input state is None for a nested rule pub fn parse( css: &str, + insert_rule_context: InsertRuleContext, parent_stylesheet_contents: &StylesheetContents, shared_lock: &SharedRwLock, - state: Option, + state: State, loader: Option<&StylesheetLoader>, - ) -> Result<(Self, State), SingleRuleParseError> { + ) -> Result { let url_data = parent_stylesheet_contents.url_data.read(); let error_reporter = NullReporter; let context = ParserContext::new( @@ -257,28 +243,23 @@ impl CssRule { let mut guard = parent_stylesheet_contents.namespaces.write(); // 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, + context, error_context: ParserErrorContext { error_reporter: &error_reporter, }, shared_lock: &shared_lock, - loader: loader, - state: state, - had_hierarchy_error: false, + loader, + state, + dom_error: None, namespaces: &mut *guard, + insert_rule_context: Some(insert_rule_context), }; parse_one_rule(&mut input, &mut rule_parser) - .map(|result| (result, rule_parser.state)) .map_err(|_| { - if rule_parser.take_had_hierarchy_error() { - SingleRuleParseError::Hierarchy - } else { - SingleRuleParseError::Syntax - } + rule_parser.dom_error.unwrap_or(RulesMutateError::Syntax) }) } } diff --git a/components/style/stylesheets/rule_list.rs b/components/style/stylesheets/rule_list.rs index 18acdd583bc..35edc22b3da 100644 --- a/components/style/stylesheets/rule_list.rs +++ b/components/style/stylesheets/rule_list.rs @@ -13,7 +13,7 @@ use std::fmt::{self, Write}; use str::CssStringWriter; use stylesheets::{CssRule, RulesMutateError}; use stylesheets::loader::StylesheetLoader; -use stylesheets::rule_parser::State; +use stylesheets::rule_parser::{InsertRuleContext, State}; use stylesheets::stylesheet::StylesheetContents; /// A list of CSS rules. @@ -141,7 +141,7 @@ impl CssRulesHelpers for RawOffsetArc> { nested: bool, loader: Option<&StylesheetLoader>, ) -> Result { - let state = { + let new_rule = { let read_guard = lock.read(); let rules = self.read_with(&read_guard); @@ -151,39 +151,33 @@ impl CssRulesHelpers for RawOffsetArc> { } // Computes the parser state at the given index - if nested { - None + let state = if nested { + State::Body } else if index == 0 { - Some(State::Start) + State::Start } else { - rules.0.get(index - 1).map(CssRule::rule_state) - } - }; + rules.0.get(index - 1).map(CssRule::rule_state).unwrap_or(State::Body) + }; - // Step 3, 4 - // XXXManishearth should we also store the namespace map? - let (new_rule, new_state) = - CssRule::parse(&rule, parent_stylesheet_contents, lock, state, loader)?; + let insert_rule_context = InsertRuleContext { + rule_list: &rules.0, + index, + }; + + // Steps 3, 4, 5, 6 + CssRule::parse( + &rule, + insert_rule_context, + parent_stylesheet_contents, + lock, + state, + loader, + )? + }; { let mut write_guard = lock.write(); let rules = self.write_with(&mut write_guard); - // Step 5 - // Computes the maximum allowed parser state at a given index. - let rev_state = rules.0.get(index).map_or(State::Body, CssRule::rule_state); - if new_state > rev_state { - // We inserted a rule too early, e.g. inserting - // a regular style rule before @namespace rules - return Err(RulesMutateError::HierarchyRequest); - } - - // Step 6 - if let CssRule::Namespace(..) = new_rule { - if !rules.only_ns_or_import() { - return Err(RulesMutateError::InvalidState); - } - } - rules.0.insert(index, new_rule.clone()); } diff --git a/components/style/stylesheets/rule_parser.rs b/components/style/stylesheets/rule_parser.rs index 93f0eaab650..4964db5aae1 100644 --- a/components/style/stylesheets/rule_parser.rs +++ b/components/style/stylesheets/rule_parser.rs @@ -19,7 +19,7 @@ use servo_arc::Arc; use shared_lock::{Locked, SharedRwLock}; use str::starts_with_ignore_ascii_case; use style_traits::{ParseError, StyleParseErrorKind}; -use stylesheets::{CssRule, CssRuleType, CssRules, Origin, StylesheetLoader}; +use stylesheets::{CssRule, CssRuleType, CssRules, Origin, RulesMutateError, StylesheetLoader}; use stylesheets::{DocumentRule, FontFeatureValuesRule, KeyframesRule, MediaRule}; use stylesheets::{NamespaceRule, PageRule, StyleRule, SupportsRule, ViewportRule}; use stylesheets::document_rule::DocumentCondition; @@ -31,6 +31,14 @@ use stylesheets::viewport_rule; use values::{CssUrl, CustomIdent, KeyframesName}; use values::computed::font::FamilyName; +/// The information we need particularly to do CSSOM insertRule stuff. +pub struct InsertRuleContext<'a> { + /// The rule list we're about to insert into. + pub rule_list: &'a [CssRule], + /// The index we're about to get inserted at. + pub index: usize, +} + /// The parser for the top-level rules in a stylesheet. pub struct TopLevelRuleParser<'a, R: 'a> { /// The origin of the stylesheet we're parsing. @@ -51,11 +59,13 @@ pub struct TopLevelRuleParser<'a, R: 'a> { /// 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 `take_had_hierarchy_error` is called. - pub had_hierarchy_error: bool, + pub dom_error: Option, /// 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: &'a mut Namespaces, + /// The info we need insert a rule in a list. + pub insert_rule_context: Option>, } impl<'b, R> TopLevelRuleParser<'b, R> { @@ -74,14 +84,43 @@ impl<'b, R> TopLevelRuleParser<'b, R> { 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 take_had_hierarchy_error(&mut self) -> bool { - let had_hierarchy_error = self.had_hierarchy_error; - self.had_hierarchy_error = false; - had_hierarchy_error + /// Checks whether we can parse a rule that would transition us to + /// `new_state`. + /// + /// This is usually a simple branch, but we may need more bookkeeping if + /// doing `insertRule` from CSSOM. + fn check_state(&mut self, new_state: State) -> bool { + if self.state > new_state { + self.dom_error = Some(RulesMutateError::HierarchyRequest); + return false; + } + + let ctx = match self.insert_rule_context { + Some(ref ctx) => ctx, + None => return true, + }; + + let next_rule_state = match ctx.rule_list.get(ctx.index) { + None => return true, + Some(rule) => rule.rule_state(), + }; + + if new_state > next_rule_state { + self.dom_error = Some(RulesMutateError::HierarchyRequest); + return false; + } + + // If there's anything that isn't a namespace rule (or import rule, but + // we checked that already at the beginning), reject with a + // StateError. + if new_state == State::Namespaces && + ctx.rule_list[ctx.index..].iter().any(|r| !matches!(*r, CssRule::Namespace(..))) + { + self.dom_error = Some(RulesMutateError::InvalidState); + return false; + } + + true } } @@ -151,9 +190,7 @@ impl<'a, 'i, R: ParseErrorReporter> AtRuleParser<'i> for TopLevelRuleParser<'a, let location = input.current_source_location(); match_ignore_ascii_case! { &*name, "import" => { - if self.state > State::Imports { - // "@import must be before any rule but @charset" - self.had_hierarchy_error = true; + if !self.check_state(State::Imports) { return Err(input.new_custom_error(StyleParseErrorKind::UnexpectedImportRule)) } @@ -168,9 +205,7 @@ impl<'a, 'i, R: ParseErrorReporter> AtRuleParser<'i> for TopLevelRuleParser<'a, return Ok(AtRuleType::WithoutBlock(prelude)); }, "namespace" => { - if self.state > State::Namespaces { - // "@namespace must be before any rule but @charset and @import" - self.had_hierarchy_error = true; + if !self.check_state(State::Namespaces) { return Err(input.new_custom_error(StyleParseErrorKind::UnexpectedNamespaceRule)) } @@ -190,12 +225,16 @@ impl<'a, 'i, R: ParseErrorReporter> AtRuleParser<'i> for TopLevelRuleParser<'a, // @charset is removed by rust-cssparser if it’s the first rule in the stylesheet // anything left is invalid. "charset" => { - self.had_hierarchy_error = true; + self.dom_error = Some(RulesMutateError::HierarchyRequest); return Err(input.new_custom_error(StyleParseErrorKind::UnexpectedCharsetRule)) } _ => {} } + if !self.check_state(State::Body) { + return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) + } + AtRuleParser::parse_prelude(&mut self.nested(), name, input) } @@ -264,6 +303,10 @@ impl<'a, 'i, R: ParseErrorReporter> QualifiedRuleParser<'i> for TopLevelRulePars &mut self, input: &mut Parser<'i, 't>, ) -> Result> { + if !self.check_state(State::Body) { + return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) + } + QualifiedRuleParser::parse_prelude(&mut self.nested(), input) } diff --git a/components/style/stylesheets/stylesheet.rs b/components/style/stylesheets/stylesheet.rs index 5f10c1dd503..b33875a149c 100644 --- a/components/style/stylesheets/stylesheet.rs +++ b/components/style/stylesheets/stylesheet.rs @@ -368,7 +368,8 @@ impl Stylesheet { context, error_context, state: State::Start, - had_hierarchy_error: false, + dom_error: None, + insert_rule_context: None, namespaces, };