Bug 1597884 - [css-grid] Make some grid/grid-template values serialize to a shorter form. r=emilio

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

--HG--
extra : moz-landing-system : lando

layout/style/test/test_grid_shorthand_serialization.html

@@ -119,7 +119,7 @@ var grid_template_test_cases = [
 grid_test_cases = grid_template_test_cases.concat([
     {
         gridAutoFlow: "row",
-        shorthand: "none / none",
+        shorthand: "none",
     },
     {
         gridAutoRows: "40px",
@@ -160,18 +160,14 @@ grid_test_cases = grid_template_test_cases.concat([
         gridTemplateColumns: "100px",
         shorthand: "auto-flow 40px / 100px",
     },
-    {
-        gridAutoFlow: "row",
-        shorthand: "none / none",
-    },
     // Test that grid-gap properties don't affect serialization.
     {
         gridRowGap: "1px",
-        shorthand: "none / none",
+        shorthand: "none",
     },
     {
         gridColumnGap: "1px",
-        shorthand: "none / none",
+        shorthand: "none",
     },
 ]);
 

servo/components/style/properties/shorthands/position.mako.rs

@@ -360,7 +360,7 @@
 
                 value
             } else {
-                GenericGridTemplateComponent::None
+                GridTemplateComponent::default()
             };
 
             Ok((
@@ -409,6 +409,9 @@
         W: Write {
         match *template_areas {
             GridTemplateAreas::None => {
+                if template_rows.is_initial() && template_columns.is_initial() {
+                    return GridTemplateComponent::default().to_css(dest);
+                }
                 template_rows.to_css(dest)?;
                 dest.write_str(" / ")?;
                 template_columns.to_css(dest)
@@ -465,8 +468,12 @@
                     }
 
                     string.to_css(dest)?;
-                    dest.write_str(" ")?;
-                    value.to_css(dest)?;
+
+                    // If the track size is the initial value then it's redundant here.
+                    if !value.is_initial() {
+                        dest.write_str(" ")?;
+                        value.to_css(dest)?;
+                    }
                 }
 
                 if let Some(names) = names_iter.next() {
@@ -513,8 +520,8 @@
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Longhands, ParseError<'i>> {
-        let mut temp_rows = GridTemplateComponent::None;
-        let mut temp_cols = GridTemplateComponent::None;
+        let mut temp_rows = GridTemplateComponent::default();
+        let mut temp_cols = GridTemplateComponent::default();
         let mut temp_areas = GridTemplateAreas::None;
         let mut auto_rows = ImplicitGridTracks::default();
         let mut auto_cols = ImplicitGridTracks::default();
@@ -587,8 +594,8 @@
     impl<'a> ToCss for LonghandsToSerialize<'a> {
         fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
             if *self.grid_template_areas != GridTemplateAreas::None ||
-               (*self.grid_template_rows != GridTemplateComponent::None &&
-                   *self.grid_template_columns != GridTemplateComponent::None) ||
+               (!self.grid_template_rows.is_initial() &&
+                !self.grid_template_columns.is_initial()) ||
                self.is_grid_template() {
                 return super::grid_template::serialize_grid_template(self.grid_template_rows,
                                                                      self.grid_template_columns,
@@ -598,7 +605,7 @@
             if self.grid_auto_flow.autoflow == AutoFlow::Column {
                 // It should fail to serialize if other branch of the if condition's values are set.
                 if !self.grid_auto_rows.is_initial() ||
-                   *self.grid_template_columns != GridTemplateComponent::None {
+                    !self.grid_template_columns.is_initial() {
                     return Ok(());
                 }
 
@@ -622,7 +629,7 @@
             } else {
                 // It should fail to serialize if other branch of the if condition's values are set.
                 if !self.grid_auto_columns.is_initial() ||
-                   *self.grid_template_rows != GridTemplateComponent::None {
+                    !self.grid_template_rows.is_initial() {
                     return Ok(());
                 }
 

servo/components/style/values/generics/grid.rs

@@ -261,6 +261,19 @@ pub enum GenericTrackSize<L> {
 pub use self::GenericTrackSize as TrackSize;
 
 impl<L> TrackSize<L> {
+    /// The initial value.
+    const INITIAL_VALUE: Self = TrackSize::Breadth(TrackBreadth::Auto);
+
+    /// Returns the initial value.
+    pub const fn initial_value() -> Self {
+        Self::INITIAL_VALUE
+    }
+
+    /// Returns true if `self` is the initial value.
+    pub fn is_initial(&self) -> bool {
+        matches!(*self, TrackSize::Breadth(TrackBreadth::Auto)) // FIXME: can't use Self::INITIAL_VALUE here yet: https://github.com/rust-lang/rust/issues/66585
+    }
+
     /// Check whether this is a `<fixed-size>`
     ///
     /// <https://drafts.csswg.org/css-grid/#typedef-fixed-size>
@@ -286,17 +299,9 @@ impl<L> TrackSize<L> {
     }
 }
 
-impl<L: PartialEq> TrackSize<L> {
-    /// Return true if it is `auto`.
-    #[inline]
-    pub fn is_auto(&self) -> bool {
-        *self == TrackSize::Breadth(TrackBreadth::Auto)
-    }
-}
-
 impl<L> Default for TrackSize<L> {
     fn default() -> Self {
-        TrackSize::Breadth(TrackBreadth::Auto)
+        Self::initial_value()
     }
 }
 
@@ -513,9 +518,24 @@ pub enum GenericTrackListValue<LengthPercentage, Integer> {
 pub use self::GenericTrackListValue as TrackListValue;
 
 impl<L, I> TrackListValue<L, I> {
+    // FIXME: can't use TrackSize::initial_value() here b/c rustc error "is not yet stable as a const fn"
+    const INITIAL_VALUE: Self = TrackListValue::TrackSize(TrackSize::Breadth(TrackBreadth::Auto));
+
     fn is_repeat(&self) -> bool {
         matches!(*self, TrackListValue::TrackRepeat(..))
     }
+
+    /// Returns true if `self` is the initial value.
+    pub fn is_initial(&self) -> bool {
+        matches!(*self, TrackListValue::TrackSize(TrackSize::Breadth(TrackBreadth::Auto))) // FIXME: can't use Self::INITIAL_VALUE here yet: https://github.com/rust-lang/rust/issues/66585
+    }
+}
+
+impl<L, I> Default for TrackListValue<L, I> {
+    #[inline]
+    fn default() -> Self {
+        Self::INITIAL_VALUE
+    }
 }
 
 /// A grid `<track-list>` type.
@@ -755,6 +775,9 @@ pub enum GenericGridTemplateComponent<L, I> {
 pub use self::GenericGridTemplateComponent as GridTemplateComponent;
 
 impl<L, I> GridTemplateComponent<L, I> {
+    /// The initial value.
+    const INITIAL_VALUE: Self = Self::None;
+
     /// Returns length of the <track-list>s <track-size>
     pub fn track_list_len(&self) -> usize {
         match *self {
@@ -762,4 +785,16 @@ impl<L, I> GridTemplateComponent<L, I> {
             _ => 0,
         }
     }
+
+    /// Returns true if `self` is the initial value.
+    pub fn is_initial(&self) -> bool {
+        matches!(*self, Self::None) // FIXME: can't use Self::INITIAL_VALUE here yet: https://github.com/rust-lang/rust/issues/66585
+    }
+}
+
+impl<L, I> Default for GridTemplateComponent<L, I> {
+    #[inline]
+    fn default() -> Self {
+        Self::INITIAL_VALUE
+    }
 }

servo/components/style/values/specified/grid.rs

@@ -102,8 +102,8 @@ impl Parse for ImplicitGridTracks<TrackSize<LengthPercentage>> {
     ) -> Result<Self, ParseError<'i>> {
         use style_traits::{Separator, Space};
         let track_sizes = Space::parse(input, |i| TrackSize::parse(context, i))?;
-        if track_sizes.len() == 1 && track_sizes[0].is_auto() {
-            //`auto`, which is the initial value, is always represented by an empty slice.
+        if track_sizes.len() == 1 && track_sizes[0].is_initial() {
+            // A single track with the initial value is always represented by an empty slice.
             return Ok(Default::default());
         }
         return Ok(ImplicitGridTracks(track_sizes.into()));

testing/web-platform/tests/css/css-grid/parsing/grid-shorthand-valid.html

@@ -14,6 +14,8 @@
 <script>
 
 // <'grid-template'> values
+test_valid_value("grid", 'none');
+test_valid_value("grid", 'none / none', 'none');
 test_valid_value("grid", 'auto / auto');
 test_valid_value("grid", 'none / [a] 0px');
 test_valid_value("grid", 'none / [] 0px', 'none / 0px');
@@ -25,6 +27,30 @@ test_valid_value("grid", '[a] "a" 10px []', '[a] "a" 10px');
 test_valid_value("grid", '"a" 10px');
 test_valid_value("grid", '[] "a" 10px', '"a" 10px');
 test_valid_value("grid", '[a] "a" 10px [a]');
+test_valid_value("grid", '"a"');
+test_valid_value("grid", '"a" auto', '"a"');
+test_valid_value("grid", '"a" / 10px');
+test_valid_value("grid", '"a" / 20%');
+test_valid_value("grid", '"a" / 5fr');
+test_valid_value("grid", '[a] "a"');
+test_valid_value("grid", '[a] "a" [a]');
+test_valid_value("grid", '[] "a"', '"a"');
+test_valid_value("grid", '"a" [] [] "b"', '"a" "b"');
+test_valid_value("grid", '"a" [] "b"', '"a" "b"');
+test_valid_value("grid", '"a" [a] [b] "b"', '"a" [a b] "b"');
+test_valid_value("grid", '"a" [a] "b"');
+test_valid_value("grid", '"a" / 0', '"a" / 0px');
+test_valid_value("grid", '"a" 10px / 10px');
+test_valid_value("grid", '"a" [a] "b" 10px');
+test_valid_value("grid", '"a" [a] "b" 10px [a]');
+test_valid_value("grid", '"a" [a] [a] "b" 10px', '"a" [a a] "b" 10px');
+test_valid_value("grid", '"a" [a] [] "b" 10px', '"a" [a] "b" 10px');
+test_valid_value("grid", '"a" 10px [a] "b" [a]');
+test_valid_value("grid", '"a" [a] "b" [a]');
+test_valid_value("grid", '[a] "a" [a] "b" [a]');
+test_valid_value("grid", '"a" "a" [a] "b" [a]');
+test_valid_value("grid", '"a" [a] "b" [a] / 0', '"a" [a] "b" [a] / 0px');
+test_valid_value("grid", '"a" "a" [a] [a] "b" / auto', '"a" "a" [a a] "b" / auto');
 
 // FIXME: add more values to test full syntax
 

testing/web-platform/tests/css/css-grid/parsing/grid-template-shorthand-valid.html

@@ -13,6 +13,8 @@
 <body>
 <script>
 
+test_valid_value("grid-template", 'none');
+test_valid_value("grid-template", 'none / none', 'none');
 test_valid_value("grid-template", 'auto / auto');
 test_valid_value("grid-template", 'none / [a] 0px');
 test_valid_value("grid-template", 'none / [] 0px', 'none / 0px');
@@ -24,6 +26,30 @@ test_valid_value("grid-template", '[a] "a" 10px []', '[a] "a" 10px');
 test_valid_value("grid-template", '"a" 10px');
 test_valid_value("grid-template", '[] "a" 10px', '"a" 10px');
 test_valid_value("grid-template", '[a] "a" 10px [a]');
+test_valid_value("grid-template", '"a"');
+test_valid_value("grid-template", '"a" auto', '"a"');
+test_valid_value("grid-template", '"a" / 10px');
+test_valid_value("grid-template", '"a" / 20%');
+test_valid_value("grid-template", '"a" / 5fr');
+test_valid_value("grid-template", '[a] "a"');
+test_valid_value("grid-template", '[a] "a" [a]');
+test_valid_value("grid-template", '[] "a"', '"a"');
+test_valid_value("grid-template", '"a" [] [] "b"', '"a" "b"');
+test_valid_value("grid-template", '"a" [] "b"', '"a" "b"');
+test_valid_value("grid-template", '"a" [a] [b] "b"', '"a" [a b] "b"');
+test_valid_value("grid-template", '"a" [a] "b"');
+test_valid_value("grid-template", '"a" / 0', '"a" / 0px');
+test_valid_value("grid-template", '"a" 10px / 10px');
+test_valid_value("grid-template", '"a" [a] "b" 10px');
+test_valid_value("grid-template", '"a" [a] "b" 10px [a]');
+test_valid_value("grid-template", '"a" [a] [a] "b" 10px', '"a" [a a] "b" 10px');
+test_valid_value("grid-template", '"a" [a] [] "b" 10px', '"a" [a] "b" 10px');
+test_valid_value("grid-template", '"a" 10px [a] "b" [a]');
+test_valid_value("grid-template", '"a" [a] "b" [a]');
+test_valid_value("grid-template", '[a] "a" [a] "b" [a]');
+test_valid_value("grid-template", '"a" "a" [a] "b" [a]');
+test_valid_value("grid-template", '"a" [a] "b" [a] / 0', '"a" [a] "b" [a] / 0px');
+test_valid_value("grid-template", '"a" "a" [a] [a] "b" / auto', '"a" "a" [a a] "b" / auto');
 
 // FIXME: add more values to test full syntax