Ignore non-margin properties in @page rule

Extend Servo's @page parsing to match Gecko's CSS 2.2 behavior, where only
margin properties are allowed in an @page rule.  Other properties are ignored.

MozReview-Commit-ID: IPYUlnkLYSb

components/style/keyframes.rs

@@ -18,7 +18,7 @@ use shared_lock::{SharedRwLock, SharedRwLockReadGuard, Locked, ToCssWithGuard};
 use std::fmt;
 use std::sync::Arc;
 use style_traits::ToCss;
-use stylesheets::{MemoryHoleReporter, Stylesheet};
+use stylesheets::{CssRuleType, MemoryHoleReporter, Stylesheet};
 
 /// A number from 0 to 1, indicating the percentage of the animation when this
 /// keyframe should run.
@@ -403,7 +403,7 @@ impl<'a, 'b> DeclarationParser for KeyframeDeclarationParser<'a, 'b> {
 
     fn parse_value(&mut self, name: &str, input: &mut Parser) -> Result<ParsedDeclaration, ()> {
         let id = try!(PropertyId::parse(name.into()));
-        match ParsedDeclaration::parse(id, self.context, input, true) {
+        match ParsedDeclaration::parse(id, self.context, input, true, CssRuleType::Keyframe) {
             Ok(parsed) => {
                 // In case there is still unparsed text in the declaration, we should roll back.
                 if !input.is_exhausted() {

components/style/properties/data.py

@@ -97,7 +97,8 @@ class Longhand(object):
                  need_clone=False, need_index=False, gecko_ffi_name=None, depend_on_viewport_size=False,
                  allowed_in_keyframe_block=True, complex_color=False, cast_type='u8',
                  has_uncacheable_values=False, logical=False, alias=None, extra_prefixes=None, boxed=False,
-                 creates_stacking_context=False, fixpos_cb=False, abspos_cb=False):
+                 creates_stacking_context=False, fixpos_cb=False, abspos_cb=False,
+                 allowed_in_page_rule=False):
         self.name = name
         if not spec:
             raise TypeError("Spec should be specified for %s" % name)
@@ -124,6 +125,7 @@ class Longhand(object):
         self.creates_stacking_context = arg_to_bool(creates_stacking_context)
         self.fixpos_cb = arg_to_bool(fixpos_cb)
         self.abspos_cb = arg_to_bool(abspos_cb)
+        self.allowed_in_page_rule = arg_to_bool(allowed_in_page_rule)
 
         # https://drafts.csswg.org/css-animations/#keyframes
         # > The <declaration-list> inside of <keyframe-block> accepts any CSS property
@@ -154,7 +156,8 @@ class Longhand(object):
 
 class Shorthand(object):
     def __init__(self, name, sub_properties, spec=None, experimental=False, internal=False,
-                 allowed_in_keyframe_block=True, alias=None, extra_prefixes=None):
+                 allowed_in_keyframe_block=True, alias=None, extra_prefixes=None,
+                 allowed_in_page_rule=False):
         self.name = name
         if not spec:
             raise TypeError("Spec should be specified for %s" % name)
@@ -167,6 +170,7 @@ class Shorthand(object):
         self.internal = internal
         self.alias = alias.split() if alias else []
         self.extra_prefixes = extra_prefixes.split() if extra_prefixes else []
+        self.allowed_in_page_rule = arg_to_bool(allowed_in_page_rule)
 
         # https://drafts.csswg.org/css-animations/#keyframes
         # > The <declaration-list> inside of <keyframe-block> accepts any CSS property

components/style/properties/declaration_block.rs

@@ -12,7 +12,7 @@ use error_reporting::ParseErrorReporter;
 use parser::{ParserContext, log_css_error};
 use std::fmt;
 use style_traits::ToCss;
-use stylesheets::{Origin, UrlExtraData};
+use stylesheets::{CssRuleType, Origin, UrlExtraData};
 use super::*;
 #[cfg(feature = "gecko")] use properties::animated_properties::AnimationValueMap;
 
@@ -613,7 +613,7 @@ pub fn parse_style_attribute(input: &str,
                              error_reporter: &ParseErrorReporter)
                              -> PropertyDeclarationBlock {
     let context = ParserContext::new(Origin::Author, url_data, error_reporter);
-    parse_property_declaration_list(&context, &mut Parser::new(input))
+    parse_property_declaration_list(&context, &mut Parser::new(input), CssRuleType::Style)
 }
 
 /// Parse a given property declaration. Can result in multiple
@@ -628,7 +628,7 @@ pub fn parse_one_declaration(id: PropertyId,
                              -> Result<ParsedDeclaration, ()> {
     let context = ParserContext::new(Origin::Author, url_data, error_reporter);
     Parser::new(input).parse_entirely(|parser| {
-        ParsedDeclaration::parse(id, &context, parser, false)
+        ParsedDeclaration::parse(id, &context, parser, false, CssRuleType::Style)
             .map_err(|_| ())
     })
 }
@@ -636,6 +636,7 @@ pub fn parse_one_declaration(id: PropertyId,
 /// A struct to parse property declarations.
 struct PropertyDeclarationParser<'a, 'b: 'a> {
     context: &'a ParserContext<'b>,
+    rule_type: CssRuleType,
 }
 
 
@@ -653,7 +654,7 @@ impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> {
                    -> Result<(ParsedDeclaration, Importance), ()> {
         let id = try!(PropertyId::parse(name.into()));
         let parsed = input.parse_until_before(Delimiter::Bang, |input| {
-            ParsedDeclaration::parse(id, self.context, input, false)
+            ParsedDeclaration::parse(id, self.context, input, false, self.rule_type)
                 .map_err(|_| ())
         })?;
         let importance = match input.try(parse_important) {
@@ -672,11 +673,13 @@ impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> {
 /// Parse a list of property declarations and return a property declaration
 /// block.
 pub fn parse_property_declaration_list(context: &ParserContext,
-                                       input: &mut Parser)
+                                       input: &mut Parser,
+                                       rule_type: CssRuleType)
                                        -> PropertyDeclarationBlock {
     let mut block = PropertyDeclarationBlock::new();
     let parser = PropertyDeclarationParser {
         context: context,
+        rule_type: rule_type,
     };
     let mut iter = DeclarationListParser::new(input, parser);
     while let Some(declaration) = iter.next() {

components/style/properties/longhand/margin.mako.rs

@@ -15,5 +15,6 @@
     ${helpers.predefined_type("margin-%s" % side[0], "LengthOrPercentageOrAuto",
                               "computed::LengthOrPercentageOrAuto::Length(Au(0))",
                               alias=maybe_moz_logical_alias(product, side, "-moz-margin-%s"),
-                              animation_type="normal", logical = side[1], spec = spec)}
+                              animation_type="normal", logical = side[1], spec = spec,
+                              allowed_in_page_rule=True)}
 % endfor

components/style/properties/properties.mako.rs

@@ -32,7 +32,7 @@ use properties::animated_properties::TransitionProperty;
 #[cfg(feature = "servo")] use servo_config::prefs::PREFS;
 use shared_lock::StylesheetGuards;
 use style_traits::ToCss;
-use stylesheets::{Origin, UrlExtraData};
+use stylesheets::{CssRuleType, Origin, UrlExtraData};
 #[cfg(feature = "servo")] use values::Either;
 use values::{HasViewportPercentage, computed};
 use cascade_info::CascadeInfo;
@@ -977,8 +977,12 @@ impl ParsedDeclaration {
     /// to Importance::Normal. Parsing Importance values is the job of PropertyDeclarationParser,
     /// we only set them here so that we don't have to reallocate
     pub fn parse(id: PropertyId, context: &ParserContext, input: &mut Parser,
-                 in_keyframe_block: bool)
+                 in_keyframe_block: bool, rule_type: CssRuleType)
                  -> Result<ParsedDeclaration, PropertyDeclarationParseError> {
+        debug_assert!(rule_type == CssRuleType::Keyframe ||
+                      rule_type == CssRuleType::Page ||
+                      rule_type == CssRuleType::Style,
+                      "Declarations are only expected inside a keyframe, page, or style rule.");
         match id {
             PropertyId::Custom(name) => {
                 let value = match input.try(|i| CSSWideKeyword::parse(context, i)) {
@@ -1004,6 +1008,11 @@ impl ParsedDeclaration {
                                 return Err(PropertyDeclarationParseError::UnknownProperty)
                             }
                         % endif
+                        % if not property.allowed_in_page_rule:
+                            if rule_type == CssRuleType::Page {
+                                return Err(PropertyDeclarationParseError::NotAllowedInPageRule)
+                            }
+                        % endif
 
                         ${property_pref_check(property)}
 
@@ -1032,6 +1041,11 @@ impl ParsedDeclaration {
                             return Err(PropertyDeclarationParseError::UnknownProperty)
                         }
                     % endif
+                    % if not shorthand.allowed_in_page_rule:
+                        if rule_type == CssRuleType::Page {
+                            return Err(PropertyDeclarationParseError::NotAllowedInPageRule)
+                        }
+                    % endif
 
                     ${property_pref_check(shorthand)}
 
@@ -1105,6 +1119,8 @@ pub enum PropertyDeclarationParseError {
     ///
     /// See: https://drafts.csswg.org/css-animations/#keyframes
     AnimationPropertyInKeyframeBlock,
+    /// The property is not allowed within a page rule.
+    NotAllowedInPageRule,
 }
 
 impl fmt::Debug for PropertyDeclaration {

components/style/properties/shorthand/margin.mako.rs

@@ -5,4 +5,5 @@
 <%namespace name="helpers" file="/helpers.mako.rs" />
 
 ${helpers.four_sides_shorthand("margin", "margin-%s", "specified::LengthOrPercentageOrAuto::parse",
-                               spec="https://drafts.csswg.org/css-box/#propdef-margin")}
+                               spec="https://drafts.csswg.org/css-box/#propdef-margin",
+                               allowed_in_page_rule=True)}

components/style/stylesheets.rs

@@ -262,6 +262,7 @@ pub enum CssRule {
 }
 
 #[allow(missing_docs)]
+#[derive(PartialEq, Eq, Copy, Clone)]
 pub enum CssRuleType {
     // https://drafts.csswg.org/cssom/#the-cssrule-interface
     Style               = 1,
@@ -1114,7 +1115,7 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> {
                 }))))
             }
             AtRulePrelude::Page => {
-                let declarations = parse_property_declaration_list(self.context, input);
+                let declarations = parse_property_declaration_list(self.context, input, CssRuleType::Page);
                 Ok(CssRule::Page(Arc::new(self.shared_lock.wrap(PageRule(
                     Arc::new(self.shared_lock.wrap(declarations))
                 )))))
@@ -1137,7 +1138,7 @@ impl<'a, 'b> QualifiedRuleParser for NestedRuleParser<'a, 'b> {
 
     fn parse_block(&mut self, prelude: SelectorList<SelectorImpl>, input: &mut Parser)
                    -> Result<CssRule, ()> {
-        let declarations = parse_property_declaration_list(self.context, input);
+        let declarations = parse_property_declaration_list(self.context, input, CssRuleType::Style);
         Ok(CssRule::Style(Arc::new(self.shared_lock.wrap(StyleRule {
             selectors: prelude,
             block: Arc::new(self.shared_lock.wrap(declarations))

components/style/supports.rs

@@ -9,6 +9,7 @@ use parser::ParserContext;
 use properties::{PropertyId, ParsedDeclaration};
 use std::fmt;
 use style_traits::ToCss;
+use stylesheets::CssRuleType;
 
 #[derive(Debug)]
 /// An @supports condition
@@ -211,7 +212,7 @@ impl Declaration {
             return false
         };
         let mut input = Parser::new(&self.val);
-        let res = ParsedDeclaration::parse(id, cx, &mut input, /* in_keyframe */ false);
+        let res = ParsedDeclaration::parse(id, cx, &mut input, /* in_keyframe */ false, CssRuleType::Style);
         let _ = input.try(parse_important);
         res.is_ok() && input.is_exhausted()
     }

ports/geckolib/glue.rs

@@ -79,7 +79,7 @@ use style::selector_parser::PseudoElementCascadeType;
 use style::sequential;
 use style::shared_lock::{SharedRwLock, SharedRwLockReadGuard, StylesheetGuards, ToCssWithGuard, Locked};
 use style::string_cache::Atom;
-use style::stylesheets::{CssRule, CssRules, ImportRule, MediaRule, NamespaceRule};
+use style::stylesheets::{CssRule, CssRules, CssRuleType, ImportRule, MediaRule, NamespaceRule};
 use style::stylesheets::{Origin, Stylesheet, StyleRule};
 use style::stylesheets::StylesheetLoader as StyleStylesheetLoader;
 use style::supports::parse_condition_or_declaration;
@@ -922,7 +922,7 @@ pub extern "C" fn Servo_ParseProperty(property: *const nsACString, value: *const
     let reporter = StdoutErrorReporter;
     let context = ParserContext::new(Origin::Author, url_data, &reporter);
 
-    match ParsedDeclaration::parse(id, &context, &mut Parser::new(value), false) {
+    match ParsedDeclaration::parse(id, &context, &mut Parser::new(value), false, CssRuleType::Style) {
         Ok(parsed) => {
             let global_style_data = &*GLOBAL_STYLE_DATA;
             let mut block = PropertyDeclarationBlock::new();

tests/unit/style/properties/serialization.rs

@@ -10,7 +10,7 @@ use style::parser::ParserContext;
 use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, Importance, PropertyId};
 use style::properties::longhands::outline_color::computed_value::T as ComputedColor;
 use style::properties::parse_property_declaration_list;
-use style::stylesheets::Origin;
+use style::stylesheets::{CssRuleType, Origin};
 use style::values::{RGBA, Auto};
 use style::values::specified::{BorderStyle, BorderWidth, CSSColor, Length, NoCalcLength};
 use style::values::specified::{LengthOrPercentage, LengthOrPercentageOrAuto, LengthOrPercentageOrAutoOrContent};
@@ -23,7 +23,7 @@ fn parse_declaration_block(css_properties: &str) -> PropertyDeclarationBlock {
     let reporter = CSSErrorReporterTest;
     let context = ParserContext::new(Origin::Author, &url, &reporter);
     let mut parser = Parser::new(css_properties);
-    parse_property_declaration_list(&context, &mut parser)
+    parse_property_declaration_list(&context, &mut parser, CssRuleType::Style)
 }
 
 #[test]