Auto merge of #15278 - atheed:zero-parsing, r=Wafflespeanut

Parsing "0" as Number for line-height and border-image-outset

Fixes #15171 by correctly parsing `0` as `0` (rather than as `0px`, as was the case earlier) for the `line-height` and `border-image-outset` CSS properties. Wrote unit tests for both; `./mach test-unit -p style` passes all tests.

---
<!-- 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 #15171

<!-- 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/15278)
<!-- Reviewable:end -->

components/style/properties/longhand/inherited_text.mako.rs

@@ -48,23 +48,27 @@
     pub fn parse(_context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
         use cssparser::Token;
         use std::ascii::AsciiExt;
-        input.try(specified::LengthOrPercentage::parse_non_negative)
-        .map(SpecifiedValue::LengthOrPercentage)
+        // We try to parse as a Number first because, for 'line-height', we want "0" to be
+        // parsed as a plain Number rather than a Length (0px); this matches the behaviour
+        // of all major browsers
+        input.try(specified::Number::parse_non_negative)
+        .map(|n| SpecifiedValue::Number(n.0))
         .or_else(|()| {
-            match try!(input.next()) {
-                Token::Number(ref value) if value.value >= 0. => {
-                    Ok(SpecifiedValue::Number(value.value))
+            input.try(specified::LengthOrPercentage::parse_non_negative)
+            .map(SpecifiedValue::LengthOrPercentage)
+            .or_else(|()| {
+                match try!(input.next()) {
+                    Token::Ident(ref value) if value.eq_ignore_ascii_case("normal") => {
+                        Ok(SpecifiedValue::Normal)
+                    }
+                    % if product == "gecko":
+                    Token::Ident(ref value) if value.eq_ignore_ascii_case("-moz-block-height") => {
+                        Ok(SpecifiedValue::MozBlockHeight)
+                    }
+                    % endif
+                    _ => Err(()),
                 }
-                Token::Ident(ref value) if value.eq_ignore_ascii_case("normal") => {
-                    Ok(SpecifiedValue::Normal)
-                }
-                % if product == "gecko":
-                Token::Ident(ref value) if value.eq_ignore_ascii_case("-moz-block-height") => {
-                    Ok(SpecifiedValue::MozBlockHeight)
-                }
-                % endif
-                _ => Err(()),
-            }
+            })
         })
     }
     pub mod computed_value {

components/style/values/specified/length.rs

@@ -1196,10 +1196,13 @@ pub type LengthOrNumber = Either<Length, Number>;
 impl LengthOrNumber {
     /// Parse a non-negative LengthOrNumber.
     pub fn parse_non_negative(input: &mut Parser) -> Result<Self, ()> {
-        if let Ok(v) = input.try(Length::parse_non_negative) {
-            Ok(Either::First(v))
+        // We try to parse as a Number first because, for cases like LengthOrNumber,
+        // we want "0" to be parsed as a plain Number rather than a Length (0px); this
+        // matches the behaviour of all major browsers
+        if let Ok(v) = input.try(Number::parse_non_negative) {
+            Ok(Either::Second(v))
         } else {
-            Number::parse_non_negative(input).map(Either::Second)
+            Length::parse_non_negative(input).map(Either::First)
         }
     }
 }

tests/unit/style/parsing/border.rs

@@ -104,3 +104,21 @@ fn border_image_outset_should_error_on_negative_number() {
     let result = border_image_outset::parse(&context, &mut parser);
     assert_eq!(result, Err(()));
 }
+
+#[test]
+fn border_image_outset_should_return_number_on_plain_zero() {
+    let url = ServoUrl::parse("http://localhost").unwrap();
+    let context = ParserContext::new(Origin::Author, &url, Box::new(CSSErrorReporterTest));
+    let mut parser = Parser::new("0");
+    let result = border_image_outset::parse(&context, &mut parser);
+    assert_eq!(result.unwrap(), parse_longhand!(border_image_outset, "0"));
+}
+
+#[test]
+fn border_image_outset_should_return_length_on_length_zero() {
+    let url = ServoUrl::parse("http://localhost").unwrap();
+    let context = ParserContext::new(Origin::Author, &url, Box::new(CSSErrorReporterTest));
+    let mut parser = Parser::new("0em");
+    let result = border_image_outset::parse(&context, &mut parser);
+    assert_eq!(result.unwrap(), parse_longhand!(border_image_outset, "0em"));
+}

tests/unit/style/parsing/inherited_text.rs

@@ -102,3 +102,29 @@ fn webkit_text_stroke_shorthand_should_parse_properly() {
     assert_eq!(result._webkit_text_stroke_color.unwrap(), parse_longhand!(_webkit_text_stroke_color, "red"));
     assert_eq!(result._webkit_text_stroke_width.unwrap(), parse_longhand!(_webkit_text_stroke_width, "thin"));
 }
+
+#[test]
+fn line_height_should_return_number_on_plain_zero() {
+    use media_queries::CSSErrorReporterTest;
+    use servo_url::ServoUrl;
+    use style::properties::longhands::line_height;
+
+    let url = ServoUrl::parse("http://localhost").unwrap();
+    let context = ParserContext::new(Origin::Author, &url, Box::new(CSSErrorReporterTest));
+    let mut parser = Parser::new("0");
+    let result = line_height::parse(&context, &mut parser);
+    assert_eq!(result.unwrap(), parse_longhand!(line_height, "0"));
+}
+
+#[test]
+fn line_height_should_return_length_on_length_zero() {
+    use media_queries::CSSErrorReporterTest;
+    use servo_url::ServoUrl;
+    use style::properties::longhands::line_height;
+
+    let url = ServoUrl::parse("http://localhost").unwrap();
+    let context = ParserContext::new(Origin::Author, &url, Box::new(CSSErrorReporterTest));
+    let mut parser = Parser::new("0px");
+    let result = line_height::parse(&context, &mut parser);
+    assert_eq!(result.unwrap(), parse_longhand!(line_height, "0px"));
+}