fix impl of parse_length()

components/servo/Cargo.lock

@@ -1923,6 +1923,7 @@ dependencies = [
 name = "util_tests"
 version = "0.0.1"
 dependencies = [
+ "app_units 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "euclid 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "libc 0.1.10 (registry+https://github.com/rust-lang/crates.io-index)",
  "plugins 0.0.1",

components/util/str.rs

@@ -303,20 +303,39 @@ pub enum LengthOrPercentageOrAuto {
     Length(Au),
 }
 
-/// Parses a length per HTML5 § 2.4.4.4. If unparseable, `Auto` is returned.
+/// TODO: this function can be rewritten to return Result<LengthOrPercentage, _>
+/// Parses a dimension value per HTML5 § 2.4.4.4. If unparseable, `Auto` is
+/// returned.
+/// https://html.spec.whatwg.org/multipage/#rules-for-parsing-dimension-values
 pub fn parse_length(mut value: &str) -> LengthOrPercentageOrAuto {
+    // Steps 1 & 2 are not relevant
+
+    // Step 3
     value = value.trim_left_matches(WHITESPACE);
-    if value.is_empty() {
+
-        return LengthOrPercentageOrAuto::Auto
+    // Step 4
-    }
-    if value.starts_with("+") {
-        value = &value[1..]
-    }
-    value = value.trim_left_matches('0');
     if value.is_empty() {
         return LengthOrPercentageOrAuto::Auto
     }
 
+    // Step 5
+    if value.starts_with("+") {
+        value = &value[1..]
+    }
+
+    // Steps 6 & 7
+    match value.chars().nth(0) {
+        Some('0'...'9') => {},
+        _ => return LengthOrPercentageOrAuto::Auto,
+    }
+
+    // Steps 8 to 13
+    // We trim the string length to the minimum of:
+    // 1. the end of the string
+    // 2. the first occurence of a '%' (U+0025 PERCENT SIGN)
+    // 3. the second occurrence of a '.' (U+002E FULL STOP)
+    // 4. the occurrence of a character that is neither a digit nor '%' nor '.'
+    // Note: Step 10 is directly subsumed by FromStr::from_str
     let mut end_index = value.len();
     let (mut found_full_stop, mut found_percent) = (false, false);
     for (i, ch) in value.chars().enumerate() {

tests/unit/util/Cargo.toml

@@ -16,6 +16,7 @@ path = "../../../components/util"
 path = "../../../components/plugins"
 
 [dependencies]
+app_units = {version = "0.1", features = ["plugins"]}
 libc = "0.1"
 euclid = {version = "0.3", features = ["plugins"]}
 

tests/unit/util/lib.rs

@@ -7,6 +7,7 @@
 #![feature(alloc)]
 
 extern crate alloc;
+extern crate app_units;
 extern crate euclid;
 extern crate libc;
 extern crate util;

tests/unit/util/str.rs

@@ -2,9 +2,25 @@
  * 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 util::str::{search_index, split_html_space_chars, str_join};
+use app_units::Au;
+use util::str::LengthOrPercentageOrAuto;
+use util::str::{parse_length, search_index, split_html_space_chars, str_join};
 
 
+#[test]
+pub fn test_parse_length() {
+    fn check(input: &str, expected: LengthOrPercentageOrAuto) {
+        let parsed = parse_length(input);
+        assert_eq!(parsed, expected);
+    }
+
+    check("0", LengthOrPercentageOrAuto::Length(Au::from_px(0)));
+    check("0.000%", LengthOrPercentageOrAuto::Percentage(0.0));
+    check("+5.82%", LengthOrPercentageOrAuto::Percentage(0.0582));
+    check("invalid", LengthOrPercentageOrAuto::Auto);
+    check("12 followed by invalid", LengthOrPercentageOrAuto::Length(Au::from_px(12)));
+}
+
 #[test]
 pub fn split_html_space_chars_whitespace() {
     assert!(split_html_space_chars("").collect::<Vec<_>>().is_empty());