Auto merge of #15662 - canaltinova:column, r=Manishearth

Fix parsing methods of column-{gap,width}

<!-- Please describe your changes on the following line: -->
They weren't accepting {normal, auto} keywords. Fixed parsing methods of these properties.

---
<!-- 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 #15088 (github issue number if applicable).

<!-- 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. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15662)
<!-- Reviewable:end -->

components/style/gecko/values.rs

@@ -11,7 +11,7 @@ use cssparser::RGBA;
 use gecko_bindings::structs::{nsStyleCoord, StyleGridTrackBreadth, StyleShapeRadius};
 use gecko_bindings::sugar::ns_style_coord::{CoordData, CoordDataMut, CoordDataValue};
 use std::cmp::max;
-use values::{Auto, Either, None_};
+use values::{Auto, Either, None_, Normal};
 use values::computed::{Angle, LengthOrPercentageOrNone, Number};
 use values::computed::{LengthOrPercentage, LengthOrPercentageOrAuto};
 use values::computed::basic_shape::ShapeRadius;
@@ -257,6 +257,20 @@ impl GeckoStyleCoordConvertible for None_ {
     }
 }
 
+impl GeckoStyleCoordConvertible for Normal {
+    fn to_gecko_style_coord<T: CoordDataMut>(&self, coord: &mut T) {
+        coord.set_value(CoordDataValue::Normal)
+    }
+
+    fn from_gecko_style_coord<T: CoordData>(coord: &T) -> Option<Self> {
+        if let CoordDataValue::Normal = coord.as_value() {
+            Some(Normal)
+        } else {
+            None
+        }
+    }
+}
+
 /// Convert a given RGBA value to `nscolor`.
 pub fn convert_rgba_to_nscolor(rgba: &RGBA) -> u32 {
     ((rgba.alpha as u32) << 24) |

components/style/properties/gecko.mako.rs

@@ -628,6 +628,7 @@ impl Debug for ${style_struct.gecko_struct_name} {
     # Types used with predefined_type()-defined properties that we can auto-generate.
     predefined_types = {
         "length::LengthOrAuto": impl_style_coord,
+        "length::LengthOrNormal": impl_style_coord,
         "Length": impl_absolute_length,
         "Position": impl_position,
         "LengthOrPercentage": impl_style_coord,
@@ -3041,7 +3042,7 @@ clip-path
 </%self:impl_trait>
 
 <%self:impl_trait style_struct_name="Column"
-                  skip_longhands="column-count column-gap column-rule-width">
+                  skip_longhands="column-count column-rule-width">
 
     #[allow(unused_unsafe)]
     pub fn set_column_count(&mut self, v: longhands::column_count::computed_value::T) {
@@ -3057,17 +3058,6 @@ clip-path
 
     ${impl_simple_copy('column_count', 'mColumnCount')}
 
-    pub fn set_column_gap(&mut self, v: longhands::column_gap::computed_value::T) {
-        use values::Either;
-
-        match v {
-            Either::First(len) => self.gecko.mColumnGap.set(len),
-            Either::Second(_normal) => self.gecko.mColumnGap.set_value(CoordDataValue::Normal),
-        }
-    }
-
-    <%call expr="impl_coord_copy('column_gap', 'mColumnGap')"></%call>
-
     <% impl_app_units("column_rule_width", "mColumnRuleWidth", need_clone=True,
                       round_to_pixels=True) %>
 </%self:impl_trait>

components/style/values/specified/length.rs

@@ -496,10 +496,24 @@ impl Parse for Length {
     }
 }
 
-impl<T> Either<Length, T> {
+impl Either<Length, Normal> {
     #[inline]
     #[allow(missing_docs)]
-    pub fn parse_non_negative_length(_context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
+    pub fn parse_non_negative_length(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
+        if input.try(|input| Normal::parse(context, input)).is_ok() {
+            return Ok(Either::Second(Normal));
+        }
+        Length::parse_internal(input, AllowedNumericType::NonNegative).map(Either::First)
+    }
+}
+
+impl Either<Length, Auto> {
+    #[inline]
+    #[allow(missing_docs)]
+    pub fn parse_non_negative_length(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
+        if input.try(|input| Auto::parse(context, input)).is_ok() {
+            return Ok(Either::Second(Auto));
+        }
         Length::parse_internal(input, AllowedNumericType::NonNegative).map(Either::First)
     }
 }

tests/unit/style/parsing/column.rs

@@ -0,0 +1,42 @@
+/* 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/. */
+
+use cssparser::Parser;
+use media_queries::CSSErrorReporterTest;
+use servo_url::ServoUrl;
+use style::parser::ParserContext;
+use style::stylesheets::Origin;
+use style_traits::ToCss;
+
+#[test]
+fn test_column_width() {
+    use style::properties::longhands::column_width;
+
+    assert_roundtrip_with_context!(column_width::parse, "auto");
+    assert_roundtrip_with_context!(column_width::parse, "6px");
+    assert_roundtrip_with_context!(column_width::parse, "2.5em");
+    assert_roundtrip_with_context!(column_width::parse, "0.3vw");
+
+    let url = ServoUrl::parse("http://localhost").unwrap();
+    let context = ParserContext::new(Origin::Author, &url, Box::new(CSSErrorReporterTest));
+
+    let mut negative = Parser::new("-6px");
+    assert!(column_width::parse(&context, &mut negative).is_err());
+}
+
+#[test]
+fn test_column_gap() {
+    use style::properties::longhands::column_gap;
+
+    assert_roundtrip_with_context!(column_gap::parse, "normal");
+    assert_roundtrip_with_context!(column_gap::parse, "6px");
+    assert_roundtrip_with_context!(column_gap::parse, "2.5em");
+    assert_roundtrip_with_context!(column_gap::parse, "0.3vw");
+
+    let url = ServoUrl::parse("http://localhost").unwrap();
+    let context = ParserContext::new(Origin::Author, &url, Box::new(CSSErrorReporterTest));
+
+    let mut negative = Parser::new("-6px");
+    assert!(column_gap::parse(&context, &mut negative).is_err());
+}

tests/unit/style/parsing/mod.rs

@@ -70,6 +70,7 @@ mod animation;
 mod background;
 mod basic_shape;
 mod border;
+mod column;
 mod font;
 mod image;
 mod inherited_box;