Make background-position a shorthand instead of a longhand

components/layout/display_list_builder.rs

@@ -755,11 +755,12 @@ impl FragmentDisplayListBuilding for Fragment {
                 }
             };
 
-            let position = *get_cyclic(&background.background_position.0, index);
+            let horiz_position = *get_cyclic(&background.background_position_x.0, index);
+            let vert_position = *get_cyclic(&background.background_position_y.0, index);
             // Use `background-position` to get the offset.
-            let horizontal_position = model::specified(position.horizontal,
+            let horizontal_position = model::specified(horiz_position.0,
                                                        bounds.size.width - image_size.width);
-            let vertical_position = model::specified(position.vertical,
+            let vertical_position = model::specified(vert_position.0,
                                                      bounds.size.height - image_size.height);
 
             // The anchor position for this background, based on both the background-attachment

components/script/dom/webidls/CSSStyleDeclaration.webidl

@@ -38,6 +38,10 @@ partial interface CSSStyleDeclaration {
   [SetterThrows, TreatNullAs=EmptyString] attribute DOMString background-color;
   [SetterThrows, TreatNullAs=EmptyString] attribute DOMString backgroundPosition;
   [SetterThrows, TreatNullAs=EmptyString] attribute DOMString background-position;
+  [SetterThrows, TreatNullAs=EmptyString] attribute DOMString backgroundPositionX;
+  [SetterThrows, TreatNullAs=EmptyString] attribute DOMString background-position-x;
+  [SetterThrows, TreatNullAs=EmptyString] attribute DOMString backgroundPositionY;
+  [SetterThrows, TreatNullAs=EmptyString] attribute DOMString background-position-y;
   [SetterThrows, TreatNullAs=EmptyString] attribute DOMString backgroundRepeat;
   [SetterThrows, TreatNullAs=EmptyString] attribute DOMString background-repeat;
   [SetterThrows, TreatNullAs=EmptyString] attribute DOMString backgroundImage;

components/style/properties/gecko.mako.rs

@@ -1384,6 +1384,7 @@ fn static_assert() {
         }
     </%self:simple_image_array_property>
 
+    % if shorthand != "background":
     pub fn copy_${shorthand}_position_from(&mut self, other: &Self) {
         use gecko_bindings::structs::nsStyleImageLayers_LayerType as LayerType;
 
@@ -1419,7 +1420,7 @@ fn static_assert() {
     pub fn clone_${shorthand}_position(&self)
         -> longhands::${shorthand}_position::computed_value::T {
         use values::computed::position::Position;
-        longhands::background_position::computed_value::T(
+        longhands::${shorthand}_position::computed_value::T(
             self.gecko.${image_layers_field}.mLayers.iter()
                 .take(self.gecko.${image_layers_field}.mPositionXCount as usize)
                 .take(self.gecko.${image_layers_field}.mPositionYCount as usize)
@@ -1448,6 +1449,7 @@ fn static_assert() {
             geckolayer.mPosition.mYPosition = servo.vertical.into();
         }
     }
+    % endif
 
     <%self:simple_image_array_property name="size" shorthand="${shorthand}" field_name="mSize">
         use gecko_bindings::structs::nsStyleImageLayers_Size_Dimension;

components/style/properties/helpers/animated_properties.mako.rs

@@ -7,7 +7,8 @@ use cssparser::{Color as CSSParserColor, Parser, RGBA};
 use euclid::{Point2D, Size2D};
 use properties::PropertyDeclaration;
 use properties::longhands;
-use properties::longhands::background_position::computed_value::T as BackgroundPosition;
+use properties::longhands::background_position_x::computed_value::T as BackgroundPositionX;
+use properties::longhands::background_position_y::computed_value::T as BackgroundPositionY;
 use properties::longhands::background_size::computed_value::T as BackgroundSize;
 use properties::longhands::font_weight::computed_value::T as FontWeight;
 use properties::longhands::line_height::computed_value::T as LineHeight;
@@ -574,13 +575,6 @@ impl Interpolate for Position {
 
 impl RepeatableListInterpolate for Position {}
 
-impl Interpolate for BackgroundPosition {
-    #[inline]
-    fn interpolate(&self, other: &Self, progress: f64) -> Result<Self, ()> {
-        Ok(BackgroundPosition(try!(self.0.interpolate(&other.0, progress))))
-    }
-}
-
 /// https://drafts.csswg.org/css-transitions/#animtype-simple-list
 impl Interpolate for HorizontalPosition {
     #[inline]
@@ -601,24 +595,19 @@ impl Interpolate for VerticalPosition {
 
 impl RepeatableListInterpolate for VerticalPosition {}
 
-% if product == "gecko":
+impl Interpolate for BackgroundPositionX {
-    use properties::longhands::background_position_x::computed_value::T as BackgroundPositionX;
-    use properties::longhands::background_position_y::computed_value::T as BackgroundPositionY;
-
-    impl Interpolate for BackgroundPositionX {
     #[inline]
     fn interpolate(&self, other: &Self, progress: f64) -> Result<Self, ()> {
         Ok(BackgroundPositionX(try!(self.0.interpolate(&other.0, progress))))
     }
-    }
+}
 
-    impl Interpolate for BackgroundPositionY {
+impl Interpolate for BackgroundPositionY {
     #[inline]
     fn interpolate(&self, other: &Self, progress: f64) -> Result<Self, ()> {
         Ok(BackgroundPositionY(try!(self.0.interpolate(&other.0, progress))))
     }
-    }
+}
-% endif
 
 /// https://drafts.csswg.org/css-transitions/#animtype-shadow-list
 impl Interpolate for TextShadow {

components/style/properties/longhand/background.mako.rs

@@ -86,52 +86,7 @@ ${helpers.predefined_type("background-color", "CSSColor",
     }
 </%helpers:vector_longhand>
 
-<%helpers:vector_longhand name="background-position" animatable="True">
+<%helpers:vector_longhand name="background-position-x" animatable="True">
-        use std::fmt;
-        use style_traits::ToCss;
-        use values::HasViewportPercentage;
-        use values::specified::position::Position;
-
-        pub mod computed_value {
-            use values::computed::position::Position;
-            use properties::animated_properties::{Interpolate, RepeatableListInterpolate};
-
-            pub type T = Position;
-        }
-
-        pub type SpecifiedValue = Position;
-
-        #[inline]
-        pub fn get_initial_value() -> computed_value::T {
-            use values::computed::position::Position;
-            Position {
-                horizontal: computed::LengthOrPercentage::Percentage(0.0),
-                vertical: computed::LengthOrPercentage::Percentage(0.0),
-            }
-        }
-        #[inline]
-        pub fn get_initial_specified_value() -> SpecifiedValue {
-            use values::specified::Percentage;
-            use values::specified::position::{HorizontalPosition, VerticalPosition};
-            Position {
-                horizontal: HorizontalPosition {
-                    keyword: None,
-                    position: Some(specified::LengthOrPercentage::Percentage(Percentage(0.0))),
-                },
-                vertical: VerticalPosition {
-                    keyword: None,
-                    position: Some(specified::LengthOrPercentage::Percentage(Percentage(0.0))),
-                },
-            }
-        }
-
-        pub fn parse(context: &ParserContext, input: &mut Parser)
-                     -> Result<SpecifiedValue, ()> {
-            Ok(try!(Position::parse(context, input)))
-        }
-</%helpers:vector_longhand>
-
-<%helpers:vector_longhand name="background-position-x" products="gecko" animatable="True">
         use std::fmt;
         use style_traits::ToCss;
         use values::HasViewportPercentage;
@@ -174,7 +129,7 @@ ${helpers.predefined_type("background-color", "CSSColor",
         }
 </%helpers:vector_longhand>
 
-<%helpers:vector_longhand name="background-position-y" products="gecko" animatable="True">
+<%helpers:vector_longhand name="background-position-y" animatable="True">
         use std::fmt;
         use style_traits::ToCss;
         use values::HasViewportPercentage;

components/style/properties/longhand/svg.mako.rs

@@ -91,21 +91,59 @@ ${helpers.single_keyword("mask-repeat",
                          products="gecko",
                          animatable=False)}
 
-<%helpers:longhand name="mask-position" products="gecko" animatable="True">
+<%helpers:vector_longhand name="mask-position" products="gecko" animatable="True">
-    use properties::longhands::background_position;
+    use std::fmt;
-    pub use ::properties::longhands::background_position::SpecifiedValue;
+    use style_traits::ToCss;
-    pub use ::properties::longhands::background_position::single_value as single_value;
+    use values::HasViewportPercentage;
-    pub use ::properties::longhands::background_position::computed_value as computed_value;
+    use values::specified::position::Position;
+
+    pub mod computed_value {
+        use values::computed::position::Position;
+        use properties::animated_properties::{Interpolate, RepeatableListInterpolate};
+        use properties::longhands::mask_position::computed_value::T as MaskPosition;
+
+        pub type T = Position;
+
+        impl RepeatableListInterpolate for MaskPosition {}
+
+        impl Interpolate for MaskPosition {
+            fn interpolate(&self, other: &Self, progress: f64) -> Result<Self, ()> {
+                Ok(MaskPosition(try!(self.0.interpolate(&other.0, progress))))
+            }
+        }
+    }
+
+    pub type SpecifiedValue = Position;
 
     #[inline]
     pub fn get_initial_value() -> computed_value::T {
-        background_position::get_initial_value()
+        use values::computed::position::Position;
+        Position {
+            horizontal: computed::LengthOrPercentage::Percentage(0.0),
+            vertical: computed::LengthOrPercentage::Percentage(0.0),
+        }
+    }
+    #[inline]
+    pub fn get_initial_specified_value() -> SpecifiedValue {
+        use values::specified::Percentage;
+        use values::specified::position::{HorizontalPosition, VerticalPosition};
+        Position {
+            horizontal: HorizontalPosition {
+                keyword: None,
+                position: Some(specified::LengthOrPercentage::Percentage(Percentage(0.0))),
+            },
+            vertical: VerticalPosition {
+                keyword: None,
+                position: Some(specified::LengthOrPercentage::Percentage(Percentage(0.0))),
+            },
+        }
     }
 
-    pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
+    pub fn parse(context: &ParserContext, input: &mut Parser)
-        background_position::parse(context, input)
+                 -> Result<SpecifiedValue, ()> {
+        Position::parse(context, input)
     }
-</%helpers:longhand>
+</%helpers:vector_longhand>
 
 // missing: margin-box fill-box stroke-box view-box no-clip
 // (gecko doesn't implement these)

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

@@ -6,10 +6,14 @@
 
 // TODO: other background-* properties
 <%helpers:shorthand name="background"
-                    sub_properties="background-color background-position background-repeat background-attachment
+                    sub_properties="background-color background-position-x background-position-y background-repeat
-                                    background-image background-size background-origin background-clip">
+                                    background-attachment background-image background-size background-origin
-    use properties::longhands::{background_color, background_position, background_repeat, background_attachment};
+                                    background-clip">
-    use properties::longhands::{background_image, background_size, background_origin, background_clip};
+    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 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) ->
@@ -28,11 +32,11 @@
     pub fn parse_value(context: &ParserContext, input: &mut Parser) -> Result<Longhands, ()> {
         let mut background_color = None;
 
-        % for name in "image position repeat size attachment origin clip".split():
+        % for name in "image position_x position_y repeat size attachment origin clip".split():
             let mut background_${name} = background_${name}::SpecifiedValue(Vec::new());
         % endfor
         try!(input.parse_comma_separated(|input| {
-            % for name in "image position repeat size attachment origin clip".split():
+            % for name in "image position_x position_y repeat size attachment origin clip".split():
                 let mut ${name} = None;
             % endfor
             loop {
@@ -45,10 +49,10 @@
                         return Err(())
                     }
                 }
-                if position.is_none() {
+                if position_x.is_none() && position_y.is_none() {
-                    if let Ok(value) = input.try(|input| background_position::single_value
+                    if let Ok(value) = input.try(|input| Position::parse(context, input)) {
-                                                                            ::parse(context, input)) {
+                        position_x = Some(value.horizontal);
-                        position = Some(value);
+                        position_y = Some(value.vertical);
 
                         // Parse background size, if applicable.
                         size = input.try(|input| {
@@ -76,12 +80,24 @@
                 }
             }
             let mut any = false;
-            % for name in "image position repeat size attachment origin clip".split():
+            % for name in "image position_x position_y repeat size attachment origin clip".split():
                 any = any || ${name}.is_some();
             % endfor
             any = any || background_color.is_some();
             if any {
-                % for name in "image position repeat size attachment origin clip".split():
+                if position_x.is_some() || position_y.is_some() {
+                    % for name in "position_x position_y".split():
+                        if let Some(bg_${name}) = ${name} {
+                            background_${name}.0.push(bg_${name});
+                        }
+                    % endfor
+                } else {
+                    % for name in "position_x position_y".split():
+                        background_${name}.0.push(background_${name}::single_value
+                                                                    ::get_initial_position_value());
+                    % endfor
+                }
+                % for name in "image repeat size attachment origin clip".split():
                     if let Some(bg_${name}) = ${name} {
                         background_${name}.0.push(bg_${name});
                     } else {
@@ -98,7 +114,8 @@
         Ok(Longhands {
              background_color: background_color,
              background_image: Some(background_image),
-             background_position: Some(background_position),
+             background_position_x: Some(background_position_x),
+             background_position_y: Some(background_position_y),
              background_repeat: Some(background_repeat),
              background_attachment: Some(background_attachment),
              background_size: Some(background_size),
@@ -118,7 +135,7 @@
             }
             use std::cmp;
             let mut len = 0;
-            % for name in "image position repeat size attachment origin clip".split():
+            % 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
@@ -130,7 +147,7 @@
 
             let mut first = true;
             for i in 0..len {
-                % for name in "image position repeat size attachment origin clip".split():
+                % 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())
                     } else {
@@ -185,8 +202,14 @@
 
                 try!(write!(dest, " "));
 
-                try!(position.unwrap_or(&background_position::single_value
+                try!(position_x.unwrap_or(&background_position_x::single_value
-                                                            ::get_initial_specified_value())
+                                                                ::get_initial_position_value())
+                             .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 {
@@ -223,6 +246,87 @@
             }
 
 
+            Ok(())
+        }
+    }
+</%helpers:shorthand>
+
+<%helpers:shorthand name="background-position"
+                    sub_properties="background-position-x background-position-y">
+    use properties::longhands::{background_position_x,background_position_y};
+    use values::specified::position::Position;
+    use parser::Parse;
+
+    pub fn parse_value(context: &ParserContext, input: &mut Parser) -> Result<Longhands, ()> {
+        let mut position_x = background_position_x::SpecifiedValue(Vec::new());
+        let mut position_y = background_position_y::SpecifiedValue(Vec::new());
+        let mut any = false;
+
+        try!(input.parse_comma_separated(|input| {
+            loop {
+                if let Ok(value) = input.try(|input| Position::parse(context, input)) {
+                    position_x.0.push(value.horizontal);
+                    position_y.0.push(value.vertical);
+                    any = true;
+                    continue
+                }
+                break
+            }
+            Ok(())
+        }));
+        if any == false {
+            return Err(());
+        }
+
+        Ok(Longhands {
+            background_position_x: Some(position_x),
+            background_position_y: Some(position_y),
+        })
+    }
+
+    impl<'a> LonghandsToSerialize<'a>  {
+        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 "x y".split():
+                len = cmp::max(len, extract_value(self.background_position_${name})
+                                                      .map(|i| i.0.len())
+                                                      .unwrap_or(0));
+            % endfor
+
+            // There should be at least one declared value
+            if len == 0 {
+                return dest.write_str("")
+            }
+
+            for i in 0..len {
+                % for name in "x y".split():
+                    let position_${name} = if let DeclaredValue::Value(ref arr) =
+                                           *self.background_position_${name} {
+                        arr.0.get(i % arr.0.len())
+                    } else {
+                        None
+                    };
+                % endfor
+
+                try!(position_x.unwrap_or(&background_position_x::single_value
+                                                                ::get_initial_position_value())
+                               .to_css(dest));
+
+                try!(write!(dest, " "));
+
+                try!(position_y.unwrap_or(&background_position_y::single_value
+                                                                ::get_initial_position_value())
+                               .to_css(dest));
+            }
+
             Ok(())
         }
     }

components/style/servo/restyle_damage.rs

@@ -232,10 +232,10 @@ fn compute_damage(old: &ServoComputedValues, new: &ServoComputedValues) -> Servo
     ]) || add_if_not_equal!(old, new, damage,
                             [REPAINT], [
         get_color.color, get_background.background_color,
-        get_background.background_image, get_background.background_position,
+        get_background.background_image, get_background.background_position_x,
-        get_background.background_repeat, get_background.background_attachment,
+        get_background.background_position_y, get_background.background_repeat,
-        get_background.background_clip, get_background.background_origin,
+        get_background.background_attachment, get_background.background_clip,
-        get_background.background_size,
+        get_background.background_origin, get_background.background_size,
         get_border.border_top_color, get_border.border_right_color,
         get_border.border_bottom_color, get_border.border_left_color,
         get_border.border_top_style, get_border.border_right_style,