servo: Merge #15733 - Background serialization of multiple values (from absoludity:fix-background-serialization); r=upsuper
authorMichael Nelson <absoludity@gmail.com>
Sun, 26 Feb 2017 16:54:11 -0800
changeset 489978 bc202c8a82358b949ecf825f3a291aa48aa3f937
parent 489977 0ee3593e23151e498f26f7fb9af07035272ebbce
child 489979 586854d5c3d4bab19a41c1695a06da4408005094
push id46964
push userbmo:mjzffr@gmail.com
push dateMon, 27 Feb 2017 13:49:06 +0000
reviewersupsuper
milestone54.0a1
servo: Merge #15733 - Background serialization of multiple values (from absoludity:fix-background-serialization); r=upsuper Similar to animation, the serialization of multiple backgrounds should return the longhands serialization if there are an unequal number of values for the required properties. This is part of #15398 <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [] These changes fix #__ (github issue number if applicable). <!-- Either: --> - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____ <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 3086b3d291253a11e83943a34464e21fb1283fba
servo/components/style/properties/shorthand/background.mako.rs
servo/components/style/properties/shorthand/box.mako.rs
servo/tests/unit/style/properties/serialization.rs
--- a/servo/components/style/properties/shorthand/background.mako.rs
+++ b/servo/components/style/properties/shorthand/background.mako.rs
@@ -8,16 +8,18 @@
 <%helpers:shorthand name="background"
                     sub_properties="background-color background-position-x background-position-y background-repeat
                                     background-attachment background-image background-size background-origin
                                     background-clip"
                     spec="https://drafts.csswg.org/css-backgrounds/#the-background">
     use properties::longhands::{background_color, background_position_x, background_position_y, background_repeat};
     use properties::longhands::{background_attachment, background_image, background_size, background_origin};
     use properties::longhands::background_clip;
+    use properties::longhands::background_clip::single_value::computed_value::T as Clip;
+    use properties::longhands::background_origin::single_value::computed_value::T as Origin;
     use values::specified::position::Position;
     use parser::Parse;
 
     impl From<background_origin::single_value::SpecifiedValue> for background_clip::single_value::SpecifiedValue {
         fn from(origin: background_origin::single_value::SpecifiedValue) ->
             background_clip::single_value::SpecifiedValue {
             match origin {
                 background_origin::single_value::SpecifiedValue::content_box =>
@@ -129,37 +131,42 @@
         fn to_css_declared<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
             // mako doesn't like ampersands following `<`
             fn extract_value<T>(x: &DeclaredValue<T>) -> Option< &T> {
                 match *x {
                     DeclaredValue::Value(ref val) => Some(val),
                     _ => None,
                 }
             }
-            use std::cmp;
-            let mut len = 0;
-            % for name in "image position_x position_y repeat size attachment origin clip".split():
-                len = cmp::max(len, extract_value(self.background_${name}).map(|i| i.0.len())
-                                                                          .unwrap_or(0));
-            % endfor
 
+            let len = extract_value(self.background_image).map(|i| i.0.len()).unwrap_or(0);
             // There should be at least one declared value
             if len == 0 {
                 return dest.write_str("")
             }
 
+            // If a value list length is differs then we don't do a shorthand serialization.
+            // The exceptions to this is color which appears once only and is serialized
+            // with the last item.
+            % for name in "image position_x position_y size repeat origin clip attachment".split():
+                if len != extract_value(self.background_${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 "image position_x position_y repeat size attachment origin clip".split():
                     let ${name} = if let DeclaredValue::Value(ref arr) = *self.background_${name} {
-                        arr.0.get(i % arr.0.len())
+                        &arr.0[i]
                     } else {
-                        None
+                        unreachable!()
                     };
                 % endfor
+
                 let color = if i == len - 1 {
                     Some(self.background_color)
                 } else {
                     None
                 };
 
                 if first {
                     first = false;
@@ -173,85 +180,43 @@
                     },
                     Some(_) => {
                         try!(write!(dest, "transparent "));
                     }
                     // Not yet the last one
                     None => ()
                 };
 
-
-                if let Some(image) = image {
-                    try!(image.to_css(dest));
-                } else {
-                    try!(write!(dest, "none"));
-                }
-
-                try!(write!(dest, " "));
-
-                if let Some(repeat) = repeat {
-                    try!(repeat.to_css(dest));
-                } else {
-                    try!(write!(dest, "repeat"));
-                }
+                % for name in "image repeat attachment position_x position_y".split():
+                    try!(${name}.to_css(dest));
+                    try!(write!(dest, " "));
+                % endfor
 
-                try!(write!(dest, " "));
-
-                if let Some(attachment) = attachment {
-                    try!(attachment.to_css(dest));
-                } else {
-                    try!(write!(dest, "scroll"));
-                }
-
-                try!(write!(dest, " "));
-
-                try!(position_x.unwrap_or(&background_position_x::single_value
-                                                                ::get_initial_position_value())
-                             .to_css(dest));
-
+                try!(write!(dest, "/ "));
+                try!(size.to_css(dest));
                 try!(write!(dest, " "));
 
-                try!(position_y.unwrap_or(&background_position_y::single_value
-                                                                ::get_initial_position_value())
-                             .to_css(dest));
-
-                if let Some(size) = size {
-                    try!(write!(dest, " / "));
-                    try!(size.to_css(dest));
-                }
-
                 match (origin, clip) {
-                    (Some(origin), Some(clip)) => {
-                        use properties::longhands::background_origin::single_value::computed_value::T as Origin;
-                        use properties::longhands::background_clip::single_value::computed_value::T as Clip;
-
+                    (&Origin::padding_box, &Clip::padding_box) => {
+                        try!(origin.to_css(dest));
+                    },
+                    (&Origin::border_box, &Clip::border_box) => {
+                        try!(origin.to_css(dest));
+                    },
+                    (&Origin::content_box, &Clip::content_box) => {
+                        try!(origin.to_css(dest));
+                    },
+                    _ => {
+                        try!(origin.to_css(dest));
                         try!(write!(dest, " "));
-
-                        match (origin, clip) {
-                            (&Origin::padding_box, &Clip::padding_box) => {
-                                try!(origin.to_css(dest));
-                            },
-                            (&Origin::border_box, &Clip::border_box) => {
-                                try!(origin.to_css(dest));
-                            },
-                            (&Origin::content_box, &Clip::content_box) => {
-                                try!(origin.to_css(dest));
-                            },
-                            _ => {
-                                try!(origin.to_css(dest));
-                                try!(write!(dest, " "));
-                                try!(clip.to_css(dest));
-                            }
-                        }
-                    },
-                    _ => {}
+                        try!(clip.to_css(dest));
+                    }
                 };
             }
 
-
             Ok(())
         }
     }
 </%helpers:shorthand>
 
 <%helpers:shorthand name="background-position"
                     sub_properties="background-position-x background-position-y"
                     spec="https://drafts.csswg.org/css-backgrounds-4/#the-background-position">
--- a/servo/components/style/properties/shorthand/box.mako.rs
+++ b/servo/components/style/properties/shorthand/box.mako.rs
@@ -128,26 +128,61 @@ macro_rules! try_parse_one {
             transition_timing_function:
                 Some(transition_timing_function::SpecifiedValue(timing_functions)),
             transition_delay: Some(transition_delay::SpecifiedValue(delays)),
         })
     }
 
     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,
+                }
+            }
+
+            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("")
+            }
+
+            // 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
 
-            try!(self.transition_duration.to_css(dest));
-            try!(write!(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
 
-            try!(self.transition_timing_function.to_css(dest));
-            try!(write!(dest, " "));
+                if first {
+                    first = false;
+                } else {
+                    try!(write!(dest, ", "));
+                }
 
-            self.transition_delay.to_css(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>
 
 <%helpers:shorthand name="animation" extra_prefixes="moz webkit"
                     sub_properties="animation-name animation-duration
                                     animation-timing-function animation-delay
                                     animation-iteration-count animation-direction
--- a/servo/tests/unit/style/properties/serialization.rs
+++ b/servo/tests/unit/style/properties/serialization.rs
@@ -1,22 +1,33 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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/. */
 
-pub use std::sync::Arc;
-pub use style::computed_values::display::T::inline_block;
-pub use style::properties::{DeclaredValue, PropertyDeclaration, PropertyDeclarationBlock, Importance, PropertyId};
-pub use style::values::specified::{BorderStyle, BorderWidth, CSSColor, Length, NoCalcLength};
-pub use style::values::specified::{LengthOrPercentage, LengthOrPercentageOrAuto, LengthOrPercentageOrAutoOrContent};
-pub use style::properties::longhands::outline_color::computed_value::T as ComputedColor;
-pub use style::properties::longhands::outline_style::SpecifiedValue as OutlineStyle;
-pub use style::values::{RGBA, Auto};
-pub use style::values::specified::url::{UrlExtraData, SpecifiedUrl};
-pub use style_traits::ToCss;
+use cssparser::Parser;
+use media_queries::CSSErrorReporterTest;
+use servo_url::ServoUrl;
+use style::computed_values::display::T::inline_block;
+use style::parser::ParserContext;
+use style::properties::{DeclaredValue, PropertyDeclaration, PropertyDeclarationBlock, Importance, PropertyId};
+use style::properties::longhands::outline_color::computed_value::T as ComputedColor;
+use style::properties::parse_property_declaration_list;
+use style::stylesheets::Origin;
+use style::values::{RGBA, Auto};
+use style::values::specified::{BorderStyle, BorderWidth, CSSColor, Length, NoCalcLength};
+use style::values::specified::{LengthOrPercentage, LengthOrPercentageOrAuto, LengthOrPercentageOrAutoOrContent};
+use style::values::specified::url::SpecifiedUrl;
+use style_traits::ToCss;
+
+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);
+    parse_property_declaration_list(&context, &mut parser)
+}
 
 #[test]
 fn property_declaration_block_should_serialize_correctly() {
     use style::properties::longhands::overflow_x::SpecifiedValue as OverflowXValue;
     use style::properties::longhands::overflow_y::SpecifiedValue as OverflowYContainer;
 
     let declarations = vec![
         (PropertyDeclaration::Width(
@@ -548,56 +559,16 @@ mod shorthand_serialization {
         properties.push(PropertyDeclaration::ColumnWidth(width));
         properties.push(PropertyDeclaration::ColumnCount(count));
 
         let serialization = shorthand_properties_to_string(properties);
         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;
         use style::values::specified::Percentage as PercentageContainer;
 
         let mut properties = Vec::new();
 
         let grow = DeclaredValue::Value(NumberContainer(2f32));
         let shrink = DeclaredValue::Value(NumberContainer(3f32));
@@ -671,230 +642,112 @@ mod shorthand_serialization {
             properties.push(PropertyDeclaration::LineHeight(line_height));
 
             let serialization = shorthand_properties_to_string(properties);
             assert_eq!(serialization, "font:;");
         }
     }
     */
 
-    // 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 style::properties::longhands::background_attachment as attachment;
-        use style::properties::longhands::background_clip as clip;
-        use style::properties::longhands::background_image as image;
-        use style::properties::longhands::background_origin as origin;
-        use style::properties::longhands::background_position_x as position_x;
-        use style::properties::longhands::background_position_y as position_y;
-        use style::properties::longhands::background_repeat as repeat;
-        use style::properties::longhands::background_size as size;
-        use style::values::specified::Image;
-        use style::values::specified::position::{HorizontalPosition, VerticalPosition};
         use super::*;
-        macro_rules! single_vec_value_typedef {
-            ($name:ident, $path:expr) => {
-                DeclaredValue::Value($name::SpecifiedValue(
-                    vec![$path]
-                ))
-            };
-        }
-        macro_rules! single_vec_value {
-            ($name:ident, $path:expr) => {
-                DeclaredValue::Value($name::SpecifiedValue(
-                    vec![$name::single_value::SpecifiedValue($path)]
-                ))
-            };
-        }
-        macro_rules! single_vec_keyword_value {
-            ($name:ident, $kw:ident) => {
-                DeclaredValue::Value($name::SpecifiedValue(
-                    vec![$name::single_value::SpecifiedValue::$kw]
-                ))
-            };
-        }
-        macro_rules! single_vec_variant_value {
-            ($name:ident, $variant:expr) => {
-                DeclaredValue::Value($name::SpecifiedValue(
-                        vec![$variant]
-                ))
-            };
-        }
+
         #[test]
         fn background_should_serialize_all_available_properties_when_specified() {
-            let mut properties = Vec::new();
-
-            let color = DeclaredValue::Value(CSSColor {
-                parsed: ComputedColor::RGBA(RGBA::new(255, 0, 0, 255)),
-                authored: None
-            });
-
-            let position_x = single_vec_value_typedef!(position_x,
-                HorizontalPosition {
-                    keyword: None,
-                    position: Some(LengthOrPercentage::Length(NoCalcLength::from_px(7f32))),
-                }
-            );
-
-            let position_y = single_vec_value_typedef!(position_y,
-                VerticalPosition {
-                    keyword: None,
-                    position: Some(LengthOrPercentage::Length(NoCalcLength::from_px(4f32))),
-                }
-            );
-
-            let repeat = single_vec_keyword_value!(repeat, repeat_x);
-            let attachment = single_vec_keyword_value!(attachment, scroll);
+            let block_text = "\
+                background-color: rgb(255, 0, 0); \
+                background-image: url(\"http://servo/test.png\"); \
+                background-repeat: repeat-x; \
+                background-attachment: scroll; \
+                background-size: 70px 50px; \
+                background-position-x: 7px; \
+                background-position-y: 4px; \
+                background-origin: border-box; \
+                background-clip: padding-box;";
+            let block = parse_declaration_block(block_text);
 
-            let image = single_vec_value!(image,
-                Some(Image::Url(SpecifiedUrl::new_for_testing("http://servo/test.png"))));
-
-            let size = single_vec_variant_value!(size,
-                size::single_value::SpecifiedValue::Explicit(
-                    size::single_value::ExplicitSize {
-                        width: LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(70f32)),
-                        height: LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(50f32))
-                    }
-                )
-            );
-
-            let origin = single_vec_keyword_value!(origin, border_box);
-            let clip = single_vec_keyword_value!(clip, padding_box);
-
-            properties.push(PropertyDeclaration::BackgroundColor(color));
-            properties.push(PropertyDeclaration::BackgroundPositionX(position_x));
-            properties.push(PropertyDeclaration::BackgroundPositionY(position_y));
-            properties.push(PropertyDeclaration::BackgroundRepeat(repeat));
-            properties.push(PropertyDeclaration::BackgroundAttachment(attachment));
-            properties.push(PropertyDeclaration::BackgroundImage(image));
-            properties.push(PropertyDeclaration::BackgroundSize(size));
-            properties.push(PropertyDeclaration::BackgroundOrigin(origin));
-            properties.push(PropertyDeclaration::BackgroundClip(clip));
-
-            let serialization = shorthand_properties_to_string(properties);
+            let serialization = block.to_css_string();
 
             assert_eq!(
                 serialization,
                 "background: rgb(255, 0, 0) url(\"http://servo/test.png\") repeat-x \
                 scroll 7px 4px / 70px 50px border-box padding-box;"
             );
         }
 
         #[test]
         fn background_should_combine_origin_and_clip_properties_when_equal() {
-            let mut properties = Vec::new();
-
-            let color = DeclaredValue::Value(CSSColor {
-                parsed: ComputedColor::RGBA(RGBA::new(255, 0, 0, 255)),
-                authored: None
-            });
-
-            let position_x = single_vec_value_typedef!(position_x,
-                HorizontalPosition {
-                    keyword: None,
-                    position: Some(LengthOrPercentage::Length(NoCalcLength::from_px(7f32))),
-                }
-            );
-
-            let position_y = single_vec_value_typedef!(position_y,
-                VerticalPosition {
-                    keyword: None,
-                    position: Some(LengthOrPercentage::Length(NoCalcLength::from_px(4f32))),
-                }
-            );
-
-            let repeat = single_vec_keyword_value!(repeat, repeat_x);
-            let attachment = single_vec_keyword_value!(attachment, scroll);
+            let block_text = "\
+                background-color: rgb(255, 0, 0); \
+                background-image: url(\"http://servo/test.png\"); \
+                background-repeat: repeat-x; \
+                background-attachment: scroll; \
+                background-size: 70px 50px; \
+                background-position-x: 7px; \
+                background-position-y: 4px; \
+                background-origin: padding-box; \
+                background-clip: padding-box;";
+            let block = parse_declaration_block(block_text);
 
-            let image = single_vec_value!(image,
-                        Some(Image::Url(SpecifiedUrl::new_for_testing("http://servo/test.png"))));
-
-            let size = single_vec_variant_value!(size,
-                size::single_value::SpecifiedValue::Explicit(
-                    size::single_value::ExplicitSize {
-                        width: LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(70f32)),
-                        height: LengthOrPercentageOrAuto::Length(NoCalcLength::from_px(50f32))
-                    }
-                )
-            );
+            let serialization = block.to_css_string();
 
-            let origin = single_vec_keyword_value!(origin, padding_box);
-            let clip = single_vec_keyword_value!(clip, padding_box);
-
-            properties.push(PropertyDeclaration::BackgroundColor(color));
-            properties.push(PropertyDeclaration::BackgroundPositionX(position_x));
-            properties.push(PropertyDeclaration::BackgroundPositionY(position_y));
-            properties.push(PropertyDeclaration::BackgroundRepeat(repeat));
-            properties.push(PropertyDeclaration::BackgroundAttachment(attachment));
-            properties.push(PropertyDeclaration::BackgroundImage(image));
-            properties.push(PropertyDeclaration::BackgroundSize(size));
-            properties.push(PropertyDeclaration::BackgroundOrigin(origin));
-            properties.push(PropertyDeclaration::BackgroundClip(clip));
-
-            let serialization = shorthand_properties_to_string(properties);
             assert_eq!(
                 serialization,
                 "background: rgb(255, 0, 0) url(\"http://servo/test.png\") repeat-x \
                 scroll 7px 4px / 70px 50px padding-box;"
             );
         }
 
         #[test]
-        fn background_should_always_print_color_and_url_and_repeat_and_attachment_and_position() {
-            let mut properties = Vec::new();
-
-            let color = DeclaredValue::Value(CSSColor {
-                parsed: ComputedColor::RGBA(RGBA::new(255, 0, 0, 255)),
-                authored: None
-            });
+        fn serialize_multiple_backgrounds() {
+            let block_text = "\
+                background-color: rgb(0, 0, 255); \
+                background-image: url(\"http://servo/test.png\"), none; \
+                background-repeat: repeat-x, repeat-y; \
+                background-attachment: scroll, scroll; \
+                background-size: 70px 50px, 20px 30px; \
+                background-position-x: 7px, 70px; \
+                background-position-y: 4px, 40px; \
+                background-origin: border-box, padding-box; \
+                background-clip: padding-box, padding-box;";
+            let block = parse_declaration_block(block_text);
 
-            let position_x = single_vec_value_typedef!(position_x,
-                HorizontalPosition {
-                    keyword: None,
-                    position: Some(LengthOrPercentage::Length(NoCalcLength::from_px(0f32))),
-                }
-            );
+            let serialization = block.to_css_string();
 
-            let position_y = single_vec_value_typedef!(position_y,
-                VerticalPosition {
-                    keyword: None,
-                    position: Some(LengthOrPercentage::Length(NoCalcLength::from_px(0f32))),
-                }
+            assert_eq!(
+                serialization, "background: \
+                url(\"http://servo/test.png\") repeat-x scroll 7px 4px / 70px 50px border-box padding-box, \
+                rgb(0, 0, 255) none repeat-y scroll 70px 40px / 20px 30px padding-box;"
             );
+        }
 
-            let repeat = single_vec_keyword_value!(repeat, repeat_x);
-            let attachment = single_vec_keyword_value!(attachment, scroll);
-
-            let image = single_vec_value!(image, None);
-
-            let size = DeclaredValue::Initial;
-
-            let origin = DeclaredValue::Initial;
-            let clip = DeclaredValue::Initial;
+        #[test]
+        fn serialize_multiple_backgrounds_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 )
+            // With background, the color is one exception as it should only appear once for
+            // multiple backgrounds.
+            // Below, background-position and background-origin only have one value.
+            let block_text = "\
+                background-color: rgb(0, 0, 255); \
+                background-image: url(\"http://servo/test.png\"), none; \
+                background-repeat: repeat-x, repeat-y; \
+                background-attachment: scroll, scroll; \
+                background-size: 70px 50px, 20px 30px; \
+                background-position: 7px 4px; \
+                background-origin: border-box; \
+                background-clip: padding-box, padding-box;";
+            let block = parse_declaration_block(block_text);
 
-            properties.push(PropertyDeclaration::BackgroundColor(color));
-            properties.push(PropertyDeclaration::BackgroundPositionX(position_x));
-            properties.push(PropertyDeclaration::BackgroundPositionY(position_y));
-            properties.push(PropertyDeclaration::BackgroundRepeat(repeat));
-            properties.push(PropertyDeclaration::BackgroundAttachment(attachment));
-            properties.push(PropertyDeclaration::BackgroundImage(image));
-            properties.push(PropertyDeclaration::BackgroundSize(size));
-            properties.push(PropertyDeclaration::BackgroundOrigin(origin));
-            properties.push(PropertyDeclaration::BackgroundClip(clip));
+            let serialization = block.to_css_string();
 
-            let serialization = shorthand_properties_to_string(properties);
-            assert_eq!(serialization, "background: rgb(255, 0, 0) none repeat-x scroll 0px 0px;");
+            assert_eq!(serialization, block_text);
         }
     }
 
     mod mask {
         use style::properties::longhands::mask_clip as clip;
         use style::properties::longhands::mask_composite as composite;
         use style::properties::longhands::mask_image as image;
         use style::properties::longhands::mask_mode as mode;
@@ -1135,50 +988,37 @@ mod shorthand_serialization {
 
             assert_eq!(try_serialize.is_ok(), true);
             assert_eq!(s, "none");
         }
     }
 
     mod animation {
         pub use super::*;
-        use cssparser::Parser;
-        use media_queries::CSSErrorReporterTest;
-        use servo_url::ServoUrl;
-        use style::parser::ParserContext;
-        use style::properties::{parse_property_declaration_list, PropertyDeclarationBlock};
-        use style::stylesheets::Origin;
-
-        fn property_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);
-            parse_property_declaration_list(&context, &mut parser)
-        }
 
         #[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;\
                 animation-delay: 0s;\
                 animation-direction: normal;\
                 animation-fill-mode: forwards;\
                 animation-iteration-count: infinite;\
                 animation-play-state: paused;");
 
             let serialization = block.to_css_string();
 
             assert_eq!(serialization, "animation: 1s ease-in 0s normal forwards infinite paused bounce;")
         }
 
         #[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;\
                 animation-delay: 0s, 1s;\
                 animation-direction: normal, reverse;\
                 animation-fill-mode: forwards, backwards;\
                 animation-iteration-count: infinite, 2;\
                 animation-play-state: paused, running;");
@@ -1190,43 +1030,94 @@ mod shorthand_serialization {
                                    0.2s linear 1s reverse backwards 2 running roll;");
         }
 
         #[test]
         fn serialize_multiple_animations_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 (both here and for background and transition. See
+            // lists have the same length (this affects background, transition and animation).
             // https://github.com/servo/servo/issues/15398 )
             let block_text = "\
                 animation-name: bounce, roll, flip, jump; \
                 animation-duration: 1s, 0.2s; \
                 animation-timing-function: ease-in, linear; \
                 animation-delay: 0s, 1s, 0.5s; \
                 animation-direction: normal; \
                 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);
         }
 
         #[test]
         fn serialize_multiple_without_all_properties_returns_longhand() {
             // timing function and direction are missing, so no shorthand is returned.
             let block_text = "animation-name: bounce, roll; \
                               animation-duration: 1s, 0.2s; \
                               animation-delay: 0s, 1s; \
                               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();
 
             assert_eq!(serialization, block_text);
         }
     }
 }