Auto merge of #15793 - upsuper:animation, r=emilio,bholley

Fix animation shorthand parsing

which is somehow broken after #15779.

But it seems there are various issue around the animation shorthand parsing, so I try to fix it to make it match the spec.

I expect this change to fix most parsing failures in Gecko's test suite, although I haven't tested.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15793)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-03-03 03:28:21 -08:00 committed by GitHub
commit 7cd4c69c40
7 changed files with 99 additions and 97 deletions

View file

@ -398,7 +398,9 @@ impl<E: TElement> StyleSharingCandidateCache<E> {
return;
}
if box_style.animation_name_count() > 0 {
let animation_count = box_style.animation_name_count();
debug_assert!(animation_count > 0);
if animation_count > 1 || box_style.animation_name_at(0).0 != atom!("") {
debug!("Failing to insert to the cache: animations");
return;
}

View file

@ -1324,7 +1324,7 @@ fn static_assert() {
<%def name="impl_animation_time_value(ident, gecko_ffi_name)">
#[allow(non_snake_case)]
pub fn set_animation_${ident}(&mut self, v: longhands::animation_${ident}::computed_value::T) {
assert!(v.0.len() > 0);
debug_assert!(!v.0.is_empty());
unsafe { self.gecko.mAnimations.ensure_len(v.0.len()) };
self.gecko.mAnimation${gecko_ffi_name}Count = v.0.len() as u32;
@ -1348,7 +1348,7 @@ fn static_assert() {
use properties::longhands::animation_${ident}::single_value::computed_value::T as Keyword;
use gecko_bindings::structs;
assert!(v.0.len() > 0);
debug_assert!(!v.0.is_empty());
unsafe { self.gecko.mAnimations.ensure_len(v.0.len()) };
self.gecko.mAnimation${gecko_ffi_name}Count = v.0.len() as u32;
@ -1727,15 +1727,14 @@ fn static_assert() {
pub fn set_animation_name(&mut self, v: longhands::animation_name::computed_value::T) {
use nsstring::nsCString;
debug_assert!(!v.0.is_empty());
unsafe { self.gecko.mAnimations.ensure_len(v.0.len()) };
if v.0.len() > 0 {
self.gecko.mAnimationNameCount = v.0.len() as u32;
for (servo, gecko) in v.0.into_iter().zip(self.gecko.mAnimations.iter_mut()) {
gecko.mName.assign_utf8(&nsCString::from(servo.0.to_string()));
}
} else {
unsafe { self.gecko.mAnimations[0].mName.truncate(); }
self.gecko.mAnimationNameCount = v.0.len() as u32;
for (servo, gecko) in v.0.into_iter().zip(self.gecko.mAnimations.iter_mut()) {
// TODO This is inefficient. We should fix this in bug 1329169.
gecko.mName.assign_utf8(&nsCString::from(servo.0.to_string()));
}
}
pub fn animation_name_at(&self, index: usize)
@ -1773,7 +1772,7 @@ fn static_assert() {
use std::f32;
use properties::longhands::animation_iteration_count::single_value::SpecifiedValue as AnimationIterationCount;
assert!(v.0.len() > 0);
debug_assert!(!v.0.is_empty());
unsafe { self.gecko.mAnimations.ensure_len(v.0.len()) };
self.gecko.mAnimationIterationCountCount = v.0.len() as u32;
@ -1799,7 +1798,7 @@ fn static_assert() {
${impl_copy_animation_value('iteration_count', 'IterationCount')}
pub fn set_animation_timing_function(&mut self, v: longhands::animation_timing_function::computed_value::T) {
assert!(v.0.len() > 0);
debug_assert!(!v.0.is_empty());
unsafe { self.gecko.mAnimations.ensure_len(v.0.len()) };
self.gecko.mAnimationTimingFunctionCount = v.0.len() as u32;

View file

@ -460,6 +460,11 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
Time(0.0)
}
#[inline]
pub fn get_initial_specified_value() -> SpecifiedValue {
SpecifiedValue(0.0)
}
pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue,()> {
Time::parse(context, input)
}
@ -727,7 +732,7 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
#[inline]
pub fn get_initial_specified_value() -> SpecifiedValue {
ToComputedValue::from_computed_value(&ease())
SpecifiedValue::Keyword(FunctionKeyword::Ease)
}
pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue,()> {
@ -776,7 +781,6 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
</%helpers:vector_longhand>
<%helpers:vector_longhand name="animation-name"
allow_empty="True"
need_index="True"
animatable="False",
extra_prefixes="moz webkit"
@ -797,6 +801,16 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
pub struct SpecifiedValue(pub Atom);
#[inline]
pub fn get_initial_value() -> computed_value::T {
get_initial_specified_value()
}
#[inline]
pub fn get_initial_specified_value() -> SpecifiedValue {
SpecifiedValue(atom!(""))
}
impl fmt::Display for SpecifiedValue {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
self.0.fmt(f)
@ -805,7 +819,11 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
impl ToCss for SpecifiedValue {
fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
dest.write_str(&*self.0.to_string())
if self.0 == atom!("") {
dest.write_str("none")
} else {
dest.write_str(&*self.0.to_string())
}
}
}
@ -813,7 +831,11 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
fn parse(_context: &ParserContext, input: &mut ::cssparser::Parser) -> Result<Self, ()> {
use cssparser::Token;
Ok(match input.next() {
Ok(Token::Ident(ref value)) if value != "none" => SpecifiedValue(Atom::from(&**value)),
Ok(Token::Ident(ref value)) => SpecifiedValue(if value == "none" {
atom!("")
} else {
Atom::from(&**value)
}),
Ok(Token::QuotedString(value)) => SpecifiedValue(Atom::from(&*value)),
_ => return Err(()),
})
@ -835,6 +857,7 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
spec="https://drafts.csswg.org/css-animations/#propdef-animation-duration",
allowed_in_keyframe_block="False">
pub use properties::longhands::transition_duration::single_value::computed_value;
pub use properties::longhands::transition_duration::single_value::get_initial_specified_value;
pub use properties::longhands::transition_duration::single_value::{get_initial_value, parse};
pub use properties::longhands::transition_duration::single_value::SpecifiedValue;
</%helpers:vector_longhand>
@ -903,7 +926,12 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
#[inline]
pub fn get_initial_value() -> computed_value::T {
computed_value::T::Number(1.0)
get_initial_specified_value()
}
#[inline]
pub fn get_initial_specified_value() -> SpecifiedValue {
SpecifiedValue::Number(1.0)
}
#[inline]
@ -955,6 +983,7 @@ ${helpers.single_keyword("animation-fill-mode",
spec="https://drafts.csswg.org/css-animations/#propdef-animation-delay",
allowed_in_keyframe_block="False">
pub use properties::longhands::transition_duration::single_value::computed_value;
pub use properties::longhands::transition_duration::single_value::get_initial_specified_value;
pub use properties::longhands::transition_duration::single_value::{get_initial_value, parse};
pub use properties::longhands::transition_duration::single_value::SpecifiedValue;
</%helpers:vector_longhand>

View file

@ -153,32 +153,26 @@ macro_rules! try_parse_one {
animation-fill-mode animation-play-state"
allowed_in_keyframe_block="False"
spec="https://drafts.csswg.org/css-animations/#propdef-animation">
<%
props = "name duration timing_function delay iteration_count \
direction fill_mode play_state".split()
%>
use parser::Parse;
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};
% for prop in props:
use properties::longhands::animation_${prop};
% endfor
pub fn parse_value(context: &ParserContext, input: &mut Parser) -> Result<Longhands, ()> {
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,
% for prop in props:
animation_${prop}: animation_${prop}::SingleSpecifiedValue,
% endfor
}
fn parse_one_animation(context: &ParserContext, input: &mut Parser) -> Result<SingleAnimation,()> {
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;
% for prop in props:
let mut ${prop} = None;
% endfor
// NB: Name must be the last one here so that keywords valid for other
// longhands are not interpreted as names.
@ -198,62 +192,29 @@ macro_rules! try_parse_one {
break
}
if let Some(name) = name {
Ok(SingleAnimation {
animation_name: name,
animation_duration:
duration.unwrap_or_else(animation_duration::single_value::get_initial_value),
animation_timing_function:
timing_function.unwrap_or_else(animation_timing_function::single_value
::get_initial_specified_value),
animation_delay:
delay.unwrap_or_else(animation_delay::single_value::get_initial_value),
animation_iteration_count:
iteration_count.unwrap_or_else(animation_iteration_count::single_value::get_initial_value),
animation_direction:
direction.unwrap_or_else(animation_direction::single_value::get_initial_value),
animation_fill_mode:
fill_mode.unwrap_or_else(animation_fill_mode::single_value::get_initial_value),
animation_play_state:
play_state.unwrap_or_else(animation_play_state::single_value::get_initial_value),
})
} else {
Err(())
}
Ok(SingleAnimation {
% for prop in props:
animation_${prop}: ${prop}.unwrap_or_else(animation_${prop}::single_value
::get_initial_specified_value),
% endfor
})
}
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 prop in props:
let mut ${prop}s = vec![];
% endfor
if input.try(|input| input.expect_ident_matching("none")).is_err() {
let results = try!(input.parse_comma_separated(|i| parse_one_animation(context, i)));
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);
}
let results = try!(input.parse_comma_separated(|i| parse_one_animation(context, i)));
for result in results.into_iter() {
% for prop in props:
${prop}s.push(result.animation_${prop});
% endfor
}
Ok(Longhands {
animation_name: animation_name::SpecifiedValue(names),
animation_duration: animation_duration::SpecifiedValue(durations),
animation_timing_function: animation_timing_function::SpecifiedValue(timing_functions),
animation_delay: animation_delay::SpecifiedValue(delays),
animation_iteration_count: animation_iteration_count::SpecifiedValue(iteration_counts),
animation_direction: animation_direction::SpecifiedValue(directions),
animation_fill_mode: animation_fill_mode::SpecifiedValue(fill_modes),
animation_play_state: animation_play_state::SpecifiedValue(play_states),
% for prop in props:
animation_${prop}: animation_${prop}::SpecifiedValue(${prop}s),
% endfor
})
}
@ -265,14 +226,9 @@ macro_rules! try_parse_one {
return Ok(());
}
<%
subproperties = "duration timing_function delay direction \
fill_mode iteration_count play_state".split()
%>
// If any value list length is differs then we don't do a shorthand serialization
// either.
% for name in subproperties:
% for name in props[1:]:
if len != self.animation_${name}.0.len() {
return Ok(())
}
@ -283,7 +239,7 @@ macro_rules! try_parse_one {
try!(write!(dest, ", "));
}
% for name in subproperties:
% for name in props[1:]:
self.animation_${name}.0[i].to_css(dest)?;
dest.write_str(" ")?;
% endfor