Add serialize_four_sides, use for serializing BorderRadius

components/style/properties/properties.mako.rs

@@ -73,7 +73,8 @@ pub mod longhands {
 }
 
 pub mod shorthands {
-    use cssparser::Parser;
+    use cssparser::{Parser, ToCss};
+    use std::fmt;
     use parser::ParserContext;
     use values::specified;
 
@@ -120,6 +121,33 @@ pub mod shorthands {
         Ok((top, right, bottom, left))
     }
 
+    /// Serialize a set of top,left,bottom,right values, in <margin>-shorthand style,
+    /// attempting to minimize the output
+    pub fn serialize_four_sides<T, W>(sides: (&T, &T, &T, &T), dest: &mut W) -> fmt::Result
+        where W: fmt::Write, T: ToCss+PartialEq {
+        if sides.0 == sides.1 && sides.0 == sides.2 && sides.0 == sides.3 {
+            sides.0.to_css(dest)
+        } else if sides.0 == sides.2 && sides.1 == sides.3 {
+            try!(sides.0.to_css(dest));
+            try!(dest.write_str(" "));
+            sides.1.to_css(dest)
+        } else if sides.1 == sides.3 {
+            try!(sides.0.to_css(dest));
+            try!(dest.write_str(" "));
+            try!(sides.1.to_css(dest));
+            try!(dest.write_str(" "));
+            sides.2.to_css(dest)
+        } else {
+            try!(sides.0.to_css(dest));
+            try!(dest.write_str(" "));
+            try!(sides.1.to_css(dest));
+            try!(dest.write_str(" "));
+            try!(sides.2.to_css(dest));
+            try!(dest.write_str(" "));
+            sides.3.to_css(dest)
+        }
+    }
+
     <%include file="/shorthand/background.mako.rs" />
     <%include file="/shorthand/border.mako.rs" />
     <%include file="/shorthand/box.mako.rs" />

components/style/values/specified/basic_shape.rs

@@ -9,7 +9,7 @@
 
 use app_units::Au;
 use cssparser::{Parser, ToCss};
-use properties::shorthands::parse_four_sides;
+use properties::shorthands::{parse_four_sides, serialize_four_sides};
 use std::fmt;
 use values::computed::basic_shape as computed_basic_shape;
 use values::computed::{Context, ToComputedValue, ComputedValueAsSpecified};
@@ -408,21 +408,28 @@ impl ToCss for BorderRadius {
     // a helper function somewhere, for all the parse_four_sides-like
     // values
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-        try!(self.top_left.0.width.to_css(dest));
+        if self.top_left.0.width == self.top_left.0.height &&
-        try!(dest.write_str(" "));
+           self.top_right.0.width == self.top_right.0.height &&
-        try!(self.top_right.0.width.to_css(dest));
+           self.bottom_left.0.width == self.bottom_left.0.height &&
-        try!(dest.write_str(" "));
+           self.bottom_right.0.width == self.bottom_right.0.height {
-        try!(self.bottom_left.0.width.to_css(dest));
+            serialize_four_sides((&self.top_left.0.width,
-        try!(dest.write_str(" "));
+                                  &self.top_right.0.width,
-        try!(self.bottom_right.0.width.to_css(dest));
+                                  &self.bottom_left.0.width,
-        try!(dest.write_str(" / "));
+                                  &self.bottom_right.0.width),
-        try!(self.top_left.0.height.to_css(dest));
+                                  dest)
-        try!(dest.write_str(" "));
+        } else {
-        try!(self.top_right.0.height.to_css(dest));
+            try!(serialize_four_sides((&self.top_left.0.width,
-        try!(dest.write_str(" "));
+                                       &self.top_right.0.width,
-        try!(self.bottom_left.0.height.to_css(dest));
+                                       &self.bottom_left.0.width,
-        try!(dest.write_str(" "));
+                                       &self.bottom_right.0.width),
-        self.bottom_right.0.height.to_css(dest)
+                                       dest));
+            try!(dest.write_str(" / "));
+            serialize_four_sides((&self.top_left.0.height,
+                                  &self.top_right.0.height,
+                                  &self.bottom_left.0.height,
+                                  &self.bottom_right.0.height),
+                                  dest)
+        }
     }
 }
 

tests/unit/style/parsing/basic_shape.rs

@@ -2,7 +2,7 @@
  * 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/. */
 
-use parsing::parse;
+use parsing::{parse, to_string};
 use style::values::specified::basic_shape::*;
 
 // Ensure that basic-shape sub-functions parse as both basic shapes
@@ -14,6 +14,23 @@ macro_rules! assert_roundtrip_basicshape {
     }
 }
 
+macro_rules! assert_border_radius_values {
+    ($input:expr; $tlw:expr, $trw:expr, $blw:expr, $brw:expr ;
+                  $tlh:expr, $trh:expr, $blh:expr, $brh:expr) => {
+        let input = parse(BorderRadius::parse, $input)
+                          .expect(&format!("Failed parsing {} as border radius",
+                                  $input));
+        assert_eq!(to_string(input.top_left.0.width), $tlw);
+        assert_eq!(to_string(input.top_right.0.width), $trw);
+        assert_eq!(to_string(input.bottom_left.0.width), $blw);
+        assert_eq!(to_string(input.bottom_right.0.width), $brw);
+        assert_eq!(to_string(input.top_left.0.height), $tlh);
+        assert_eq!(to_string(input.top_right.0.height), $trh);
+        assert_eq!(to_string(input.bottom_left.0.height), $blh);
+        assert_eq!(to_string(input.bottom_right.0.height), $brh);
+    }
+}
+
 #[test]
 fn test_inset() {
     // these are actually wrong, we should be serializing to the minimum possible result
@@ -23,17 +40,40 @@ fn test_inset() {
     assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px 20%)", "inset(10px 20% 10px 20%)");
 
     assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px round 10px)",
-                                                   "inset(10px 10px 10px 10px round 10px 10px 10px 10px \
+                                                   "inset(10px 10px 10px 10px round 10px)");
-                                                    / 10px 10px 10px 10px)");
     assert_roundtrip_basicshape!(InsetRect::parse, "inset(10px round 10px 20px 30px 40px)",
-                                                   "inset(10px 10px 10px 10px round 10px 20px 30px 40px \
+                                                   "inset(10px 10px 10px 10px round 10px 20px 30px 40px)");
-                                                    / 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)");
 }
 
+#[test]
+fn test_border_radius() {
+    assert_border_radius_values!("10px";
+                                 "10px", "10px", "10px", "10px" ;
+                                 "10px", "10px", "10px", "10px");
+    assert_border_radius_values!("10px 20px";
+                                 "10px", "20px", "10px", "20px" ;
+                                 "10px", "20px", "10px", "20px");
+    assert_border_radius_values!("10px 20px 30px";
+                                 "10px", "20px", "30px", "20px" ;
+                                 "10px", "20px", "30px", "20px");
+    assert_border_radius_values!("10px 20px 30px 40px";
+                                 "10px", "20px", "30px", "40px" ;
+                                 "10px", "20px", "30px", "40px");
+    assert_border_radius_values!("10% / 20px";
+                                 "10%", "10%", "10%", "10%" ;
+                                 "20px", "20px", "20px", "20px");
+    assert_border_radius_values!("10px / 20px 30px";
+                                 "10px", "10px", "10px", "10px" ;
+                                 "20px", "30px", "20px", "30px");
+    assert_border_radius_values!("10px 20px 30px 40px / 1px 2px 3px 4px";
+                                 "10px", "20px", "30px", "40px" ;
+                                 "1px", "2px", "3px", "4px");
+}
+
 #[test]
 fn test_circle() {
     assert_roundtrip_basicshape!(Circle::parse, "circle(at center)", "circle(at 50% 50%)");

tests/unit/style/parsing/mod.rs

@@ -4,26 +4,30 @@
 
 //! Tests for parsing and serialization of values/properties
 
-use cssparser::Parser;
+use cssparser::{Parser, ToCss};
 
 fn parse<T, F: Fn(&mut Parser) -> Result<T, ()>>(f: F, s: &str) -> Result<T, ()> {
     let mut parser = Parser::new(s);
     f(&mut parser)
 }
 
+fn to_string<T: ToCss>(x: T) -> String {
+    let mut serialized = String::new();
+    x.to_css(&mut serialized).expect("Failed to serialize");
+    serialized
+}
+
 // This is a macro so that the file/line information
 // is preserved in the panic
 macro_rules! assert_roundtrip {
     ($fun:expr, $input:expr, $output:expr) => {
         let parsed = $crate::parsing::parse($fun, $input)
                         .expect(&format!("Failed to parse {}", $input));
-        let mut serialized = String::new();
+        let serialized = $crate::parsing::to_string(parsed);
-        ::cssparser::ToCss::to_css(&parsed, &mut serialized)
-                        .expect("Failed to serialize");
         assert_eq!(serialized, $output);
     }
 }
 
 
 mod basic_shape;
-mod position;
+mod position;

tests/unit/style/parsing/position.rs

@@ -25,6 +25,10 @@ fn test_position() {
     assert_roundtrip!(Position::parse, "center 10%", "50% 10%");
     assert_roundtrip!(Position::parse, "right 10%", "100% 10%");
 
+    // Only keywords can be reordered
+    assert!(parse(Position::parse, "top 40%").is_err());
+    assert!(parse(Position::parse, "40% left").is_err());
+
     // we don't yet handle 4-valued positions
     // https://github.com/servo/servo/issues/12690
 }