Auto merge of #16597 - Manishearth:stylo-overflow, r=emilio

stylo: support all overflow values

overflow:clip doesn't exist, it's just called clip internally. Renamed, and added the other missing values.

I also removed the overflow newtype -- no need for extra code bloat, and it's not protecting us from much.

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

components/layout/block.rs

@@ -53,7 +53,7 @@ use servo_geometry::max_rect;
 use std::cmp::{max, min};
 use std::fmt;
 use std::sync::Arc;
-use style::computed_values::{border_collapse, box_sizing, display, float, overflow_x, overflow_y};
+use style::computed_values::{border_collapse, box_sizing, display, float, overflow_x};
 use style::computed_values::{position, text_align};
 use style::context::SharedStyleContext;
 use style::logical_geometry::{LogicalPoint, LogicalRect, LogicalSize, WritingMode};
@@ -1440,7 +1440,7 @@ impl BlockFlow {
                 FormattingContextType::Other
             }
             _ if style.get_box().overflow_x != overflow_x::T::visible ||
-                    style.get_box().overflow_y != overflow_y::T(overflow_x::T::visible) ||
+                    style.get_box().overflow_y != overflow_x::T::visible ||
                     style.is_multicol() => {
                 FormattingContextType::Block
             }
@@ -1677,7 +1677,7 @@ impl BlockFlow {
 
     pub fn overflow_style_may_require_scroll_root(&self) -> bool {
         match (self.fragment.style().get_box().overflow_x,
-               self.fragment.style().get_box().overflow_y.0) {
+               self.fragment.style().get_box().overflow_y) {
             (overflow_x::T::auto, _) | (overflow_x::T::scroll, _) | (overflow_x::T::hidden, _) |
             (_, overflow_x::T::auto) | (_, overflow_x::T::scroll) | (_, overflow_x::T::hidden) =>
                 true,

components/layout/display_list_builder.rs

@@ -2132,7 +2132,7 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
             self.base.overflow.scroll.size.width > content_box.size.width ||
             self.base.overflow.scroll.size.height > content_box.size.height ||
             overflow_x::T::hidden == self.fragment.style.get_box().overflow_x ||
-            overflow_x::T::hidden == self.fragment.style.get_box().overflow_y.0;
+            overflow_x::T::hidden == self.fragment.style.get_box().overflow_y;
 
         self.mark_scrolling_overflow(has_scrolling_overflow);
         if !has_scrolling_overflow {
@@ -2163,11 +2163,11 @@ impl BlockFlowDisplayListBuilding for BlockFlow {
             content_size.width = content_box.size.width;
         }
 
-        if overflow_x::T::hidden == self.fragment.style.get_box().overflow_y.0 {
+        if overflow_x::T::hidden == self.fragment.style.get_box().overflow_y {
             content_size.height = content_box.size.height;
         }
 
-        if overflow_x::T::hidden == self.fragment.style.get_box().overflow_y.0 ||
+        if overflow_x::T::hidden == self.fragment.style.get_box().overflow_y ||
            overflow_x::T::hidden == self.fragment.style.get_box().overflow_x {
             preserved_state.push_clip(state, &border_box, self.positioning());
         }

components/layout/flow.rs

@@ -276,7 +276,7 @@ pub trait Flow: fmt::Debug + Sync + Send + 'static {
             overflow.scroll.origin.x = Au(0);
             overflow.scroll.size.width = border_box.size.width;
         }
-        if overflow_x::T::visible != self.as_block().fragment.style.get_box().overflow_y.0 {
+        if overflow_x::T::visible != self.as_block().fragment.style.get_box().overflow_y {
             overflow.paint.origin.y = Au(0);
             overflow.paint.size.height = border_box.size.height;
             overflow.scroll.origin.y = Au(0);

components/layout/fragment.rs

@@ -2150,7 +2150,7 @@ impl Fragment {
             let block_flow = flow.as_block();
             let start_margin = block_flow.fragment.margin.block_start;
             let end_margin = block_flow.fragment.margin.block_end;
-            if style.get_box().overflow_y.0 == overflow_x::T::visible {
+            if style.get_box().overflow_y == overflow_x::T::visible {
                 if let Some(baseline_offset) = flow.baseline_offset_of_last_line_box_in_flow() {
                     let ascent = baseline_offset + start_margin;
                     let space_below_baseline = block_flow.fragment.border_box.size.block -
@@ -2491,7 +2491,7 @@ impl Fragment {
         match (self.style().get_box().position,
                self.style().get_position().z_index,
                self.style().get_box().overflow_x,
-               self.style().get_box().overflow_y.0) {
+               self.style().get_box().overflow_y) {
             (position::T::absolute,
              Either::Second(Auto),
              overflow_x::T::visible,

components/layout/query.rs

@@ -885,7 +885,7 @@ pub fn process_node_overflow_request<N: LayoutNode>(requested_node: N) -> NodeOv
     let style = &*layout_node.as_element().unwrap().resolved_style();
     let style_box = style.get_box();
 
-    NodeOverflowResponse(Some((Point2D::new(style_box.overflow_x, style_box.overflow_y.0))))
+    NodeOverflowResponse(Some((Point2D::new(style_box.overflow_x, style_box.overflow_y))))
 }
 
 pub fn process_margin_style_query<N: LayoutNode>(requested_node: N)

components/style/properties/gecko.mako.rs

@@ -1832,13 +1832,11 @@ fn static_assert() {
 
     <%call expr="impl_keyword_clone('display', 'mDisplay', display_keyword)"></%call>
 
-    // overflow-y is implemented as a newtype of overflow-x, so we need special handling.
-    // We could generalize this if we run into other newtype keywords.
     <% overflow_x = data.longhands_by_name["overflow-x"] %>
     pub fn set_overflow_y(&mut self, v: longhands::overflow_y::computed_value::T) {
         use properties::longhands::overflow_x::computed_value::T as BaseType;
         // FIXME(bholley): Align binary representations and ditch |match| for cast + static_asserts
-        self.gecko.mOverflowY = match v.0 {
+        self.gecko.mOverflowY = match v {
             % for value in overflow_x.keyword.values_for('gecko'):
                 BaseType::${to_rust_ident(value)} => structs::${overflow_x.keyword.gecko_constant(value)} as u8,
             % endfor
@@ -1847,11 +1845,10 @@ fn static_assert() {
     ${impl_simple_copy('overflow_y', 'mOverflowY')}
     pub fn clone_overflow_y(&self) -> longhands::overflow_y::computed_value::T {
         use properties::longhands::overflow_x::computed_value::T as BaseType;
-        use properties::longhands::overflow_y::computed_value::T as NewType;
         // FIXME(bholley): Align binary representations and ditch |match| for cast + static_asserts
         match self.gecko.mOverflowY as u32 {
             % for value in overflow_x.keyword.values_for('gecko'):
-            structs::${overflow_x.keyword.gecko_constant(value)} => NewType(BaseType::${to_rust_ident(value)}),
+            structs::${overflow_x.keyword.gecko_constant(value)} => BaseType::${to_rust_ident(value)},
             % endfor
             x => panic!("Found unexpected value in style struct for overflow_y property: {}", x),
         }

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

@@ -351,57 +351,23 @@ ${helpers.single_keyword("overflow-clip-box", "padding-box content-box",
     spec="Internal, not web-exposed, \
           may be standardized in the future (https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-clip-box)")}
 
+<%
+    overflow_custom_consts = { "-moz-hidden-unscrollable": "CLIP" }
+%>
+
 // FIXME(pcwalton, #2742): Implement scrolling for `scroll` and `auto`.
 ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
-                         extra_gecko_values="clip",
                          need_clone=True, animation_value_type="none",
+                         extra_gecko_aliases="-moz-scrollbars-none=hidden",
+                         extra_gecko_values="-moz-hidden-unscrollable",
+                         custom_consts=overflow_custom_consts,
                          gecko_constant_prefix="NS_STYLE_OVERFLOW",
                          spec="https://drafts.csswg.org/css-overflow/#propdef-overflow-x")}
 
 // FIXME(pcwalton, #2742): Implement scrolling for `scroll` and `auto`.
 <%helpers:longhand name="overflow-y" need_clone="True" animation_value_type="none"
                    spec="https://drafts.csswg.org/css-overflow/#propdef-overflow-y">
-    use super::overflow_x;
+    pub use super::overflow_x::{SpecifiedValue, parse, get_initial_value, computed_value};
-
-    use std::fmt;
-    use style_traits::ToCss;
-    use values::computed::ComputedValueAsSpecified;
-    use values::HasViewportPercentage;
-
-    no_viewport_percentage!(SpecifiedValue);
-
-    impl ToCss for SpecifiedValue {
-        fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-            self.0.to_css(dest)
-        }
-    }
-
-    /// The specified and computed value for overflow-y is a wrapper on top of
-    /// `overflow-x`, so we re-use the logic, but prevent errors from mistakenly
-    /// assign one to other.
-    ///
-    /// TODO(Manishearth, emilio): We may want to just use the same value.
-    pub mod computed_value {
-        pub use super::SpecifiedValue as T;
-    }
-
-    #[derive(Debug, Clone, Copy, PartialEq)]
-    #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-    pub struct SpecifiedValue(pub super::overflow_x::SpecifiedValue);
-
-    impl ComputedValueAsSpecified for SpecifiedValue {}
-
-    #[inline]
-    #[allow(missing_docs)]
-    pub fn get_initial_value() -> computed_value::T {
-        computed_value::T(overflow_x::get_initial_value())
-    }
-
-    #[inline]
-    #[allow(missing_docs)]
-    pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue,()> {
-        overflow_x::parse(context, input).map(SpecifiedValue)
-    }
 </%helpers:longhand>
 
 <%helpers:vector_longhand name="transition-duration"

components/style/properties/properties.mako.rs

@@ -2434,10 +2434,9 @@ pub fn apply_declarations<'a, F, I>(device: &Device,
 
     {
         use computed_values::overflow_x::T as overflow;
-        use computed_values::overflow_y;
 
         let original_overflow_x = style.get_box().clone_overflow_x();
-        let original_overflow_y = style.get_box().clone_overflow_y().0;
+        let original_overflow_y = style.get_box().clone_overflow_y();
         let mut overflow_x = original_overflow_x;
         let mut overflow_y = original_overflow_y;
 
@@ -2457,10 +2456,10 @@ pub fn apply_declarations<'a, F, I>(device: &Device,
             % if product == "gecko":
                 // overflow: clip is deprecated, so convert to hidden if it's
                 // specified in only one dimension.
-                if overflow_x == overflow::clip {
+                if overflow_x == overflow::_moz_hidden_unscrollable {
                     overflow_x = overflow::hidden;
                 }
-                if overflow_y == overflow::clip {
+                if overflow_y == overflow::_moz_hidden_unscrollable {
                     overflow_y = overflow::hidden;
                 }
             % endif
@@ -2471,10 +2470,10 @@ pub fn apply_declarations<'a, F, I>(device: &Device,
             // When 'contain: paint', update overflow from 'visible' to 'clip'.
             if style.get_box().clone_contain().contains(contain::PAINT) {
                 if overflow_x == overflow::visible {
-                    overflow_x = overflow::clip;
+                    overflow_x = overflow::_moz_hidden_unscrollable;
                 }
                 if overflow_y == overflow::visible {
-                    overflow_y = overflow::clip;
+                    overflow_y = overflow::_moz_hidden_unscrollable;
                 }
             }
         % endif
@@ -2483,7 +2482,7 @@ pub fn apply_declarations<'a, F, I>(device: &Device,
            overflow_y != original_overflow_y {
             let mut box_style = style.mutate_box();
             box_style.set_overflow_x(overflow_x);
-            box_style.set_overflow_y(overflow_y::T(overflow_y));
+            box_style.set_overflow_y(overflow_y);
         }
     }
 

components/style/properties/shorthand/box.mako.rs

@@ -6,19 +6,43 @@
 
 <%helpers:shorthand name="overflow" sub_properties="overflow-x overflow-y"
                     spec="https://drafts.csswg.org/css-overflow/#propdef-overflow">
-    use properties::longhands::{overflow_x, overflow_y};
+    use properties::longhands::overflow_x::parse as parse_overflow;
+    % if product == "gecko":
+        use properties::longhands::overflow_x::SpecifiedValue;
+    % endif
 
     pub fn parse_value(context: &ParserContext, input: &mut Parser) -> Result<Longhands, ()> {
-        let overflow = try!(overflow_x::parse(context, input));
+        % if product == "gecko":
+            let moz_kw_found = input.try(|i| match_ignore_ascii_case! {
+                &i.expect_ident()?,
+                "-moz-scrollbars-horizontal" => {
+                    Ok(Longhands {
+                        overflow_x: SpecifiedValue::scroll,
+                        overflow_y: SpecifiedValue::hidden,
+                    })
+                }
+                "-moz-scrollbars-vertical" => {
+                    Ok(Longhands {
+                        overflow_x: SpecifiedValue::hidden,
+                        overflow_y: SpecifiedValue::scroll,
+                    })
+                }
+                _ => Err(())
+            });
+            if moz_kw_found.is_ok() {
+                return moz_kw_found
+            }
+        % endif
+        let overflow = try!(parse_overflow(context, input));
         Ok(Longhands {
             overflow_x: overflow,
-            overflow_y: overflow_y::SpecifiedValue(overflow),
+            overflow_y: overflow,
         })
     }
 
     impl<'a> ToCss for LonghandsToSerialize<'a>  {
         fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-            if *self.overflow_x == self.overflow_y.0 {
+            if self.overflow_x == self.overflow_y {
                 self.overflow_x.to_css(dest)
             } else {
                 Ok(())

tests/unit/style/properties/serialization.rs

@@ -16,8 +16,7 @@ use stylesheets::block_from;
 
 #[test]
 fn property_declaration_block_should_serialize_correctly() {
-    use style::properties::longhands::overflow_x::SpecifiedValue as OverflowXValue;
+    use style::properties::longhands::overflow_x::SpecifiedValue as OverflowValue;
-    use style::properties::longhands::overflow_y::SpecifiedValue as OverflowYContainer;
 
     let declarations = vec![
         (PropertyDeclaration::Width(
@@ -37,11 +36,11 @@ fn property_declaration_block_should_serialize_correctly() {
          Importance::Normal),
 
         (PropertyDeclaration::OverflowX(
-            OverflowXValue::auto),
+            OverflowValue::auto),
          Importance::Normal),
 
         (PropertyDeclaration::OverflowY(
-            OverflowYContainer(OverflowXValue::auto)),
+            OverflowValue::auto),
          Importance::Normal),
     ];
 
@@ -68,18 +67,15 @@ mod shorthand_serialization {
 
     mod overflow {
         pub use super::*;
-        use style::properties::longhands::overflow_x::SpecifiedValue as OverflowXValue;
+        use style::properties::longhands::overflow_x::SpecifiedValue as OverflowValue;
-        use style::properties::longhands::overflow_y::SpecifiedValue as OverflowYContainer;
 
         #[test]
         fn equal_overflow_properties_should_serialize_to_single_value() {
             let mut properties = Vec::new();
 
-            let overflow_x = OverflowXValue::auto;
+            let overflow = OverflowValue::auto;
-            properties.push(PropertyDeclaration::OverflowX(overflow_x));
+            properties.push(PropertyDeclaration::OverflowX(overflow));
-
+            properties.push(PropertyDeclaration::OverflowY(overflow));
-            let overflow_y = OverflowYContainer(OverflowXValue::auto);
-            properties.push(PropertyDeclaration::OverflowY(overflow_y));
 
             let serialization = shorthand_properties_to_string(properties);
             assert_eq!(serialization, "overflow: auto;");
@@ -89,10 +85,10 @@ mod shorthand_serialization {
         fn different_overflow_properties_should_serialize_to_two_values() {
             let mut properties = Vec::new();
 
-            let overflow_x = OverflowXValue::scroll;
+            let overflow_x = OverflowValue::scroll;
             properties.push(PropertyDeclaration::OverflowX(overflow_x));
 
-            let overflow_y = OverflowYContainer(OverflowXValue::auto);
+            let overflow_y = OverflowValue::auto;
             properties.push(PropertyDeclaration::OverflowY(overflow_y));
 
             let serialization = shorthand_properties_to_string(properties);