style: Improve border shorthand serialization

Fix some bugs caught by css/cssom/shorthand-values. In particular:

  * Make the shorthand order match the spec.
  * Omit values when we can.

Fix a subtest that wasn't correct. Shorthands can be serialized as long
as !important matches in all components.

Differential Revision: https://phabricator.services.mozilla.com/D180466

components/style/properties/properties.mako.rs

@@ -106,32 +106,6 @@ pub mod shorthands {
     use style_traits::{ParseError, StyleParseErrorKind};
     use crate::values::specified;
 
-    use style_traits::{CssWriter, ToCss};
-    use crate::values::specified::{BorderStyle, Color};
-    use std::fmt::{self, Write};
-
-    fn serialize_directional_border<W, I,>(
-        dest: &mut CssWriter<W>,
-        width: &I,
-        style: &BorderStyle,
-        color: &Color,
-    ) -> fmt::Result
-    where
-        W: Write,
-        I: ToCss,
-    {
-        width.to_css(dest)?;
-        // FIXME(emilio): Should we really serialize the border style if it's
-        // `solid`?
-        dest.write_char(' ')?;
-        style.to_css(dest)?;
-        if *color != Color::CurrentColor {
-            dest.write_char(' ')?;
-            color.to_css(dest)?;
-        }
-        Ok(())
-    }
-
     % for style_struct in data.style_structs:
     include!("${repr(os.path.join(OUT_DIR, 'shorthands/{}.rs'.format(style_struct.name_lower)))[1:-1]}");
     % endfor

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

@@ -107,7 +107,7 @@ pub fn parse_border<'i, 't>(
         engines="gecko servo"
         sub_properties="${' '.join(
             'border-%s-%s' % (side, prop)
-            for prop in ['color', 'style', 'width']
+            for prop in ['width', 'style', 'color']
         )}"
         aliases="${maybe_moz_logical_alias(engine, (side, logical), '-moz-border-%s')}"
         spec="${spec}">
@@ -126,7 +126,7 @@ pub fn parse_border<'i, 't>(
 
     impl<'a> ToCss for LonghandsToSerialize<'a>  {
         fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
-            super::serialize_directional_border(
+            crate::values::specified::border::serialize_directional_border(
                 dest,
                 self.border_${to_rust_ident(side)}_width,
                 self.border_${to_rust_ident(side)}_style,
@@ -141,8 +141,8 @@ pub fn parse_border<'i, 't>(
 <%helpers:shorthand name="border"
     engines="gecko servo"
     sub_properties="${' '.join('border-%s-%s' % (side, prop)
-        for side in PHYSICAL_SIDES
+        for side in PHYSICAL_SIDES for prop in ['width', 'style', 'color']
-        for prop in ['color', 'style', 'width'])}
+        )}
         ${' '.join('border-image-%s' % name
         for name in ['outset', 'repeat', 'slice', 'source', 'width'])}"
     derive_value_info="False"
@@ -205,16 +205,15 @@ pub fn parse_border<'i, 't>(
 
             // If all longhands are all present, then all sides should be the same,
             // so we can just one set of color/style/width
-            if all_equal {
+            if !all_equal {
-                super::serialize_directional_border(
+                return Ok(())
-                    dest,
-                    self.border_${side}_width,
-                    self.border_${side}_style,
-                    self.border_${side}_color
-                )
-            } else {
-                Ok(())
             }
+            crate::values::specified::border::serialize_directional_border(
+                dest,
+                self.border_${side}_width,
+                self.border_${side}_style,
+                self.border_${side}_color
+            )
         }
     }
 
@@ -359,14 +358,27 @@ pub fn parse_border<'i, 't>(
     impl<'a> ToCss for LonghandsToSerialize<'a>  {
         fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
             self.border_image_source.to_css(dest)?;
-            dest.write_char(' ')?;
+            % for name in "slice outset width repeat".split():
-            self.border_image_slice.to_css(dest)?;
+            let has_${name} = *self.border_image_${name} != border_image_${name}::get_initial_specified_value();
-            dest.write_str(" / ")?;
+            % endfor
-            self.border_image_width.to_css(dest)?;
+            let needs_slice = has_slice || has_width || has_outset;
-            dest.write_str(" / ")?;
+            if needs_slice {
-            self.border_image_outset.to_css(dest)?;
+                dest.write_char(' ')?;
-            dest.write_char(' ')?;
+                self.border_image_slice.to_css(dest)?;
-            self.border_image_repeat.to_css(dest)
+                if has_width {
+                    dest.write_str(" / ")?;
+                    self.border_image_width.to_css(dest)?;
+                }
+                if has_outset {
+                    dest.write_str(" / ")?;
+                    self.border_image_outset.to_css(dest)?;
+                }
+            }
+            if has_repeat {
+                dest.write_char(' ')?;
+                self.border_image_repeat.to_css(dest)?;
+            }
+            Ok(())
         }
     }
 </%helpers:shorthand>
@@ -454,7 +466,7 @@ pub fn parse_border<'i, 't>(
 
         impl<'a> ToCss for LonghandsToSerialize<'a>  {
             fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
-                super::serialize_directional_border(
+                crate::values::specified::border::serialize_directional_border(
                     dest,
                     self.border_${axis}_start_width,
                     self.border_${axis}_start_style,

components/style/values/specified/border.rs

@@ -16,10 +16,11 @@ use crate::values::generics::rect::Rect;
 use crate::values::generics::size::Size2D;
 use crate::values::specified::length::{NonNegativeLength, NonNegativeLengthPercentage};
 use crate::values::specified::{AllowQuirks, NonNegativeNumber, NonNegativeNumberOrPercentage};
+use crate::values::specified::Color;
 use crate::Zero;
 use cssparser::Parser;
 use std::fmt::{self, Write};
-use style_traits::{CssWriter, ParseError, ToCss};
+use style_traits::{CssWriter, ParseError, ToCss, values::SequenceWriter};
 
 /// A specified value for a single side of a `border-style` property.
 ///
@@ -316,3 +317,32 @@ impl Parse for BorderImageRepeat {
         ))
     }
 }
+
+/// Serializes a border shorthand value composed of width/style/color.
+pub fn serialize_directional_border<W>(
+    dest: &mut CssWriter<W>,
+    width: &BorderSideWidth,
+    style: &BorderStyle,
+    color: &Color,
+) -> fmt::Result
+where
+    W: Write,
+{
+    let has_style = *style != BorderStyle::None;
+    let has_color = *color != Color::CurrentColor;
+    let has_width = *width != BorderSideWidth::Medium;
+    if !has_style && !has_color && !has_width {
+        return width.to_css(dest)
+    }
+    let mut writer = SequenceWriter::new(dest, " ");
+    if has_width {
+        writer.item(width)?;
+    }
+    if has_style {
+        writer.item(style)?;
+    }
+    if has_color {
+        writer.item(color)?;
+    }
+    Ok(())
+}