diff --git a/components/style/keyframes.rs b/components/style/keyframes.rs index a0000c6e1770..3656ccee24de 100644 --- a/components/style/keyframes.rs +++ b/components/style/keyframes.rs @@ -2,10 +2,12 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use cssparser::{AtRuleParser, Delimiter, Parser, QualifiedRuleParser, RuleListParser}; +use cssparser::{AtRuleParser, Parser, QualifiedRuleParser, RuleListParser}; +use cssparser::{DeclarationListParser, DeclarationParser}; use parser::{ParserContext, log_css_error}; +use properties::PropertyDeclaration; +use properties::PropertyDeclarationParseResult; use properties::animated_properties::TransitionProperty; -use properties::{PropertyDeclaration, parse_property_declaration_list}; use std::sync::Arc; /// A number from 1 to 100, indicating the percentage of the animation where @@ -238,12 +240,49 @@ impl<'a> QualifiedRuleParser for KeyframeListParser<'a> { fn parse_block(&self, prelude: Self::Prelude, input: &mut Parser) -> Result { + let mut declarations = Vec::new(); + let parser = KeyframeDeclarationParser { + context: self.context, + }; + let mut iter = DeclarationListParser::new(input, parser); + while let Some(declaration) = iter.next() { + match declaration { + Ok(d) => declarations.extend(d), + Err(range) => { + let pos = range.start; + let message = format!("Unsupported keyframe property declaration: '{}'", + iter.input.slice(range)); + log_css_error(iter.input, pos, &*message, self.context); + } + } + // `parse_important` is not called here, `!important` is not allowed in keyframe blocks. + } Ok(Keyframe { selector: prelude, - // FIXME: needs parsing different from parse_property_declaration_list: - // https://drafts.csswg.org/css-animations/#keyframes - // Paragraph "The inside of ..." - declarations: parse_property_declaration_list(self.context, input).normal, + declarations: Arc::new(declarations), }) } } + +struct KeyframeDeclarationParser<'a, 'b: 'a> { + context: &'a ParserContext<'b>, +} + +/// Default methods reject all at rules. +impl<'a, 'b> AtRuleParser for KeyframeDeclarationParser<'a, 'b> { + type Prelude = (); + type AtRule = Vec; +} + +impl<'a, 'b> DeclarationParser for KeyframeDeclarationParser<'a, 'b> { + type Declaration = Vec; + + fn parse_value(&self, name: &str, input: &mut Parser) -> Result, ()> { + let mut results = Vec::new(); + match PropertyDeclaration::parse(name, self.context, input, &mut results, true) { + PropertyDeclarationParseResult::ValidOrIgnoredDeclaration => {} + _ => return Err(()) + } + Ok(results) + } +} diff --git a/components/style/properties/data.py b/components/style/properties/data.py index 400b054ba74b..4ae14a111773 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -65,7 +65,8 @@ def maybe_cast(self, type_str): class Longhand(object): def __init__(self, style_struct, name, animatable=None, derived_from=None, keyword=None, predefined_type=None, custom_cascade=False, experimental=False, internal=False, - need_clone=False, need_index=False, gecko_ffi_name=None, depend_on_viewport_size=False): + need_clone=False, need_index=False, gecko_ffi_name=None, depend_on_viewport_size=False, + allowed_in_keyframe_block=True): self.name = name self.keyword = keyword self.predefined_type = predefined_type @@ -80,6 +81,13 @@ def __init__(self, style_struct, name, animatable=None, derived_from=None, keywo self.depend_on_viewport_size = depend_on_viewport_size self.derived_from = (derived_from or "").split() + # https://drafts.csswg.org/css-animations/#keyframes + # > The inside of accepts any CSS property + # > except those defined in this specification, + # > but does accept the `animation-play-state` property and interprets it specially. + self.allowed_in_keyframe_block = allowed_in_keyframe_block \ + and allowed_in_keyframe_block != "False" + # This is done like this since just a plain bool argument seemed like # really random. if animatable is None: @@ -98,7 +106,8 @@ def __init__(self, style_struct, name, animatable=None, derived_from=None, keywo class Shorthand(object): - def __init__(self, name, sub_properties, experimental=False, internal=False): + def __init__(self, name, sub_properties, experimental=False, internal=False, + allowed_in_keyframe_block=True): self.name = name self.ident = to_rust_ident(name) self.camel_case = to_camel_case(self.ident) @@ -107,6 +116,13 @@ def __init__(self, name, sub_properties, experimental=False, internal=False): self.sub_properties = sub_properties self.internal = internal + # https://drafts.csswg.org/css-animations/#keyframes + # > The inside of accepts any CSS property + # > except those defined in this specification, + # > but does accept the `animation-play-state` property and interprets it specially. + self.allowed_in_keyframe_block = allowed_in_keyframe_block \ + and allowed_in_keyframe_block != "False" + class Method(object): def __init__(self, name, return_type=None, arg_types=None, is_mut=False): diff --git a/components/style/properties/longhand/box.mako.rs b/components/style/properties/longhand/box.mako.rs index 492edf3d97ba..6aaa615c7de1 100644 --- a/components/style/properties/longhand/box.mako.rs +++ b/components/style/properties/longhand/box.mako.rs @@ -636,7 +636,8 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", <%helpers:longhand name="animation-name" need_index="True" - animatable="False"> + animatable="False", + allowed_in_keyframe_block="False"> use values::computed::ComputedValueAsSpecified; use values::NoViewportPercentage; @@ -699,7 +700,8 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", <%helpers:longhand name="animation-duration" need_index="True" - animatable="False"> + animatable="False", + allowed_in_keyframe_block="False"> pub use super::transition_duration::computed_value; pub use super::transition_duration::{get_initial_value, get_initial_single_value}; pub use super::transition_duration::{parse, parse_one}; @@ -709,7 +711,8 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", <%helpers:longhand name="animation-timing-function" need_index="True" - animatable="False"> + animatable="False", + allowed_in_keyframe_block="False"> pub use super::transition_timing_function::computed_value; pub use super::transition_timing_function::{get_initial_value, get_initial_single_value}; pub use super::transition_timing_function::{parse, parse_one}; @@ -719,7 +722,8 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", <%helpers:longhand name="animation-iteration-count" need_index="True" - animatable="False"> + animatable="False", + allowed_in_keyframe_block="False"> use values::computed::ComputedValueAsSpecified; use values::NoViewportPercentage; @@ -804,22 +808,28 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", ${helpers.keyword_list("animation-direction", "normal reverse alternate alternate-reverse", need_index=True, - animatable=False)} + animatable=False, + allowed_in_keyframe_block=False)} +// animation-play-state is the exception to the rule for allowed_in_keyframe_block: +// https://drafts.csswg.org/css-animations/#keyframes ${helpers.keyword_list("animation-play-state", "running paused", need_clone=True, need_index=True, - animatable=False)} + animatable=False, + allowed_in_keyframe_block=True)} ${helpers.keyword_list("animation-fill-mode", "none forwards backwards both", need_index=True, - animatable=False)} + animatable=False, + allowed_in_keyframe_block=False)} <%helpers:longhand name="animation-delay" need_index="True" - animatable="False"> + animatable="False", + allowed_in_keyframe_block="False"> pub use super::transition_duration::computed_value; pub use super::transition_duration::{get_initial_value, get_initial_single_value}; pub use super::transition_duration::{parse, parse_one}; diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 5db9379fc134..d1ad214810b7 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -517,7 +517,7 @@ pub fn parse_one_declaration(name: &str, input: &str, base_url: &Url, error_repo -> Result, ()> { let context = ParserContext::new_with_extra_data(Origin::Author, base_url, error_reporter, extra_data); let mut results = vec![]; - match PropertyDeclaration::parse(name, &context, &mut Parser::new(input), &mut results) { + match PropertyDeclaration::parse(name, &context, &mut Parser::new(input), &mut results, false) { PropertyDeclarationParseResult::ValidOrIgnoredDeclaration => Ok(results), _ => Err(()) } @@ -542,7 +542,7 @@ impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> { -> Result<(Vec, Importance), ()> { let mut results = vec![]; try!(input.parse_until_before(Delimiter::Bang, |input| { - match PropertyDeclaration::parse(name, self.context, input, &mut results) { + match PropertyDeclaration::parse(name, self.context, input, &mut results, false) { PropertyDeclarationParseResult::ValidOrIgnoredDeclaration => Ok(()), _ => Err(()) } @@ -832,6 +832,7 @@ pub enum PropertyDeclarationParseResult { UnknownProperty, ExperimentalProperty, InvalidValue, + AnimationPropertyInKeyframeBlock, ValidOrIgnoredDeclaration, } @@ -984,8 +985,16 @@ impl PropertyDeclaration { } } + /// The `in_keyframe_block` parameter controls this: + /// + /// https://drafts.csswg.org/css-animations/#keyframes + /// > The inside of accepts any CSS property + /// > except those defined in this specification, + /// > but does accept the `animation-play-state` property and interprets it specially. pub fn parse(name: &str, context: &ParserContext, input: &mut Parser, - result_list: &mut Vec) -> PropertyDeclarationParseResult { + result_list: &mut Vec, + in_keyframe_block: bool) + -> PropertyDeclarationParseResult { if let Ok(name) = ::custom_properties::parse_name(name) { let value = match input.try(CSSWideKeyword::parse) { Ok(CSSWideKeyword::UnsetKeyword) | // Custom properties are alawys inherited @@ -1003,6 +1012,11 @@ impl PropertyDeclaration { % for property in data.longhands: % if not property.derived_from: "${property.name}" => { + % if not property.allowed_in_keyframe_block: + if in_keyframe_block { + return PropertyDeclarationParseResult::AnimationPropertyInKeyframeBlock + } + % endif % if property.internal: if context.stylesheet_origin != Origin::UserAgent { return PropertyDeclarationParseResult::UnknownProperty @@ -1028,6 +1042,11 @@ impl PropertyDeclaration { % endfor % for shorthand in data.shorthands: "${shorthand.name}" => { + % if not shorthand.allowed_in_keyframe_block: + if in_keyframe_block { + return PropertyDeclarationParseResult::AnimationPropertyInKeyframeBlock + } + % endif % if shorthand.internal: if context.stylesheet_origin != Origin::UserAgent { return PropertyDeclarationParseResult::UnknownProperty diff --git a/components/style/properties/shorthand/box.mako.rs b/components/style/properties/shorthand/box.mako.rs index 6a502276f4e8..0825a8fc3144 100644 --- a/components/style/properties/shorthand/box.mako.rs +++ b/components/style/properties/shorthand/box.mako.rs @@ -152,7 +152,8 @@ macro_rules! try_parse_one { sub_properties="animation-name animation-duration animation-timing-function animation-delay animation-iteration-count animation-direction - animation-fill-mode animation-play-state"> + animation-fill-mode animation-play-state" + allowed_in_keyframe_block="False"> use properties::longhands::{animation_name, animation_duration, animation_timing_function}; use properties::longhands::{animation_delay, animation_iteration_count, animation_direction}; use properties::longhands::{animation_fill_mode, animation_play_state}; diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 5f326ba52e5c..2eb5f9f32607 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -13,6 +13,7 @@ use style::error_reporting::ParseErrorReporter; use style::keyframes::{Keyframe, KeyframeSelector, KeyframePercentage}; use style::parser::ParserContextExtraData; use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, DeclaredValue, longhands}; +use style::properties::longhands::animation_play_state; use style::stylesheets::{Stylesheet, CSSRule, StyleRule, KeyframesRule, Origin}; use style::values::specified::{LengthOrPercentageOrAuto, Percentage}; use url::Url; @@ -27,7 +28,12 @@ fn test_parse_stylesheet() { #d1 > .ok { background: blue; } @keyframes foo { from { width: 0% } - to { width: 100%} + to { + width: 100%; + width: 50% !important; /* !important not allowed here */ + animation-name: 'foo'; /* animation properties not allowed here */ + animation-play-state: running; /* … except animation-play-state */ + } }"; let url = Url::parse("about::test").unwrap(); let stylesheet = Stylesheet::from_str(css, url, Origin::UserAgent, @@ -187,6 +193,9 @@ fn test_parse_stylesheet() { declarations: Arc::new(vec![ PropertyDeclaration::Width(DeclaredValue::Value( LengthOrPercentageOrAuto::Percentage(Percentage(1.)))), + PropertyDeclaration::AnimationPlayState(DeclaredValue::Value( + animation_play_state::SpecifiedValue( + vec![animation_play_state::SingleSpecifiedValue::running]))), ]), }, ]