Fixes serialization of multiple transitions.

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

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

@@ -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));
+            fn extract_value<T>(x: &DeclaredValue<T>) -> Option< &T> {
-            try!(write!(dest, " "));
+                match *x {
+                    DeclaredValue::Value(ref val) => Some(val),
+                    _ => None,
+                }
+            }
 
-            try!(self.transition_duration.to_css(dest));
+            let len = extract_value(self.transition_property).map(|i| i.0.len()).unwrap_or(0);
-            try!(write!(dest, " "));
+            // There should be at least one declared value
+            if len == 0 {
+                return dest.write_str("")
+            }
 
-            try!(self.transition_timing_function.to_css(dest));
+            // If any value list length is differs then we don't do a shorthand serialization
-            try!(write!(dest, " "));
+            // 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
 
-            self.transition_delay.to_css(dest)
+            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
+
+                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>

tests/unit/style/properties/serialization.rs

@@ -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);
@@ -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;
@@ -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::*;
 
@@ -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();
 
@@ -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();
 
@@ -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();
 
@@ -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();
 
@@ -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;\
@@ -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;\
@@ -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();
 
@@ -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();