servo: Merge #15732 - Fix parsing of ClipRect (from canaltinova:clip-parse); r=Manishearth
authorNazım Can Altınova <canaltinova@gmail.com>
Fri, 24 Feb 2017 13:22:15 -0800
changeset 373876 7ff9ada73578824a53afb4c533f8663a4c8649c1
parent 373875 704db7ae2d859e854d61cc77ae2999e843964b17
child 373877 80a323cabf561a081da59bc2695973ec53c30336
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersManishearth
milestone54.0a1
servo: Merge #15732 - Fix parsing of ClipRect (from canaltinova:clip-parse); r=Manishearth <!-- Please describe your changes on the following line: --> Fix parsing of ClipRect for clip property. --- <!-- 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 #15728 (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. --> Source-Repo: https://github.com/servo/servo Source-Revision: bdd3608d453f30472f100ad661b1f3e855e258d2
servo/components/layout/display_list_builder.rs
servo/components/style/properties/gecko.mako.rs
servo/components/style/values/computed/mod.rs
servo/components/style/values/specified/mod.rs
servo/tests/unit/style/parsing/effects.rs
servo/tests/unit/style/parsing/mod.rs
--- a/servo/components/layout/display_list_builder.rs
+++ b/servo/components/layout/display_list_builder.rs
@@ -1254,18 +1254,20 @@ impl FragmentDisplayListBuilding for Fra
         // Account for `clip` per CSS 2.1 ยง 11.1.2.
         let style_clip_rect = match (self.style().get_box().position,
                                      self.style().get_effects().clip) {
             (position::T::absolute, Either::First(style_clip_rect)) => style_clip_rect,
             _ => return,
         };
 
         // FIXME(pcwalton, #2795): Get the real container size.
-        let clip_origin = Point2D::new(stacking_relative_border_box.origin.x + style_clip_rect.left,
-                                       stacking_relative_border_box.origin.y + style_clip_rect.top);
+        let clip_origin = Point2D::new(stacking_relative_border_box.origin.x +
+                                           style_clip_rect.left.unwrap_or(Au(0)),
+                                       stacking_relative_border_box.origin.y +
+                                           style_clip_rect.top.unwrap_or(Au(0)));
         let right = style_clip_rect.right.unwrap_or(stacking_relative_border_box.size.width);
         let bottom = style_clip_rect.bottom.unwrap_or(stacking_relative_border_box.size.height);
         let clip_size = Size2D::new(right - clip_origin.x, bottom - clip_origin.y);
         parent_clip.intersect_rect(&Rect::new(clip_origin, clip_size))
     }
 
     fn build_display_items_for_selection_if_necessary(&self,
                                                       state: &mut DisplayListBuildState,
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -2281,18 +2281,18 @@ fn static_assert() {
         match v {
             Either::Second(_auto) => {
                 self.gecko.mImageRegion.x = 0;
                 self.gecko.mImageRegion.y = 0;
                 self.gecko.mImageRegion.width = 0;
                 self.gecko.mImageRegion.height = 0;
             }
             Either::First(rect) => {
-                self.gecko.mImageRegion.x = rect.left.0;
-                self.gecko.mImageRegion.y = rect.top.0;
+                self.gecko.mImageRegion.x = rect.left.unwrap_or(Au(0)).0;
+                self.gecko.mImageRegion.y = rect.top.unwrap_or(Au(0)).0;
                 self.gecko.mImageRegion.height = rect.bottom.unwrap_or(Au(0)).0 - self.gecko.mImageRegion.y;
                 self.gecko.mImageRegion.width = rect.right.unwrap_or(Au(0)).0 - self.gecko.mImageRegion.x;
             }
         }
     }
 
     ${impl_simple_copy('_moz_image_region', 'mImageRegion')}
 
@@ -2351,25 +2351,38 @@ fn static_assert() {
             }
         }).collect();
         longhands::box_shadow::computed_value::T(buf)
     }
 
     pub fn set_clip(&mut self, v: longhands::clip::computed_value::T) {
         use gecko_bindings::structs::NS_STYLE_CLIP_AUTO;
         use gecko_bindings::structs::NS_STYLE_CLIP_RECT;
+        use gecko_bindings::structs::NS_STYLE_CLIP_LEFT_AUTO;
+        use gecko_bindings::structs::NS_STYLE_CLIP_TOP_AUTO;
         use gecko_bindings::structs::NS_STYLE_CLIP_RIGHT_AUTO;
         use gecko_bindings::structs::NS_STYLE_CLIP_BOTTOM_AUTO;
         use values::Either;
 
         match v {
             Either::First(rect) => {
                 self.gecko.mClipFlags = NS_STYLE_CLIP_RECT as u8;
-                self.gecko.mClip.x = rect.left.0;
-                self.gecko.mClip.y = rect.top.0;
+                if let Some(left) = rect.left {
+                    self.gecko.mClip.x = left.0;
+                } else {
+                    self.gecko.mClip.x = 0;
+                    self.gecko.mClipFlags |= NS_STYLE_CLIP_LEFT_AUTO as u8;
+                }
+
+                if let Some(top) = rect.top {
+                    self.gecko.mClip.y = top.0;
+                } else {
+                    self.gecko.mClip.y = 0;
+                    self.gecko.mClipFlags |= NS_STYLE_CLIP_TOP_AUTO as u8;
+                }
 
                 if let Some(bottom) = rect.bottom {
                     self.gecko.mClip.height = bottom.0 - self.gecko.mClip.y;
                 } else {
                     self.gecko.mClip.height = 1 << 30; // NS_MAXSIZE
                     self.gecko.mClipFlags |= NS_STYLE_CLIP_BOTTOM_AUTO as u8;
                 }
 
--- a/servo/components/style/values/computed/mod.rs
+++ b/servo/components/style/values/computed/mod.rs
@@ -289,42 +289,51 @@ impl ToCss for SVGPaint {
 /// <length> | <percentage> | <number>
 pub type LoPOrNumber = Either<LengthOrPercentage, Number>;
 
 #[derive(Clone, PartialEq, Eq, Copy, Debug)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 #[allow(missing_docs)]
 /// A computed cliprect for clip and image-region
 pub struct ClipRect {
-    pub top: Au,
+    pub top: Option<Au>,
     pub right: Option<Au>,
     pub bottom: Option<Au>,
-    pub left: Au,
+    pub left: Option<Au>,
 }
 
 impl ToCss for ClipRect {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
         try!(dest.write_str("rect("));
-        try!(self.top.to_css(dest));
-        try!(dest.write_str(", "));
+        if let Some(top) = self.top {
+            try!(top.to_css(dest));
+            try!(dest.write_str(", "));
+        } else {
+            try!(dest.write_str("auto, "));
+        }
+
         if let Some(right) = self.right {
             try!(right.to_css(dest));
             try!(dest.write_str(", "));
         } else {
             try!(dest.write_str("auto, "));
         }
 
         if let Some(bottom) = self.bottom {
             try!(bottom.to_css(dest));
             try!(dest.write_str(", "));
         } else {
             try!(dest.write_str("auto, "));
         }
 
-        try!(self.left.to_css(dest));
+        if let Some(left) = self.left {
+            try!(left.to_css(dest));
+        } else {
+            try!(dest.write_str("auto"));
+        }
         dest.write_str(")")
     }
 }
 
 /// rect(...) | auto
 pub type ClipRectOrAuto = Either<ClipRect, Auto>;
 
 /// The computed value of a grid `<track-breadth>`
--- a/servo/components/style/values/specified/mod.rs
+++ b/servo/components/style/values/specified/mod.rs
@@ -844,86 +844,94 @@ impl LoPOrNumber {
         } else {
             Err(())
         }
     }
 }
 
 impl HasViewportPercentage for ClipRect {
     fn has_viewport_percentage(&self) -> bool {
-        self.top.has_viewport_percentage() ||
+        self.top.as_ref().map_or(false, |x| x.has_viewport_percentage()) ||
         self.right.as_ref().map_or(false, |x| x.has_viewport_percentage()) ||
         self.bottom.as_ref().map_or(false, |x| x.has_viewport_percentage()) ||
-        self.left.has_viewport_percentage()
+        self.left.as_ref().map_or(false, |x| x.has_viewport_percentage())
     }
 }
 
 #[derive(Clone, Debug, PartialEq)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 /// rect(<top>, <left>, <bottom>, <right>) used by clip and image-region
 pub struct ClipRect {
-    /// <top> (<length> | <auto>). Auto maps to 0
-    pub top: Length,
+    /// <top> (<length> | <auto>)
+    pub top: Option<Length>,
     /// <right> (<length> | <auto>)
     pub right: Option<Length>,
     /// <bottom> (<length> | <auto>)
     pub bottom: Option<Length>,
-    /// <left> (<length> | <auto>). Auto maps to 0
-    pub left: Length,
+    /// <left> (<length> | <auto>)
+    pub left: Option<Length>,
 }
 
 
 impl ToCss for ClipRect {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
         try!(dest.write_str("rect("));
 
-        try!(self.top.to_css(dest));
-        try!(dest.write_str(", "));
+        if let Some(ref top) = self.top {
+            try!(top.to_css(dest));
+            try!(dest.write_str(", "));
+        } else {
+            try!(dest.write_str("auto, "));
+        }
 
         if let Some(ref right) = self.right {
             try!(right.to_css(dest));
             try!(dest.write_str(", "));
         } else {
             try!(dest.write_str("auto, "));
         }
 
         if let Some(ref bottom) = self.bottom {
             try!(bottom.to_css(dest));
             try!(dest.write_str(", "));
         } else {
             try!(dest.write_str("auto, "));
         }
 
-        try!(self.left.to_css(dest));
+        if let Some(ref left) = self.left {
+            try!(left.to_css(dest));
+        } else {
+            try!(dest.write_str("auto"));
+        }
 
         try!(dest.write_str(")"));
         Ok(())
     }
 }
 
 impl ToComputedValue for ClipRect {
     type ComputedValue = super::computed::ClipRect;
 
     #[inline]
     fn to_computed_value(&self, context: &Context) -> super::computed::ClipRect {
         super::computed::ClipRect {
-            top: self.top.to_computed_value(context),
+            top: self.top.as_ref().map(|top| top.to_computed_value(context)),
             right: self.right.as_ref().map(|right| right.to_computed_value(context)),
             bottom: self.bottom.as_ref().map(|bottom| bottom.to_computed_value(context)),
-            left: self.left.to_computed_value(context),
+            left: self.left.as_ref().map(|left| left.to_computed_value(context)),
         }
     }
 
     #[inline]
     fn from_computed_value(computed: &super::computed::ClipRect) -> Self {
         ClipRect {
-            top: ToComputedValue::from_computed_value(&computed.top),
+            top: computed.top.map(|top| ToComputedValue::from_computed_value(&top)),
             right: computed.right.map(|right| ToComputedValue::from_computed_value(&right)),
             bottom: computed.bottom.map(|bottom| ToComputedValue::from_computed_value(&bottom)),
-            left: ToComputedValue::from_computed_value(&computed.left),
+            left: computed.left.map(|left| ToComputedValue::from_computed_value(&left)),
         }
     }
 }
 
 impl Parse for ClipRect {
     fn parse(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
         use values::specified::Length;
 
@@ -934,17 +942,16 @@ impl Parse for ClipRect {
                 Length::parse(context, input).map(Some)
             }
         }
 
         if !try!(input.expect_function()).eq_ignore_ascii_case("rect") {
             return Err(())
         }
 
-        // NB: `top` and `left` are 0 if `auto` per CSS 2.1 11.1.2.
         input.parse_nested_block(|input| {
             let top = try!(parse_argument(context, input));
             let right;
             let bottom;
             let left;
 
             if input.try(|input| input.expect_comma()).is_ok() {
                 right = try!(parse_argument(context, input));
@@ -953,19 +960,19 @@ impl Parse for ClipRect {
                 try!(input.expect_comma());
                 left = try!(parse_argument(context, input));
             } else {
                 right = try!(parse_argument(context, input));
                 bottom = try!(parse_argument(context, input));
                 left = try!(parse_argument(context, input));
             }
             Ok(ClipRect {
-                top: top.unwrap_or(Length::zero()),
+                top: top,
                 right: right,
                 bottom: bottom,
-                left: left.unwrap_or(Length::zero()),
+                left: left,
             })
         })
     }
 }
 
 /// rect(...) | auto
 pub type ClipRectOrAuto = Either<ClipRect, Auto>;
new file mode 100644
--- /dev/null
+++ b/servo/tests/unit/style/parsing/effects.rs
@@ -0,0 +1,35 @@
+/* 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_clip() {
+    use style::properties::longhands::clip;
+
+    assert_roundtrip_with_context!(clip::parse, "auto");
+    assert_roundtrip_with_context!(clip::parse, "rect(1px, 2px, 3px, 4px)");
+    assert_roundtrip_with_context!(clip::parse, "rect(1px, auto, auto, 4px)");
+    assert_roundtrip_with_context!(clip::parse, "rect(auto, auto, auto, auto)");
+
+    // Non-standard syntax
+    assert_roundtrip_with_context!(clip::parse,
+                                   "rect(1px 2px 3px 4px)",
+                                   "rect(1px, 2px, 3px, 4px)");
+    assert_roundtrip_with_context!(clip::parse,
+                                   "rect(auto 2px 3px auto)",
+                                   "rect(auto, 2px, 3px, auto)");
+    assert_roundtrip_with_context!(clip::parse,
+                                   "rect(1px auto auto 4px)",
+                                   "rect(1px, auto, auto, 4px)");
+    assert_roundtrip_with_context!(clip::parse,
+                                   "rect(auto auto auto auto)",
+                                   "rect(auto, auto, auto, auto)");
+}
+
--- a/servo/tests/unit/style/parsing/mod.rs
+++ b/servo/tests/unit/style/parsing/mod.rs
@@ -77,16 +77,17 @@ macro_rules! parse_longhand {
     }};
 }
 
 mod animation;
 mod background;
 mod basic_shape;
 mod border;
 mod column;
+mod effects;
 mod font;
 mod image;
 mod inherited_box;
 mod inherited_text;
 mod mask;
 mod outline;
 mod position;
 mod selectors;