From 411ae84908f6e0c167a9a77e32d57dfa089dbafb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 1 Jul 2016 12:12:56 -0700 Subject: [PATCH 1/4] style: Add support to the animation shorthand and fix parsing of animation-name. --- .../dom/webidls/CSSStyleDeclaration.webidl | 1 + components/style/properties/helpers.mako.rs | 16 +- .../style/properties/longhand/box.mako.rs | 46 ++++- .../style/properties/shorthand/box.mako.rs | 173 ++++++++++++++---- ...s-flexbox-height-animation-stretch.htm.ini | 4 + 5 files changed, 194 insertions(+), 46 deletions(-) create mode 100644 tests/wpt/metadata-css/css-flexbox-1_dev/html/css-flexbox-height-animation-stretch.htm.ini diff --git a/components/script/dom/webidls/CSSStyleDeclaration.webidl b/components/script/dom/webidls/CSSStyleDeclaration.webidl index 02864d39a77..22044bcf17f 100644 --- a/components/script/dom/webidls/CSSStyleDeclaration.webidl +++ b/components/script/dom/webidls/CSSStyleDeclaration.webidl @@ -331,6 +331,7 @@ partial interface CSSStyleDeclaration { [SetterThrows, TreatNullAs=EmptyString] attribute DOMString alignSelf; [SetterThrows, TreatNullAs=EmptyString] attribute DOMString align-self; + [SetterThrows, TreatNullAs=EmptyString] attribute DOMString animation; [SetterThrows, TreatNullAs=EmptyString] attribute DOMString animation-name; [SetterThrows, TreatNullAs=EmptyString] attribute DOMString animationName; [SetterThrows, TreatNullAs=EmptyString] attribute DOMString animation-duration; diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index 81dff023605..dae60af0860 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -235,11 +235,21 @@ } } + pub use self::computed_value::${to_camel_case(name)} as SingleSpecifiedValue; + + #[inline] + pub fn parse_one(input: &mut Parser) -> Result { + SingleSpecifiedValue::parse(input) + } + #[inline] pub fn get_initial_value() -> computed_value::T { - computed_value::T(vec![ - computed_value::${to_camel_case(name)}::${to_rust_ident(values.split()[0])} - ]) + computed_value::T(vec![get_initial_single_value()]) + } + + #[inline] + pub fn get_initial_single_value() -> SingleSpecifiedValue { + SingleSpecifiedValue::${to_rust_ident(values.split()[0])} } #[inline] diff --git a/components/style/properties/longhand/box.mako.rs b/components/style/properties/longhand/box.mako.rs index 381ca4df1d9..d6a7f015e50 100644 --- a/components/style/properties/longhand/box.mako.rs +++ b/components/style/properties/longhand/box.mako.rs @@ -490,8 +490,7 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", pub fn parse_one(input: &mut Parser) -> Result { if let Ok(function_name) = input.try(|input| input.expect_function()) { - return match_ignore_ascii_case! { - function_name, + return match_ignore_ascii_case! { function_name, "cubic-bezier" => { let (mut p1x, mut p1y, mut p2x, mut p2y) = (0.0, 0.0, 0.0, 0.0); try!(input.parse_nested_block(|input| { @@ -582,6 +581,12 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", computed_value::T(Vec::new()) } + + #[inline] + pub fn parse_one(input: &mut Parser) -> Result { + SingleSpecifiedValue::parse(input) + } + pub fn parse(_: &ParserContext, input: &mut Parser) -> Result { Ok(SpecifiedValue(try!(input.parse_comma_separated(SingleSpecifiedValue::parse)))) } @@ -640,6 +645,18 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", } pub use self::computed_value::T as SpecifiedValue; + pub use string_cache::Atom as SingleSpecifiedValue; + + #[inline] + pub fn parse_one(input: &mut Parser) -> Result { + use cssparser::Token; + + Ok(match input.next() { + Ok(Token::Ident(ref value)) if value != "none" => Atom::from(&**value), + Ok(Token::QuotedString(value)) => Atom::from(&*value), + _ => return Err(()), + }) + } #[inline] pub fn get_initial_value() -> computed_value::T { @@ -648,9 +665,7 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", pub fn parse(_: &ParserContext, input: &mut Parser) -> Result { use std::borrow::Cow; - Ok(SpecifiedValue(try!(input.parse_comma_separated(|input| { - input.expect_ident().map(Atom::from) - })))) + Ok(SpecifiedValue(try!(input.parse_comma_separated(parse_one)))) } impl ComputedValueAsSpecified for SpecifiedValue {} @@ -660,16 +675,20 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", need_index="True" animatable="False"> pub use super::transition_duration::computed_value; - pub use super::transition_duration::{parse, get_initial_value}; + pub use super::transition_duration::{get_initial_value, get_initial_single_value}; + pub use super::transition_duration::{parse, parse_one}; pub use super::transition_duration::SpecifiedValue; + pub use super::transition_duration::SingleSpecifiedValue; <%helpers:longhand name="animation-timing-function" need_index="True" animatable="False"> pub use super::transition_timing_function::computed_value; - pub use super::transition_timing_function::{parse, get_initial_value}; + pub use super::transition_timing_function::{get_initial_value, get_initial_single_value}; + pub use super::transition_timing_function::{parse, parse_one}; pub use super::transition_timing_function::SpecifiedValue; + pub use super::transition_timing_function::SingleSpecifiedValue; <%helpers:longhand name="animation-iteration-count" @@ -720,8 +739,14 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", } pub use self::computed_value::AnimationIterationCount; + pub use self::computed_value::AnimationIterationCount as SingleSpecifiedValue; pub use self::computed_value::T as SpecifiedValue; + #[inline] + pub fn get_initial_single_value() -> AnimationIterationCount { + AnimationIterationCount::Number(1) + } + pub fn parse_one(input: &mut Parser) -> Result { if input.try(|input| input.expect_ident_matching("infinite")).is_ok() { Ok(AnimationIterationCount::Infinite) @@ -740,8 +765,9 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", Ok(SpecifiedValue(try!(input.parse_comma_separated(parse_one)))) } + #[inline] pub fn get_initial_value() -> computed_value::T { - computed_value::T(vec![AnimationIterationCount::Number(1)]) + computed_value::T(vec![get_initial_single_value()]) } impl ComputedValueAsSpecified for SpecifiedValue {} @@ -767,8 +793,10 @@ ${helpers.keyword_list("animation-fill-mode", need_index="True" animatable="False"> pub use super::transition_duration::computed_value; - pub use super::transition_duration::{parse, get_initial_value}; + pub use super::transition_duration::{get_initial_value, get_initial_single_value}; + pub use super::transition_duration::{parse, parse_one}; pub use super::transition_duration::SpecifiedValue; + pub use super::transition_duration::SingleSpecifiedValue; // CSSOM View Module diff --git a/components/style/properties/shorthand/box.mako.rs b/components/style/properties/shorthand/box.mako.rs index 27e923ba1aa..416923ced64 100644 --- a/components/style/properties/shorthand/box.mako.rs +++ b/components/style/properties/shorthand/box.mako.rs @@ -14,8 +14,20 @@ }) +macro_rules! try_parse_one { + ($input: expr, $var: ident, $prop_module: ident) => { + if $var.is_none() { + if let Ok(value) = $input.try($prop_module::parse_one) { + $var = Some(value); + continue; + } + } + } +} + <%helpers:shorthand name="transition" - sub_properties="transition-property transition-duration transition-timing-function + sub_properties="transition-property transition-duration + transition-timing-function transition-delay"> use properties::longhands::{transition_delay, transition_duration, transition_property}; use properties::longhands::{transition_timing_function}; @@ -31,35 +43,10 @@ let (mut property, mut duration) = (None, None); let (mut timing_function, mut delay) = (None, None); loop { - if property.is_none() { - if let Ok(value) = input.try(transition_property::SingleSpecifiedValue::parse) { - property = Some(value); - continue - } - } - - if duration.is_none() { - if let Ok(value) = input.try(|input| transition_duration::parse_one(input)) { - duration = Some(value); - continue - } - } - - if timing_function.is_none() { - if let Ok(value) = input.try(|input| { - transition_timing_function::parse_one(input) - }) { - timing_function = Some(value); - continue - } - } - - if delay.is_none() { - if let Ok(value) = input.try(|input| transition_delay::parse_one(input)) { - delay = Some(value); - continue; - } - } + try_parse_one!(input, property, transition_property); + try_parse_one!(input, duration, transition_duration); + try_parse_one!(input, timing_function, transition_timing_function); + try_parse_one!(input, delay, transition_delay); break } @@ -68,12 +55,12 @@ Ok(SingleTransition { transition_property: property, transition_duration: - duration.unwrap_or(transition_duration::get_initial_single_value()), + duration.unwrap_or_else(transition_duration::get_initial_single_value), transition_timing_function: - timing_function.unwrap_or( - transition_timing_function::get_initial_single_value()), + timing_function.unwrap_or_else( + transition_timing_function::get_initial_single_value), transition_delay: - delay.unwrap_or(transition_delay::get_initial_single_value()), + delay.unwrap_or_else(transition_delay::get_initial_single_value), }) } else { Err(()) @@ -107,3 +94,121 @@ transition_delay: Some(transition_delay::SpecifiedValue(delays)), }) + +<%helpers:shorthand name="animation" + sub_properties="animation-name animation-duration + animation-timing-function animation-delay + animation-iteration-count animation-direction + animation-fill-mode animation-play-state"> + 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}; + + struct SingleAnimation { + animation_name: animation_name::SingleSpecifiedValue, + animation_duration: animation_duration::SingleSpecifiedValue, + animation_timing_function: animation_timing_function::SingleSpecifiedValue, + animation_delay: animation_delay::SingleSpecifiedValue, + animation_iteration_count: animation_iteration_count::SingleSpecifiedValue, + animation_direction: animation_direction::SingleSpecifiedValue, + animation_fill_mode: animation_fill_mode::SingleSpecifiedValue, + animation_play_state: animation_play_state::SingleSpecifiedValue, + } + + fn parse_one_animation(input: &mut Parser) -> Result { + let mut duration = None; + let mut timing_function = None; + let mut delay = None; + let mut iteration_count = None; + let mut direction = None; + let mut fill_mode = None; + let mut play_state = None; + let mut name = None; + + // NB: Name must be the last one here so that keywords valid for other + // longhands are not interpreted as names. + // + // Also, duration must be before delay, see + // https://drafts.csswg.org/css-animations/#typedef-single-animation + loop { + try_parse_one!(input, duration, animation_duration); + try_parse_one!(input, timing_function, animation_timing_function); + try_parse_one!(input, delay, animation_delay); + try_parse_one!(input, iteration_count, animation_iteration_count); + try_parse_one!(input, direction, animation_direction); + try_parse_one!(input, fill_mode, animation_fill_mode); + try_parse_one!(input, play_state, animation_play_state); + try_parse_one!(input, name, animation_name); + + break + } + + if let Some(name) = name { + Ok(SingleAnimation { + animation_name: name, + animation_duration: + duration.unwrap_or_else(animation_duration::get_initial_single_value), + animation_timing_function: + timing_function.unwrap_or_else(animation_timing_function::get_initial_single_value), + animation_delay: + delay.unwrap_or_else(animation_delay::get_initial_single_value), + animation_iteration_count: + iteration_count.unwrap_or_else(animation_iteration_count::get_initial_single_value), + animation_direction: + direction.unwrap_or_else(animation_direction::get_initial_single_value), + animation_fill_mode: + fill_mode.unwrap_or_else(animation_fill_mode::get_initial_single_value), + animation_play_state: + play_state.unwrap_or_else(animation_play_state::get_initial_single_value), + }) + } else { + Err(()) + } + } + + if input.try(|input| input.expect_ident_matching("none")).is_ok() { + return Ok(Longhands { + animation_name: None, + animation_duration: None, + animation_timing_function: None, + animation_delay: None, + animation_iteration_count: None, + animation_direction: None, + animation_fill_mode: None, + animation_play_state: None, + }) + } + + let results = try!(input.parse_comma_separated(parse_one_animation)); + + let mut names = vec![]; + let mut durations = vec![]; + let mut timing_functions = vec![]; + let mut delays = vec![]; + let mut iteration_counts = vec![]; + let mut directions = vec![]; + let mut fill_modes = vec![]; + let mut play_states = vec![]; + + for result in results.into_iter() { + names.push(result.animation_name); + durations.push(result.animation_duration); + timing_functions.push(result.animation_timing_function); + delays.push(result.animation_delay); + iteration_counts.push(result.animation_iteration_count); + directions.push(result.animation_direction); + fill_modes.push(result.animation_fill_mode); + play_states.push(result.animation_play_state); + } + + Ok(Longhands { + animation_name: Some(animation_name::SpecifiedValue(names)), + animation_duration: Some(animation_duration::SpecifiedValue(durations)), + animation_timing_function: Some(animation_timing_function::SpecifiedValue(timing_functions)), + animation_delay: Some(animation_delay::SpecifiedValue(delays)), + animation_iteration_count: Some(animation_iteration_count::SpecifiedValue(iteration_counts)), + animation_direction: Some(animation_direction::SpecifiedValue(directions)), + animation_fill_mode: Some(animation_fill_mode::SpecifiedValue(fill_modes)), + animation_play_state: Some(animation_play_state::SpecifiedValue(play_states)), + }) + diff --git a/tests/wpt/metadata-css/css-flexbox-1_dev/html/css-flexbox-height-animation-stretch.htm.ini b/tests/wpt/metadata-css/css-flexbox-1_dev/html/css-flexbox-height-animation-stretch.htm.ini new file mode 100644 index 00000000000..3d874530583 --- /dev/null +++ b/tests/wpt/metadata-css/css-flexbox-1_dev/html/css-flexbox-height-animation-stretch.htm.ini @@ -0,0 +1,4 @@ +[css-flexbox-height-animation-stretch.htm] + type: reftest + expected: TIMEOUT + bug: https://github.com/servo/servo/issues/12328 From 8bbebd0514473e992535f10ae742467c0c071744 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 1 Jul 2016 12:50:11 -0700 Subject: [PATCH 2/4] style: bonus: Fix parsing of steps() function. Still doesn't work properly (this also happened with transitions). This is spec'd in: https://www.w3.org/TR/css3-transitions/#transition-timing-function --- .../style/properties/longhand/box.mako.rs | 17 +++++++++-------- .../html/transition-timing-function-001.htm.ini | 3 --- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/components/style/properties/longhand/box.mako.rs b/components/style/properties/longhand/box.mako.rs index d6a7f015e50..3a02b6c656d 100644 --- a/components/style/properties/longhand/box.mako.rs +++ b/components/style/properties/longhand/box.mako.rs @@ -507,16 +507,17 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto", Ok(TransitionTimingFunction::CubicBezier(p1, p2)) }, "steps" => { - let (mut step_count, mut start_end) = (0, computed_value::StartEnd::Start); + let (mut step_count, mut start_end) = (0, computed_value::StartEnd::End); try!(input.parse_nested_block(|input| { step_count = try!(specified::parse_integer(input)); - try!(input.expect_comma()); - start_end = try!(match_ignore_ascii_case! { - try!(input.expect_ident()), - "start" => Ok(computed_value::StartEnd::Start), - "end" => Ok(computed_value::StartEnd::End), - _ => Err(()) - }); + if input.try(|input| input.expect_comma()).is_ok() { + start_end = try!(match_ignore_ascii_case! { + try!(input.expect_ident()), + "start" => Ok(computed_value::StartEnd::Start), + "end" => Ok(computed_value::StartEnd::End), + _ => Err(()) + }); + } Ok(()) })); Ok(TransitionTimingFunction::Steps(step_count as u32, start_end)) diff --git a/tests/wpt/metadata-css/css-transitions-1_dev/html/transition-timing-function-001.htm.ini b/tests/wpt/metadata-css/css-transitions-1_dev/html/transition-timing-function-001.htm.ini index 1d3aaf317a8..e5ec8603b1e 100644 --- a/tests/wpt/metadata-css/css-transitions-1_dev/html/transition-timing-function-001.htm.ini +++ b/tests/wpt/metadata-css/css-transitions-1_dev/html/transition-timing-function-001.htm.ini @@ -1,8 +1,5 @@ [transition-timing-function-001.htm] type: testharness - [parse 'steps(3)'] - expected: FAIL - [parse 'cubic-bezier(-0.1, -0.2, -0.3, -0.4)'] expected: FAIL From 8ba676533bb1bba3e030041946caf3a5ac0d1d6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 1 Jul 2016 15:52:04 -0700 Subject: [PATCH 3/4] style: Fix timing-function overriding from the keyframe declaration list. The previous behavior is plain wrong, since that array has always at least one element, so we effectively couldn't specify anything else than "ease" in our animations. --- components/style/animation.rs | 4 +++- components/style/keyframes.rs | 18 +++++++++++++++ ...ming-function-override-from-keyframes.html | 23 +++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 tests/html/animation-timing-function-override-from-keyframes.html diff --git a/components/style/animation.rs b/components/style/animation.rs index 87184928eb8..1f0cbcaefde 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -626,7 +626,9 @@ where Impl: SelectorImplExt, // NB: The spec says that the timing function can be overwritten // from the keyframe style. let mut timing_function = style.get_box().animation_timing_function_mod(index); - if from_style.get_box().animation_timing_function_count() != 0 { + if last_keyframe.declared_timing_function { + // NB: animation_timing_function can never be empty, always has + // at least the default value (`ease`). timing_function = from_style.get_box().animation_timing_function_at(0); } diff --git a/components/style/keyframes.rs b/components/style/keyframes.rs index 82dc4c9200f..a6e667e9282 100644 --- a/components/style/keyframes.rs +++ b/components/style/keyframes.rs @@ -114,15 +114,33 @@ pub struct KeyframesStep { /// Declarations that will determine the final style during the step, or /// `ComputedValues` if this is an autogenerated step. pub value: KeyframesStepValue, + /// Wether a animation-timing-function declaration exists in the list of + /// declarations. + /// + /// This is used to know when to override the keyframe animation style. + pub declared_timing_function: bool, } impl KeyframesStep { #[inline] fn new(percentage: KeyframePercentage, value: KeyframesStepValue) -> Self { + let declared_timing_function = match value { + KeyframesStepValue::Declarations(ref declarations) => { + declarations.iter().any(|prop_decl| { + match *prop_decl { + PropertyDeclaration::AnimationTimingFunction(..) => true, + _ => false, + } + }) + } + _ => false, + }; + KeyframesStep { start_percentage: percentage, value: value, + declared_timing_function: declared_timing_function, } } } diff --git a/tests/html/animation-timing-function-override-from-keyframes.html b/tests/html/animation-timing-function-override-from-keyframes.html new file mode 100644 index 00000000000..7e37c2d19cb --- /dev/null +++ b/tests/html/animation-timing-function-override-from-keyframes.html @@ -0,0 +1,23 @@ + + + +

You should see an eased animation in the first-element, and a stepped one in the second one

+
+
From 3a4539f04338a0897cc768b93fe5f9f4a0461554 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 1 Jul 2016 16:03:55 -0700 Subject: [PATCH 4/4] style: stylistic nit. be consistent on how we write inline annotations. --- components/style/properties/longhand/color.mako.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/style/properties/longhand/color.mako.rs b/components/style/properties/longhand/color.mako.rs index 31368dbe9ee..1f3831517ab 100644 --- a/components/style/properties/longhand/color.mako.rs +++ b/components/style/properties/longhand/color.mako.rs @@ -25,7 +25,8 @@ use cssparser; pub type T = cssparser::RGBA; } - #[inline] pub fn get_initial_value() -> computed_value::T { + #[inline] + pub fn get_initial_value() -> computed_value::T { RGBA { red: 0., green: 0., blue: 0., alpha: 1. } /* black */ } pub fn parse_specified(_context: &ParserContext, input: &mut Parser)