style: Don't incorrectly clamp values in calc that might not be only lengths.

Expressions with percentages may be negative or positive at computed value time.

So, we can only clamp lengths at computed value time, which is what the other
browsers do.

components/servo/Cargo.lock

@@ -2268,6 +2268,7 @@ dependencies = [
 name = "style_traits"
 version = "0.0.1"
 dependencies = [
+ "app_units 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "cssparser 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "euclid 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "heapsize 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)",

components/style/media_queries.rs

@@ -40,11 +40,12 @@ impl Range<specified::Length> {
                     => value.to_computed_value(initial_font_size, initial_font_size),
                 specified::Length::ViewportPercentage(value)
                     => value.to_computed_value(viewport_size),
-                specified::Length::Calc(val)
+                specified::Length::Calc(val, range)
-                    => val.compute_from_viewport_and_font_size(viewport_size,
+                    => range.clamp(
-                                                               initial_font_size,
+                        val.compute_from_viewport_and_font_size(viewport_size,
-                                                               initial_font_size)
+                                                                initial_font_size,
-                          .length(),
+                                                                initial_font_size)
+                           .length()),
                 specified::Length::ServoCharacterWidth(..)
                     => unreachable!(),
             }

components/style/values/computed/mod.rs

@@ -71,7 +71,7 @@ impl ToComputedValue for specified::Length {
     fn to_computed_value(&self, context: &Context) -> Au {
         match *self {
             specified::Length::Absolute(length) => length,
-            specified::Length::Calc(calc) => calc.to_computed_value(context).length(),
+            specified::Length::Calc(calc, range) => range.clamp(calc.to_computed_value(context).length()),
             specified::Length::FontRelative(length) =>
                 length.to_computed_value(context.style().get_font().clone_font_size(),
                                          context.style().root_font_size()),
@@ -480,8 +480,8 @@ impl ToComputedValue for specified::LengthOrNone {
     #[inline]
     fn to_computed_value(&self, context: &Context) -> LengthOrNone {
         match *self {
-            specified::LengthOrNone::Length(specified::Length::Calc(calc)) => {
+            specified::LengthOrNone::Length(specified::Length::Calc(calc, range)) => {
-                LengthOrNone::Length(calc.to_computed_value(context).length())
+                LengthOrNone::Length(range.clamp(calc.to_computed_value(context).length()))
             }
             specified::LengthOrNone::Length(value) => {
                 LengthOrNone::Length(value.to_computed_value(context))

components/style/values/specified/mod.rs

@@ -190,14 +190,14 @@ pub enum Length {
     /// `Stylist::synthesize_rules_for_legacy_attributes()`.
     ServoCharacterWidth(CharacterWidth),
 
-    Calc(CalcLengthOrPercentage),
+    Calc(CalcLengthOrPercentage, AllowedNumericType),
 }
 
 impl HasViewportPercentage for Length {
     fn has_viewport_percentage(&self) -> bool {
         match *self {
             Length::ViewportPercentage(_) => true,
-            Length::Calc(ref calc) => calc.has_viewport_percentage(),
+            Length::Calc(ref calc, _) => calc.has_viewport_percentage(),
             _ => false
         }
     }
@@ -209,7 +209,7 @@ impl ToCss for Length {
             Length::Absolute(length) => write!(dest, "{}px", length.to_f32_px()),
             Length::FontRelative(length) => length.to_css(dest),
             Length::ViewportPercentage(length) => length.to_css(dest),
-            Length::Calc(ref calc) => calc.to_css(dest),
+            Length::Calc(ref calc, _) => calc.to_css(dest),
             Length::ServoCharacterWidth(_)
             => panic!("internal CSS values should never be serialized"),
         }
@@ -225,7 +225,7 @@ impl Mul<CSSFloat> for Length {
             Length::Absolute(Au(v)) => Length::Absolute(Au(((v as f32) * scalar) as i32)),
             Length::FontRelative(v) => Length::FontRelative(v * scalar),
             Length::ViewportPercentage(v) => Length::ViewportPercentage(v * scalar),
-            Length::Calc(_) => panic!("Can't multiply Calc!"),
+            Length::Calc(..) => panic!("Can't multiply Calc!"),
             Length::ServoCharacterWidth(_) => panic!("Can't multiply ServoCharacterWidth!"),
         }
     }
@@ -469,8 +469,6 @@ pub struct CalcLengthOrPercentage {
     pub ch: Option<FontRelativeLength>,
     pub rem: Option<FontRelativeLength>,
     pub percentage: Option<Percentage>,
-    /// Whether the value returned can be negative at computed value time.
-    pub allowed_numeric_type: AllowedNumericType,
 }
 
 impl CalcLengthOrPercentage {
@@ -623,17 +621,17 @@ impl CalcLengthOrPercentage {
 
     fn parse_length(input: &mut Parser,
                     context: AllowedNumericType) -> Result<Length, ()> {
-        CalcLengthOrPercentage::parse(input, CalcUnit::Length, context).map(Length::Calc)
+        CalcLengthOrPercentage::parse(input, CalcUnit::Length).map(|calc| {
+            Length::Calc(calc, context)
+        })
     }
 
-    fn parse_length_or_percentage(input: &mut Parser,
+    fn parse_length_or_percentage(input: &mut Parser) -> Result<CalcLengthOrPercentage, ()> {
-                                  context: AllowedNumericType) -> Result<CalcLengthOrPercentage, ()> {
+        CalcLengthOrPercentage::parse(input, CalcUnit::LengthOrPercentage)
-        CalcLengthOrPercentage::parse(input, CalcUnit::LengthOrPercentage, context)
     }
 
     fn parse(input: &mut Parser,
-             expected_unit: CalcUnit,
+             expected_unit: CalcUnit) -> Result<CalcLengthOrPercentage, ()> {
-             context: AllowedNumericType) -> Result<CalcLengthOrPercentage, ()> {
         let ast = try!(CalcLengthOrPercentage::parse_sum(input, expected_unit));
 
         let mut simplified = Vec::new();
@@ -700,7 +698,6 @@ impl CalcLengthOrPercentage {
             ch: ch.map(FontRelativeLength::Ch),
             rem: rem.map(FontRelativeLength::Rem),
             percentage: percentage.map(Percentage),
-            allowed_numeric_type: context,
         })
     }
 
@@ -787,23 +784,9 @@ impl CalcLengthOrPercentage {
             }
         }
 
-        // https://drafts.csswg.org/css-values/#calc-range
-        let mut percentage = self.percentage.map(|p| p.0);
-        if let AllowedNumericType::NonNegative = self.allowed_numeric_type {
-            if let Some(ref mut length) = length {
-                *length = cmp::max(*length, Au(0));
-            }
-
-            if let Some(ref mut percentage) = percentage {
-                if *percentage < 0. {
-                    *percentage = 0.;
-                }
-            }
-        }
-
         computed::CalcLengthOrPercentage {
             length: length,
-            percentage: percentage,
+            percentage: self.percentage.map(|p| p.0),
         }
     }
 }
@@ -919,9 +902,7 @@ impl LengthOrPercentage {
             Token::Number(ref value) if value.value == 0. =>
                 Ok(LengthOrPercentage::Length(Length::Absolute(Au(0)))),
             Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
-                let calc = try!(input.parse_nested_block(|input| {
+                let calc = try!(input.parse_nested_block(CalcLengthOrPercentage::parse_length_or_percentage));
-                    CalcLengthOrPercentage::parse_length_or_percentage(input, context)
-                }));
                 Ok(LengthOrPercentage::Calc(calc))
             },
             _ => Err(())
@@ -981,9 +962,7 @@ impl LengthOrPercentageOrAuto {
             Token::Ident(ref value) if value.eq_ignore_ascii_case("auto") =>
                 Ok(LengthOrPercentageOrAuto::Auto),
             Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
-                let calc = try!(input.parse_nested_block(|input| {
+                let calc = try!(input.parse_nested_block(CalcLengthOrPercentage::parse_length_or_percentage));
-                    CalcLengthOrPercentage::parse_length_or_percentage(input, context)
-                }));
                 Ok(LengthOrPercentageOrAuto::Calc(calc))
             },
             _ => Err(())
@@ -1040,9 +1019,7 @@ impl LengthOrPercentageOrNone {
             Token::Number(ref value) if value.value == 0. =>
                 Ok(LengthOrPercentageOrNone::Length(Length::Absolute(Au(0)))),
             Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
-                let calc = try!(input.parse_nested_block(|input| {
+                let calc = try!(input.parse_nested_block(CalcLengthOrPercentage::parse_length_or_percentage));
-                    CalcLengthOrPercentage::parse_length_or_percentage(input, context)
-                }));
                 Ok(LengthOrPercentageOrNone::Calc(calc))
             },
             Token::Ident(ref value) if value.eq_ignore_ascii_case("none") =>
@@ -1159,9 +1136,7 @@ impl LengthOrPercentageOrAutoOrContent {
             Token::Ident(ref value) if value.eq_ignore_ascii_case("content") =>
                 Ok(LengthOrPercentageOrAutoOrContent::Content),
             Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
-                let calc = try!(input.parse_nested_block(|input| {
+                let calc = try!(input.parse_nested_block(CalcLengthOrPercentage::parse_length_or_percentage));
-                    CalcLengthOrPercentage::parse_length_or_percentage(input, context)
-                }));
                 Ok(LengthOrPercentageOrAutoOrContent::Calc(calc))
             },
             _ => Err(())

components/style_traits/Cargo.toml

@@ -14,6 +14,7 @@ servo = ["heapsize", "heapsize_plugin", "serde", "serde_macros",
          "cssparser/heap_size", "cssparser/serde-serialization"]
 
 [dependencies]
+app_units = "0.3"
 cssparser = "0.6"
 euclid = "0.10.1"
 heapsize = {version = "0.3.0", optional = true}

components/style_traits/lib.rs

@@ -16,6 +16,7 @@
 #![cfg_attr(feature = "servo", plugin(serde_macros))]
 #![cfg_attr(feature = "servo", plugin(heapsize_plugin))]
 
+extern crate app_units;
 #[macro_use]
 extern crate cssparser;
 extern crate euclid;

components/style_traits/values.rs

@@ -63,6 +63,8 @@ macro_rules! __define_css_keyword_enum__actual {
 
 
 pub mod specified {
+    use app_units::Au;
+
     #[repr(u8)]
     #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
     #[derive(Clone, Copy, Debug, PartialEq, Eq)]
@@ -79,5 +81,14 @@ pub mod specified {
                 AllowedNumericType::NonNegative => value >= 0.,
             }
         }
+
+        #[inline]
+        pub fn clamp(&self, val: Au) -> Au {
+            use std::cmp;
+            match *self {
+                AllowedNumericType::All => val,
+                AllowedNumericType::NonNegative => cmp::max(Au(0), val),
+            }
+        }
     }
 }

ports/cef/Cargo.lock

@@ -2135,6 +2135,7 @@ dependencies = [
 name = "style_traits"
 version = "0.0.1"
 dependencies = [
+ "app_units 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "cssparser 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "euclid 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "heapsize 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)",

ports/geckolib/Cargo.lock

@@ -387,6 +387,7 @@ dependencies = [
 name = "style_traits"
 version = "0.0.1"
 dependencies = [
+ "app_units 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "cssparser 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "euclid 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)",

tests/wpt/mozilla/meta/MANIFEST.json

@@ -3648,6 +3648,18 @@
             "url": "/_mozilla/css/multiple_css_class_a.html"
           }
         ],
+        "css/negative-calc-cv.html": [
+          {
+            "path": "css/negative-calc-cv.html",
+            "references": [
+              [
+                "/_mozilla/css/negative-calc-cv-ref.html",
+                "=="
+              ]
+            ],
+            "url": "/_mozilla/css/negative-calc-cv.html"
+          }
+        ],
         "css/negative_margin_uncle_a.html": [
           {
             "path": "css/negative_margin_uncle_a.html",
@@ -13030,6 +13042,18 @@
           "url": "/_mozilla/css/multiple_css_class_a.html"
         }
       ],
+      "css/negative-calc-cv.html": [
+        {
+          "path": "css/negative-calc-cv.html",
+          "references": [
+            [
+              "/_mozilla/css/negative-calc-cv-ref.html",
+              "=="
+            ]
+          ],
+          "url": "/_mozilla/css/negative-calc-cv.html"
+        }
+      ],
       "css/negative_margin_uncle_a.html": [
         {
           "path": "css/negative_margin_uncle_a.html",

tests/wpt/mozilla/tests/css/negative-calc-cv-ref.html

@@ -0,0 +1,16 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Test Reference</title>
+<style>
+  .container {
+    width: 500px;
+  }
+  .kid {
+    height: 50px;
+    background: green;
+    width: 40px;
+  }
+</style>
+<div class="container">
+  <div class="kid"></div>
+</div>

tests/wpt/mozilla/tests/css/negative-calc-cv.html

@@ -0,0 +1,17 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>A negative length in a calc() expression isn't incorrectly clamped.</title>
+<link rel="match" href="negative-calc-cv-ref.html">
+<style>
+  .container {
+    width: 500px;
+  }
+  .kid {
+    height: 50px;
+    background: green;
+    width: calc(-10px + 10%);
+  }
+</style>
+<div class="container">
+  <div class="kid"></div>
+</div>