Skip to content

Commit

Permalink
Fixes serialization of multiple transitions.
Browse files Browse the repository at this point in the history 
Similar to animations and backgrounds, multiple transitions should be
serialized as a list of shorthand values only if the number of values
for each longhand property is the same. Otherwise we default to the
longhand serialization. Fixes #15398
  • Loading branch information
@absoludity
absoludity committed Feb 26, 2017
1 parent 3a812b0 commit f5af381
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 66 deletions.
49 changes: 42 additions & 7 deletions components/style/properties/shorthand/box.mako.rs
View file
Expand Up @@ -133,16 +133,51 @@ macro_rules! try_parse_one {

impl<'a> LonghandsToSerialize<'a> {
fn to_css_declared<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
try!(self.transition_property.to_css(dest));
try!(write!(dest, " "));
fn extract_value<T>(x: &DeclaredValue<T>) -> Option< &T> {
match *x {
DeclaredValue::Value(ref val) => Some(val),
_ => None,
}
}

try!(self.transition_duration.to_css(dest));
try!(write!(dest, " "));
let len = extract_value(self.transition_property).map(|i| i.0.len()).unwrap_or(0);
// There should be at least one declared value
if len == 0 {
return dest.write_str("")
}

try!(self.transition_timing_function.to_css(dest));
try!(write!(dest, " "));
// If any value list length is differs then we don't do a shorthand serialization
// either.
% for name in "property duration delay timing_function".split():
if len != extract_value(self.transition_${name}).map(|i| i.0.len()).unwrap_or(0) {
return dest.write_str("")
}
% endfor

let mut first = true;
for i in 0..len {
% for name in "property duration delay timing_function".split():
let ${name} = if let DeclaredValue::Value(ref arr) = *self.transition_${name} {
&arr.0[i]
} else {
unreachable!()
};
% endfor

self.transition_delay.to_css(dest)
if first {
first = false;
} else {
try!(write!(dest, ", "));
}

try!(property.to_css(dest));

% for name in "duration timing_function delay".split():
try!(write!(dest, " "));
try!(${name}.to_css(dest));
% endfor
}
Ok(())
}
}
</%helpers:shorthand>
Expand Down
119 changes: 60 additions & 59 deletions tests/unit/style/properties/serialization.rs
View file
Expand Up @@ -17,7 +17,7 @@ use style::values::specified::{LengthOrPercentage, LengthOrPercentageOrAuto, Len
use style::values::specified::url::SpecifiedUrl;
use style_traits::ToCss;

fn property_declaration_block(css_properties: &str) -> PropertyDeclarationBlock {
fn parse_declaration_block(css_properties: &str) -> PropertyDeclarationBlock {
let url = ServoUrl::parse("http://localhost").unwrap();
let context = ParserContext::new(Origin::Author, &url, Box::new(CSSErrorReporterTest));
let mut parser = Parser::new(css_properties);
Expand Down Expand Up @@ -563,46 +563,6 @@ mod shorthand_serialization {
assert_eq!(serialization, "columns: auto auto;");
}

#[test]
fn transition_should_serialize_all_available_properties() {
use euclid::point::Point2D;
use style::properties::animated_properties::TransitionProperty;
use style::properties::longhands::transition_delay::SpecifiedValue as DelayContainer;
use style::properties::longhands::transition_duration::SpecifiedValue as DurationContainer;
use style::properties::longhands::transition_property::SpecifiedValue as PropertyContainer;
use style::properties::longhands::transition_timing_function;
use style::values::specified::Time as TimeContainer;

let property_name = DeclaredValue::Value(
PropertyContainer(vec![TransitionProperty::MarginLeft])
);

let duration = DeclaredValue::Value(
DurationContainer(vec![TimeContainer(3f32)])
);

let delay = DeclaredValue::Value(
DelayContainer(vec![TimeContainer(4f32)])
);

let timing_function = DeclaredValue::Value(
transition_timing_function::SpecifiedValue(vec![
transition_timing_function::single_value::SpecifiedValue::CubicBezier(
Point2D::new(0f32, 5f32), Point2D::new(5f32, 10f32))
])
);

let mut properties = Vec::new();

properties.push(PropertyDeclaration::TransitionProperty(property_name));
properties.push(PropertyDeclaration::TransitionDelay(delay));
properties.push(PropertyDeclaration::TransitionDuration(duration));
properties.push(PropertyDeclaration::TransitionTimingFunction(timing_function));

let serialization = shorthand_properties_to_string(properties);
assert_eq!(serialization, "transition: margin-left 3s cubic-bezier(0, 5, 5, 10) 4s;");
}

#[test]
fn flex_should_serialize_all_available_properties() {
use style::values::specified::Number as NumberContainer;
Expand Down Expand Up @@ -687,16 +647,6 @@ mod shorthand_serialization {
}
*/

// TODO: Populate Atom Cache for testing so that the animation shorthand can be tested
/*
#[test]
fn animation_should_serialize_all_available_properties() {
let mut properties = Vec::new();
assert_eq!(serialization, "animation;");
}
*/

mod background {
use super::*;

Expand All @@ -712,7 +662,7 @@ mod shorthand_serialization {
background-position-y: 4px; \
background-origin: border-box; \
background-clip: padding-box;";
let block = property_declaration_block(block_text);
let block = parse_declaration_block(block_text);

let serialization = block.to_css_string();

Expand All @@ -735,7 +685,7 @@ mod shorthand_serialization {
background-position-y: 4px; \
background-origin: padding-box; \
background-clip: padding-box;";
let block = property_declaration_block(block_text);
let block = parse_declaration_block(block_text);

let serialization = block.to_css_string();

Expand All @@ -758,7 +708,7 @@ mod shorthand_serialization {
background-position-y: 4px, 40px; \
background-origin: border-box, padding-box; \
background-clip: padding-box, padding-box;";
let block = property_declaration_block(block_text);
let block = parse_declaration_block(block_text);

let serialization = block.to_css_string();

Expand Down Expand Up @@ -788,7 +738,7 @@ mod shorthand_serialization {
background-position: 7px 4px; \
background-origin: border-box; \
background-clip: padding-box, padding-box;";
let block = property_declaration_block(block_text);
let block = parse_declaration_block(block_text);

let serialization = block.to_css_string();

Expand Down Expand Up @@ -1046,7 +996,7 @@ mod shorthand_serialization {

#[test]
fn serialize_single_animation() {
let block = property_declaration_block("\
let block = parse_declaration_block("\
animation-name: bounce;\
animation-duration: 1s;\
animation-timing-function: ease-in;\
Expand All @@ -1063,7 +1013,7 @@ mod shorthand_serialization {

#[test]
fn serialize_multiple_animations() {
let block = property_declaration_block("\
let block = parse_declaration_block("\
animation-name: bounce, roll;\
animation-duration: 1s, 0.2s;\
animation-timing-function: ease-in, linear;\
Expand Down Expand Up @@ -1096,7 +1046,7 @@ mod shorthand_serialization {
animation-fill-mode: forwards, backwards; \
animation-iteration-count: infinite, 2; \
animation-play-state: paused, running;";
let block = property_declaration_block(block_text);
let block = parse_declaration_block(block_text);

let serialization = block.to_css_string();

Expand All @@ -1112,7 +1062,58 @@ mod shorthand_serialization {
animation-fill-mode: forwards, backwards; \
animation-iteration-count: infinite, 2; \
animation-play-state: paused, running;";
let block = property_declaration_block(block_text);
let block = parse_declaration_block(block_text);

let serialization = block.to_css_string();

assert_eq!(serialization, block_text);
}
}

mod transition {
pub use super::*;

#[test]
fn transition_should_serialize_all_available_properties() {
let block_text = "transition-property: margin-left; \
transition-duration: 3s; \
transition-delay: 4s; \
transition-timing-function: cubic-bezier(0.2, 5, 0.5, 2);";
let block = parse_declaration_block(block_text);

let serialization = block.to_css_string();

assert_eq!(serialization, "transition: margin-left 3s cubic-bezier(0.2, 5, 0.5, 2) 4s;");
}

#[test]
fn serialize_multiple_transitions() {
let block_text = "transition-property: margin-left, width; \
transition-duration: 3s, 2s; \
transition-delay: 4s, 5s; \
transition-timing-function: cubic-bezier(0.2, 5, 0.5, 2), ease;";
let block = parse_declaration_block(block_text);

let serialization = block.to_css_string();

assert_eq!(serialization, "transition: \
margin-left 3s cubic-bezier(0.2, 5, 0.5, 2) 4s, \
width 2s ease 5s;");
}

#[test]
fn serialize_multiple_transitions_unequal_property_lists() {
// When the lengths of property values are different, the shorthand serialization
// should not be used. Previously the implementation cycled values if the lists were
// uneven. This is incorrect, in that we should serialize to a shorthand only when the
// lists have the same length (this affects background, transition and animation).
// https://github.com/servo/servo/issues/15398 )
// The duration below has 1 extra value.
let block_text = "transition-property: margin-left, width; \
transition-duration: 3s, 2s, 4s; \
transition-delay: 4s, 5s; \
transition-timing-function: cubic-bezier(0.2, 5, 0.5, 2), ease;";
let block = parse_declaration_block(block_text);

let serialization = block.to_css_string();

Expand Down

0 comments on commit f5af381

Please sign in to comment.