servo: Merge #15416 - Fix border shorthand serialization (from karlding:servo-15395_border_serialization); r=emilio
authorKarl <karlding@users.noreply.github.com>
Wed, 01 Mar 2017 09:50:49 -0800
changeset 374471 01131dccb20c5228b2826356ae6ed1c7d479a40a
parent 374470 b25d9cfea3f9b5b822109af7ea4d90ae04463a7e
child 374472 42093254f824bc2676ca935ec2c11bcf8c6d0574
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
milestone54.0a1
servo: Merge #15416 - Fix border shorthand serialization (from karlding:servo-15395_border_serialization); r=emilio Fix border shorthand serialization when sides are not identical. I think I managed to get the serialization to work as expected. I added a check to ensure that the **border-width**, **border-style** and **border-color** were the same, before performing the serialization. I'm still a Rust newbie, so if there's a more idiomatic way of doing things (or any critiques in general), please let me know! --- <!-- 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 - [X] These changes fix #15395 <!-- Either: --> - [X] There are tests for these changes <!-- 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: f0257364c26ef89e7652eabe73a3788388adf5ca
servo/components/style/properties/shorthand/border.mako.rs
servo/tests/unit/style/properties/serialization.rs
--- a/servo/components/style/properties/shorthand/border.mako.rs
+++ b/servo/components/style/properties/shorthand/border.mako.rs
@@ -1,14 +1,14 @@
 /* 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/. */
 
 <%namespace name="helpers" file="/helpers.mako.rs" />
-<% from data import to_rust_ident, ALL_SIDES, maybe_moz_logical_alias %>
+<% from data import to_rust_ident, ALL_SIDES, PHYSICAL_SIDES, maybe_moz_logical_alias %>
 
 ${helpers.four_sides_shorthand("border-color", "border-%s-color", "specified::CSSColor::parse",
                                spec="https://drafts.csswg.org/css-backgrounds/#border-color")}
 
 ${helpers.four_sides_shorthand("border-style", "border-%s-style",
                                "specified::BorderStyle::parse",
                                spec="https://drafts.csswg.org/css-backgrounds/#border-style")}
 
@@ -136,24 +136,48 @@ pub fn parse_border(context: &ParserCont
                 border_${side}_style: style,
                 border_${side}_width: width.clone(),
             % endfor
         })
     }
 
     impl<'a> LonghandsToSerialize<'a>  {
         fn to_css_declared<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+            let all_equal = {
+                % for side in PHYSICAL_SIDES:
+                  let border_${side}_width = self.border_${side}_width;
+                  let border_${side}_style = self.border_${side}_style;
+                  let border_${side}_color = self.border_${side}_color;
+                % endfor
+
+                border_top_width == border_right_width &&
+                border_right_width == border_bottom_width &&
+                border_bottom_width == border_left_width &&
+
+                border_top_style == border_right_style &&
+                border_right_style == border_bottom_style &&
+                border_bottom_style == border_left_style &&
+
+                border_top_color == border_right_color &&
+                border_right_color == border_bottom_color &&
+                border_bottom_color == border_left_color
+            };
+
             // If all longhands are all present, then all sides should be the same,
             // so we can just one set of color/style/width
-            super::serialize_directional_border(
-                dest,
-                self.border_${side}_width,
-                self.border_${side}_style,
-                self.border_${side}_color
-            )
+            if all_equal {
+                super::serialize_directional_border(
+                    dest,
+                    self.border_${side}_width,
+                    self.border_${side}_style,
+                    self.border_${side}_color
+                )
+            } else {
+                Ok(())
+            }
         }
     }
 
 </%helpers:shorthand>
 
 <%helpers:shorthand name="border-radius" sub_properties="${' '.join(
     'border-%s-radius' % (corner)
      for corner in ['top-left', 'top-right', 'bottom-right', 'bottom-left']
--- a/servo/tests/unit/style/properties/serialization.rs
+++ b/servo/tests/unit/style/properties/serialization.rs
@@ -182,16 +182,82 @@ mod shorthand_serialization {
             properties.push(PropertyDeclaration::MarginBottom(bottom_px));
             properties.push(PropertyDeclaration::MarginLeft(left_px));
 
             let serialization = shorthand_properties_to_string(properties);
             assert_eq!(serialization, "margin: 8px 12px 10px 14px;");
         }
 
         #[test]
+        fn different_longhands_should_serialize_to_long_form() {
+          let mut properties = Vec::new();
+
+          let solid = DeclaredValue::Value(BorderStyle::solid);
+
+          properties.push(PropertyDeclaration::BorderTopStyle(solid.clone()));
+          properties.push(PropertyDeclaration::BorderRightStyle(solid.clone()));
+          properties.push(PropertyDeclaration::BorderBottomStyle(solid.clone()));
+          properties.push(PropertyDeclaration::BorderLeftStyle(solid.clone()));
+
+          let px_30 = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(30f32)));
+          let px_10 = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(10f32)));
+
+          properties.push(PropertyDeclaration::BorderTopWidth(px_30.clone()));
+          properties.push(PropertyDeclaration::BorderRightWidth(px_30.clone()));
+          properties.push(PropertyDeclaration::BorderBottomWidth(px_30.clone()));
+          properties.push(PropertyDeclaration::BorderLeftWidth(px_10.clone()));
+
+          let blue = DeclaredValue::Value(CSSColor {
+              parsed: ComputedColor::RGBA(RGBA::new(0, 0, 255, 255)),
+              authored: None
+          });
+
+          properties.push(PropertyDeclaration::BorderTopColor(blue.clone()));
+          properties.push(PropertyDeclaration::BorderRightColor(blue.clone()));
+          properties.push(PropertyDeclaration::BorderBottomColor(blue.clone()));
+          properties.push(PropertyDeclaration::BorderLeftColor(blue.clone()));
+
+          let serialization = shorthand_properties_to_string(properties);
+          assert_eq!(serialization,
+          "border-style: solid; border-width: 30px 30px 30px 10px; border-color: rgb(0, 0, 255);");
+        }
+
+        #[test]
+        fn same_longhands_should_serialize_correctly() {
+          let mut properties = Vec::new();
+
+          let solid = DeclaredValue::Value(BorderStyle::solid);
+
+          properties.push(PropertyDeclaration::BorderTopStyle(solid.clone()));
+          properties.push(PropertyDeclaration::BorderRightStyle(solid.clone()));
+          properties.push(PropertyDeclaration::BorderBottomStyle(solid.clone()));
+          properties.push(PropertyDeclaration::BorderLeftStyle(solid.clone()));
+
+          let px_30 = DeclaredValue::Value(BorderWidth::from_length(Length::from_px(30f32)));
+
+          properties.push(PropertyDeclaration::BorderTopWidth(px_30.clone()));
+          properties.push(PropertyDeclaration::BorderRightWidth(px_30.clone()));
+          properties.push(PropertyDeclaration::BorderBottomWidth(px_30.clone()));
+          properties.push(PropertyDeclaration::BorderLeftWidth(px_30.clone()));
+
+          let blue = DeclaredValue::Value(CSSColor {
+              parsed: ComputedColor::RGBA(RGBA::new(0, 0, 255, 255)),
+              authored: None
+          });
+
+          properties.push(PropertyDeclaration::BorderTopColor(blue.clone()));
+          properties.push(PropertyDeclaration::BorderRightColor(blue.clone()));
+          properties.push(PropertyDeclaration::BorderBottomColor(blue.clone()));
+          properties.push(PropertyDeclaration::BorderLeftColor(blue.clone()));
+
+          let serialization = shorthand_properties_to_string(properties);
+          assert_eq!(serialization, "border: 30px solid rgb(0, 0, 255);");
+        }
+
+        #[test]
         fn padding_should_serialize_correctly() {
             let mut properties = Vec::new();
 
             let px_10 = DeclaredValue::Value(LengthOrPercentage::Length(NoCalcLength::from_px(10f32)));
             let px_15 = DeclaredValue::Value(LengthOrPercentage::Length(NoCalcLength::from_px(15f32)));
             properties.push(PropertyDeclaration::PaddingTop(px_10.clone()));
             properties.push(PropertyDeclaration::PaddingRight(px_15.clone()));
             properties.push(PropertyDeclaration::PaddingBottom(px_10));