Use Rect in InsetRect

components/style/gecko/conversions.rs

@@ -369,6 +369,7 @@ pub mod basic_shape {
     use values::generics::basic_shape::{BasicShape as GenericBasicShape, InsetRect, Polygon};
     use values::generics::basic_shape::{Circle, Ellipse, FillRule};
     use values::generics::basic_shape::{GeometryBox, ShapeBox};
+    use values::generics::rect::Rect;
 
     // using Borrow so that we can have a non-moving .into()
     impl<T: Borrow<StyleBasicShape>> From<T> for BasicShape {
@@ -381,11 +382,14 @@ pub mod basic_shape {
                     let b = LengthOrPercentage::from_gecko_style_coord(&other.mCoordinates[2]);
                     let l = LengthOrPercentage::from_gecko_style_coord(&other.mCoordinates[3]);
                     let round = (&other.mRadius).into();
+                    let rect = Rect::new(
+                        t.expect("inset() offset should be a length, percentage, or calc value"),
+                        r.expect("inset() offset should be a length, percentage, or calc value"),
+                        b.expect("inset() offset should be a length, percentage, or calc value"),
+                        l.expect("inset() offset should be a length, percentage, or calc value"),
+                    );
                     GenericBasicShape::Inset(InsetRect {
-                        top: t.expect("inset() offset should be a length, percentage, or calc value"),
-                        right: r.expect("inset() offset should be a length, percentage, or calc value"),
-                        bottom: b.expect("inset() offset should be a length, percentage, or calc value"),
-                        left: l.expect("inset() offset should be a length, percentage, or calc value"),
+                        rect: rect,
                         round: Some(round),
                     })
                 }

components/style/properties/gecko.mako.rs

@@ -3914,7 +3914,7 @@ fn static_assert() {
                     }
                 }
                 match servo_shape {
-                    BasicShape::Inset(rect) => {
+                    BasicShape::Inset(inset) => {
                         let mut shape = init_shape(${ident}, StyleBasicShapeType::Inset);
                         unsafe { shape.mCoordinates.set_len(4) };
 
@@ -3926,15 +3926,15 @@ fn static_assert() {
                         // the garbage data without
                         // attempting to clean up.
                         shape.mCoordinates[0].leaky_set_null();
-                        rect.top.to_gecko_style_coord(&mut shape.mCoordinates[0]);
+                        inset.rect.top.to_gecko_style_coord(&mut shape.mCoordinates[0]);
                         shape.mCoordinates[1].leaky_set_null();
-                        rect.right.to_gecko_style_coord(&mut shape.mCoordinates[1]);
+                        inset.rect.right.to_gecko_style_coord(&mut shape.mCoordinates[1]);
                         shape.mCoordinates[2].leaky_set_null();
-                        rect.bottom.to_gecko_style_coord(&mut shape.mCoordinates[2]);
+                        inset.rect.bottom.to_gecko_style_coord(&mut shape.mCoordinates[2]);
                         shape.mCoordinates[3].leaky_set_null();
-                        rect.left.to_gecko_style_coord(&mut shape.mCoordinates[3]);
+                        inset.rect.left.to_gecko_style_coord(&mut shape.mCoordinates[3]);
 
-                        set_corners_from_radius(rect.round, &mut shape.mRadius);
+                        set_corners_from_radius(inset.round, &mut shape.mRadius);
                     }
                     BasicShape::Circle(circ) => {
                         let mut shape = init_shape(${ident}, StyleBasicShapeType::Circle);

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

@@ -33,11 +33,11 @@ ${helpers.four_sides_shorthand("border-style", "border-%s-style",
 
     impl<'a> ToCss for LonghandsToSerialize<'a>  {
         fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-            let rect = Rect::new(
+            let rect = Rect {
                 % for side in PHYSICAL_SIDES:
-                &self.border_${side}_width,
+                ${side}: &self.border_${side}_width,
                 % endfor
-            );
+            };
             rect.to_css(dest)
         }
     }

components/style/values/generics/basic_shape.rs

@@ -67,10 +67,7 @@ pub enum BasicShape<H, V, LengthOrPercentage> {
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 #[derive(Clone, Debug, PartialEq, ToComputedValue)]
 pub struct InsetRect<LengthOrPercentage> {
-    pub top: LengthOrPercentage,
-    pub right: LengthOrPercentage,
-    pub bottom: LengthOrPercentage,
-    pub left: LengthOrPercentage,
+    pub rect: Rect<LengthOrPercentage>,
     pub round: Option<BorderRadius<LengthOrPercentage>>,
 }
 
@@ -190,22 +187,16 @@ impl<H, V, L> ToCss for BasicShape<H, V, L>
     }
 }
 
-impl<L: ToCss + PartialEq> ToCss for InsetRect<L> {
-    // XXXManishearth We should try to reduce the number of values printed here
+impl<L> ToCss for InsetRect<L>
+    where L: ToCss + PartialEq
+{
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
         dest.write_str("inset(")?;
-        self.top.to_css(dest)?;
-        dest.write_str(" ")?;
-        self.right.to_css(dest)?;
-        dest.write_str(" ")?;
-        self.bottom.to_css(dest)?;
-        dest.write_str(" ")?;
-        self.left.to_css(dest)?;
+        self.rect.to_css(dest)?;
         if let Some(ref radius) = self.round {
             dest.write_str(" round ")?;
             radius.to_css(dest)?;
         }
-
         dest.write_str(")")
     }
 }

components/style/values/specified/basic_shape.rs

@@ -128,17 +128,14 @@ impl InsetRect {
     /// Parse the inner function arguments of `inset()`
     pub fn parse_function_arguments(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
         let rect = Rect::parse_with(context, input, LengthOrPercentage::parse)?;
-        let round_rect = if input.try(|i| i.expect_ident_matching("round")).is_ok() {
+        let round = if input.try(|i| i.expect_ident_matching("round")).is_ok() {
             Some(BorderRadius::parse(context, input)?)
         } else {
             None
         };
         Ok(GenericInsetRect {
-            top: rect.top,
-            right: rect.right,
-            bottom: rect.bottom,
-            left: rect.left,
-            round: round_rect,
+            rect: rect,
+            round: round,
         })
     }
 }

tests/unit/style/parsing/basic_shape.rs

@@ -13,7 +13,10 @@ macro_rules! assert_roundtrip_basicshape {
     ($fun:expr, $input:expr, $output:expr) => {
         assert_roundtrip_with_context!($fun, $input, $output);
         assert_roundtrip_with_context!(BasicShape::parse, $input, $output);
-    }
+    };
+    ($fun:expr, $input:expr) => {
+        assert_roundtrip_basicshape!($fun, $input, $input);
+    };
 }
 
 macro_rules! assert_border_radius_values {
@@ -35,20 +38,12 @@ macro_rules! assert_border_radius_values {
 
 #[test]
 fn test_inset() {
-    // these are actually wrong, we should be serializing to the minimum possible result
-    // the advantage of being wrong is that the roundtrip test actually suffices
-    // for testing the intermediate state
-    assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px)", "inset(10px 10px 10px 10px)");
-    assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px 20%)", "inset(10px 20% 10px 20%)");
+    assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px)");
+    assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px 20%)");
 
-    assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px round 10px)",
-                                                   "inset(10px 10px 10px 10px round 10px)");
-    assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px round 10px 20px 30px 40px)",
-                                                   "inset(10px 10px 10px 10px round 10px 20px 30px 40px)");
-    assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px 10px 10px 10px round 10px 20px 30px 40px \
-                                                    / 1px 2px 3px 4px)",
-                                                   "inset(10px 10px 10px 10px round 10px 20px 30px 40px \
-                                                    / 1px 2px 3px 4px)");
+    assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px round 10px)");
+    assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px round 10px 20px 30px 40px)");
+    assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px round 10px 20px 30px 40px / 1px 2px 3px 4px)");
 }
 
 #[test]