Auto merge of #16006 - upsuper:box-ordinal-group, r=Manishearth

Implement -moz-box-ordinal-group property

This PR fixes #16000.

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

components/layout/fragment.rs

@@ -42,14 +42,14 @@ use std::sync::{Arc, Mutex};
 use style::arc_ptr_eq;
 use style::computed_values::{border_collapse, box_sizing, clear, color, display, mix_blend_mode};
 use style::computed_values::{overflow_wrap, overflow_x, position, text_decoration_line, transform};
-use style::computed_values::{transform_style, vertical_align, white_space, word_break, z_index};
+use style::computed_values::{transform_style, vertical_align, white_space, word_break};
 use style::computed_values::content::ContentItem;
 use style::logical_geometry::{Direction, LogicalMargin, LogicalRect, LogicalSize, WritingMode};
 use style::properties::ServoComputedValues;
 use style::selector_parser::RestyleDamage;
 use style::servo::restyle_damage::RECONSTRUCT_FLOW;
 use style::str::char_is_whitespace;
-use style::values::{self, Either};
+use style::values::{self, Either, Auto};
 use style::values::computed::{LengthOrPercentage, LengthOrPercentageOrAuto};
 use text;
 use text::TextRunScanner;
@@ -2512,15 +2512,15 @@ impl Fragment {
                self.style().get_box().overflow_x,
                self.style().get_box().overflow_y.0) {
             (position::T::absolute,
-             z_index::T::Auto,
+             Either::Second(Auto),
              overflow_x::T::visible,
              overflow_x::T::visible) |
             (position::T::fixed,
-             z_index::T::Auto,
+             Either::Second(Auto),
              overflow_x::T::visible,
              overflow_x::T::visible) |
             (position::T::relative,
-             z_index::T::Auto,
+             Either::Second(Auto),
              overflow_x::T::visible,
              overflow_x::T::visible) => false,
             (position::T::absolute, _, _, _) |
@@ -2536,15 +2536,15 @@ impl Fragment {
     pub fn effective_z_index(&self) -> i32 {
         match self.style().get_box().position {
             position::T::static_ => {},
-            _ => return self.style().get_position().z_index.number_or_zero(),
+            _ => return self.style().get_position().z_index.integer_or(0),
         }
 
         if self.style().get_box().transform.0.is_some() {
-            return self.style().get_position().z_index.number_or_zero();
+            return self.style().get_position().z_index.integer_or(0);
         }
 
         match self.style().get_box().display {
-            display::T::flex => self.style().get_position().z_index.number_or_zero(),
+            display::T::flex => self.style().get_position().z_index.integer_or(0),
             _ => 0,
         }
     }

components/layout/multicol.rs

@@ -106,11 +106,14 @@ impl Flow for MulticolFlow {
             if let Either::First(column_width) = column_style.column_width {
                 column_count =
                     max(1, (content_inline_size + column_gap).0 / (column_width + column_gap).0);
-                if let Some(specified_column_count) = column_style.column_count.0 {
+                if let Either::First(specified_column_count) = column_style.column_count {
                     column_count = min(column_count, specified_column_count as i32);
                 }
             } else {
-                column_count = column_style.column_count.0.unwrap() as i32;
+                column_count = match column_style.column_count {
+                    Either::First(n) => n,
+                    _ => unreachable!(),
+                }
             }
             column_width =
                 max(Au(0), (content_inline_size + column_gap) / column_count - column_gap);

components/style/properties/gecko.mako.rs

@@ -962,10 +962,9 @@ fn static_assert() {
     % endfor
 
     pub fn set_z_index(&mut self, v: longhands::z_index::computed_value::T) {
-        use properties::longhands::z_index::computed_value::T;
         match v {
-            T::Auto => self.gecko.mZIndex.set_value(CoordDataValue::Auto),
+            Either::First(n) => self.gecko.mZIndex.set_value(CoordDataValue::Integer(n)),
-            T::Number(n) => self.gecko.mZIndex.set_value(CoordDataValue::Integer(n)),
+            Either::Second(Auto) => self.gecko.mZIndex.set_value(CoordDataValue::Auto),
         }
     }
 
@@ -980,13 +979,12 @@ fn static_assert() {
     }
 
     pub fn clone_z_index(&self) -> longhands::z_index::computed_value::T {
-        use properties::longhands::z_index::computed_value::T;
         return match self.gecko.mZIndex.as_value() {
-            CoordDataValue::Auto => T::Auto,
+            CoordDataValue::Integer(n) => Either::First(n),
-            CoordDataValue::Integer(n) => T::Number(n),
+            CoordDataValue::Auto => Either::Second(Auto),
             _ => {
                 debug_assert!(false);
-                T::Number(0)
+                Either::First(0)
             }
         }
     }
@@ -3191,11 +3189,11 @@ clip-path
     pub fn set_column_count(&mut self, v: longhands::column_count::computed_value::T) {
         use gecko_bindings::structs::{NS_STYLE_COLUMN_COUNT_AUTO, nsStyleColumn_kMaxColumnCount};
 
-        self.gecko.mColumnCount = match v.0 {
+        self.gecko.mColumnCount = match v {
-            Some(number) => unsafe {
+            Either::First(number) => unsafe {
-                cmp::min(number, nsStyleColumn_kMaxColumnCount)
+                cmp::min(number as u32, nsStyleColumn_kMaxColumnCount)
             },
-            None => NS_STYLE_COLUMN_COUNT_AUTO
+            Either::Second(Auto) => NS_STYLE_COLUMN_COUNT_AUTO
         };
     }
 
@@ -3336,7 +3334,7 @@ clip-path
 </%self:impl_trait>
 
 <%self:impl_trait style_struct_name="XUL"
-                  skip_longhands="-moz-stack-sizing">
+                  skip_longhands="-moz-stack-sizing -moz-box-ordinal-group">
 
     #[allow(non_snake_case)]
     pub fn set__moz_stack_sizing(&mut self, v: longhands::_moz_stack_sizing::computed_value::T) {
@@ -3345,6 +3343,13 @@ clip-path
     }
 
     ${impl_simple_copy('_moz_stack_sizing', 'mStretchStack')}
+
+    #[allow(non_snake_case)]
+    pub fn set__moz_box_ordinal_group(&mut self, v: i32) {
+        self.gecko.mBoxOrdinal = v as u32;
+    }
+
+    ${impl_simple_copy("_moz_box_ordinal_group", "mBoxOrdinal")}
 </%self:impl_trait>
 
 <%def name="define_ffi_struct_accessor(style_struct)">

components/style/properties/helpers/animated_properties.mako.rs

@@ -22,7 +22,6 @@ use properties::longhands::transform::computed_value::ComputedOperation as Trans
 use properties::longhands::transform::computed_value::T as TransformList;
 use properties::longhands::vertical_align::computed_value::T as VerticalAlign;
 use properties::longhands::visibility::computed_value::T as Visibility;
-use properties::longhands::z_index::computed_value::T as ZIndex;
 #[cfg(feature = "gecko")] use properties::{PropertyDeclarationId, LonghandId};
 use std::cmp;
 #[cfg(feature = "gecko")] use std::collections::HashMap;
@@ -35,7 +34,6 @@ use values::computed::{Angle, LengthOrPercentageOrAuto, LengthOrPercentageOrNone
 use values::computed::{BorderRadiusSize, ClipRect, LengthOrNone};
 use values::computed::{CalcLengthOrPercentage, Context, LengthOrPercentage};
 use values::computed::{MaxLength, MinLength};
-use values::computed::ColorOrAuto;
 use values::computed::position::{HorizontalPosition, Position, VerticalPosition};
 use values::computed::ToComputedValue;
 use values::specified::Angle as SpecifiedAngle;
@@ -406,6 +404,13 @@ impl Interpolate for Au {
     }
 }
 
+impl Interpolate for Auto {
+    #[inline]
+    fn interpolate(&self, _other: &Self, _progress: f64) -> Result<Self, ()> {
+        Ok(Auto)
+    }
+}
+
 impl <T> Interpolate for Option<T>
     where T: Interpolate,
 {
@@ -436,7 +441,7 @@ impl Interpolate for f64 {
     }
 }
 
-/// https://drafts.csswg.org/css-transitions/#animtype-number
+/// https://drafts.csswg.org/css-transitions/#animtype-integer
 impl Interpolate for i32 {
     #[inline]
     fn interpolate(&self, other: &i32, progress: f64) -> Result<Self, ()> {
@@ -473,20 +478,6 @@ impl Interpolate for Visibility {
     }
 }
 
-/// https://drafts.csswg.org/css-transitions/#animtype-integer
-impl Interpolate for ZIndex {
-    #[inline]
-    fn interpolate(&self, other: &Self, progress: f64) -> Result<Self, ()> {
-        match (*self, *other) {
-            (ZIndex::Number(ref this),
-             ZIndex::Number(ref other)) => {
-                this.interpolate(other, progress).map(ZIndex::Number)
-            }
-            _ => Err(()),
-        }
-    }
-}
-
 impl<T: Interpolate + Copy> Interpolate for Size2D<T> {
     #[inline]
     fn interpolate(&self, other: &Self, progress: f64) -> Result<Self, ()> {
@@ -1839,16 +1830,17 @@ impl Interpolate for TransformList {
     }
 }
 
-/// https://drafts.csswg.org/css-transitions-1/#animtype-color
+impl<T, U> Interpolate for Either<T, U>
-impl Interpolate for ColorOrAuto {
+        where T: Interpolate + Copy, U: Interpolate + Copy,
+{
     #[inline]
     fn interpolate(&self, other: &Self, progress: f64) -> Result<Self, ()> {
         match (*self, *other) {
             (Either::First(ref this), Either::First(ref other)) => {
                 this.interpolate(&other, progress).map(Either::First)
             },
-            (Either::Second(Auto), Either::Second(Auto)) => {
+            (Either::Second(ref this), Either::Second(ref other)) => {
-                Ok(Either::Second(Auto))
+                this.interpolate(&other, progress).map(Either::Second)
             },
             _ => {
                 let interpolated = if progress < 0.5 { *self } else { *other };

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

@@ -19,90 +19,14 @@ ${helpers.predefined_type("column-width",
 
 
 // FIXME: This prop should be animatable.
-<%helpers:longhand name="column-count" experimental="True" animatable="False" extra_prefixes="moz"
+${helpers.predefined_type("column-count", "IntegerOrAuto",
-                   spec="https://drafts.csswg.org/css-multicol/#propdef-column-count">
+                          "Either::Second(Auto)",
-    use std::fmt;
+                          parse_method="parse_positive",
-    use style_traits::ToCss;
+                          initial_specified_value="Either::Second(Auto)",
-    use values::HasViewportPercentage;
+                          experimental="True",
-
+                          animatable="False",
-    no_viewport_percentage!(SpecifiedValue);
+                          extra_prefixes="moz",
-
+                          spec="https://drafts.csswg.org/css-multicol/#propdef-column-count")}
-    #[derive(Debug, Clone, Copy, PartialEq)]
-    #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-    pub enum SpecifiedValue {
-        Auto,
-        Specified(u32),
-    }
-
-    impl ToCss for SpecifiedValue {
-        fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-            match *self {
-                SpecifiedValue::Auto => dest.write_str("auto"),
-                SpecifiedValue::Specified(count) => write!(dest, "{}", count),
-            }
-        }
-    }
-
-    pub mod computed_value {
-        #[derive(Debug, Clone, PartialEq)]
-        #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-        pub struct T(pub Option<u32>);
-    }
-
-    impl ToCss for computed_value::T {
-        fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-            match self.0 {
-                None => dest.write_str("auto"),
-                Some(count) => write!(dest, "{}", count),
-            }
-        }
-    }
-
-    #[inline]
-    pub fn get_initial_value() -> computed_value::T {
-        computed_value::T(None)
-    }
-
-    #[inline]
-    pub fn get_initial_specified_value() -> SpecifiedValue {
-        SpecifiedValue::Auto
-    }
-
-    impl ToComputedValue for SpecifiedValue {
-        type ComputedValue = computed_value::T;
-
-        #[inline]
-        fn to_computed_value(&self, _context: &Context) -> computed_value::T {
-            match *self {
-                SpecifiedValue::Auto => computed_value::T(None),
-                SpecifiedValue::Specified(count) =>
-                    computed_value::T(Some(count))
-            }
-        }
-
-        #[inline]
-        fn from_computed_value(computed: &computed_value::T) -> Self {
-            match *computed {
-                computed_value::T(None) => SpecifiedValue::Auto,
-                computed_value::T(Some(count)) =>
-                    SpecifiedValue::Specified(count)
-            }
-        }
-    }
-
-    pub fn parse(_context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
-        if input.try(|input| input.expect_ident_matching("auto")).is_ok() {
-            Ok(SpecifiedValue::Auto)
-        } else {
-            let count = try!(specified::parse_integer(input));
-            // Zero is invalid
-            if count <= 0 {
-                return Err(())
-            }
-            Ok(SpecifiedValue::Specified(count as u32))
-        }
-    }
-</%helpers:longhand>
 
 // FIXME: This prop should be animatable.
 ${helpers.predefined_type("column-gap",

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

@@ -23,54 +23,10 @@
                               animatable=True, logical=True)}
 % endfor
 
-<%helpers:longhand name="z-index" spec="https://www.w3.org/TR/CSS2/visuren.html#z-index" animatable="True">
+${helpers.predefined_type("z-index", "IntegerOrAuto",
-    use values::HasViewportPercentage;
+                          "Either::Second(Auto)",
-    use values::computed::ComputedValueAsSpecified;
+                          spec="https://www.w3.org/TR/CSS2/visuren.html#z-index",
-
+                          animatable="True")}
-    impl ComputedValueAsSpecified for SpecifiedValue {}
-    no_viewport_percentage!(SpecifiedValue);
-    pub type SpecifiedValue = computed_value::T;
-    pub mod computed_value {
-        use std::fmt;
-        use style_traits::ToCss;
-
-        #[derive(PartialEq, Clone, Eq, Copy, Debug)]
-        #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-        pub enum T {
-            Auto,
-            Number(i32),
-        }
-
-        impl ToCss for T {
-            fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-                match *self {
-                    T::Auto => dest.write_str("auto"),
-                    T::Number(number) => write!(dest, "{}", number),
-                }
-            }
-        }
-
-        impl T {
-            pub fn number_or_zero(self) -> i32 {
-                match self {
-                    T::Auto => 0,
-                    T::Number(value) => value,
-                }
-            }
-        }
-    }
-    #[inline]
-    pub fn get_initial_value() -> computed_value::T {
-        computed_value::T::Auto
-    }
-    fn parse(_context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
-        if input.try(|input| input.expect_ident_matching("auto")).is_ok() {
-            Ok(computed_value::T::Auto)
-        } else {
-            specified::parse_integer(input).map(computed_value::T::Number)
-        }
-    }
-</%helpers:longhand>
 
 // CSS Flexible Box Layout Module Level 1
 // http://www.w3.org/TR/css3-flexbox/

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

@@ -49,3 +49,12 @@ ${helpers.single_keyword("-moz-stack-sizing", "stretch-to-fit ignore",
                          gecko_constant_prefix="NS_STYLE_STACK_SIZING",
                          animatable=False,
                          spec="Nonstandard (https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-stack-sizing)")}
+
+${helpers.predefined_type("-moz-box-ordinal-group", "Integer", "0",
+                          parse_method="parse_non_negative",
+                          needs_context=False,
+                          products="gecko",
+                          alias="-webkit-box-ordinal-group",
+                          gecko_ffi_name="mBoxOrdinal",
+                          animatable=False,
+                          spec="Nonstandard (https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-box-ordinal-group)")}

components/style/properties/properties.mako.rs

@@ -1536,7 +1536,10 @@ impl ComputedValues {
         let style = self.get_column();
         match style.column_width {
             Either::First(_width) => true,
-            Either::Second(_auto) => style.column_count.0.is_some(),
+            Either::Second(_auto) => match style.column_count {
+                Either::First(_n) => true,
+                Either::Second(_auto) => false,
+            }
         }
     }
 

components/style/values/computed/mod.rs

@@ -11,7 +11,7 @@ use media_queries::Device;
 use properties::ComputedValues;
 use std::fmt;
 use style_traits::ToCss;
-use super::{CSSFloat, RGBA, specified};
+use super::{CSSFloat, CSSInteger, RGBA, specified};
 use super::specified::grid::{TrackBreadth as GenericTrackBreadth, TrackSize as GenericTrackSize};
 
 pub use cssparser::Color as CSSColor;
@@ -267,6 +267,23 @@ pub type Number = CSSFloat;
 /// A type used for opacity.
 pub type Opacity = CSSFloat;
 
+/// A `<integer>` value.
+pub type Integer = CSSInteger;
+
+/// <integer> | auto
+pub type IntegerOrAuto = Either<CSSInteger, Auto>;
+
+impl IntegerOrAuto {
+    /// Returns the integer value if it is an integer, otherwise return
+    /// the given value.
+    pub fn integer_or(&self, auto_value: CSSInteger) -> CSSInteger {
+        match *self {
+            Either::First(n) => n,
+            Either::Second(Auto) => auto_value,
+        }
+    }
+}
+
 
 /// An SVG paint value
 ///

components/style/values/mod.rs

@@ -66,6 +66,9 @@ pub mod specified;
 /// A CSS float value.
 pub type CSSFloat = f32;
 
+/// A CSS integer value.
+pub type CSSInteger = i32;
+
 /// The default font size.
 pub const FONT_MEDIUM_PX: i32 = 16;
 

components/style/values/specified/mod.rs

@@ -17,7 +17,7 @@ use std::f32::consts::PI;
 use std::fmt;
 use std::ops::Mul;
 use style_traits::ToCss;
-use super::{Auto, CSSFloat, HasViewportPercentage, Either, None_};
+use super::{Auto, CSSFloat, CSSInteger, HasViewportPercentage, Either, None_};
 use super::computed::{ComputedValueAsSpecified, Context};
 use super::computed::{Shadow as ComputedShadow, ToComputedValue};
 
@@ -167,7 +167,7 @@ impl<'a> Mul<CSSFloat> for &'a SimplifiedValueNode {
 }
 
 #[allow(missing_docs)]
-pub fn parse_integer(input: &mut Parser) -> Result<i32, ()> {
+pub fn parse_integer(input: &mut Parser) -> Result<CSSInteger, ()> {
     match try!(input.next()) {
         Token::Number(ref value) => value.int_value.ok_or(()),
         Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
@@ -178,7 +178,7 @@ pub fn parse_integer(input: &mut Parser) -> Result<i32, ()> {
             for ref node in ast.products {
                 match try!(CalcLengthOrPercentage::simplify_product(node)) {
                     SimplifiedValueNode::Number(val) =>
-                        result = Some(result.unwrap_or(0) + val as i32),
+                        result = Some(result.unwrap_or(0) + val as CSSInteger),
                     _ => unreachable!()
                 }
             }
@@ -576,6 +576,69 @@ impl ToCss for Opacity {
     }
 }
 
+#[derive(Clone, Copy, Debug, PartialEq, PartialOrd)]
+#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+#[allow(missing_docs)]
+pub struct Integer(pub CSSInteger);
+
+no_viewport_percentage!(Integer);
+
+impl Parse for Integer {
+    fn parse(_context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
+        parse_integer(input).map(Integer)
+    }
+}
+
+impl Integer {
+    fn parse_with_minimum(input: &mut Parser, min: i32) -> Result<Integer, ()> {
+        match parse_integer(input) {
+            Ok(value) if value < min => Err(()),
+            value => value.map(Integer),
+        }
+    }
+
+    #[allow(missing_docs)]
+    pub fn parse_non_negative(input: &mut Parser) -> Result<Integer, ()> {
+        Integer::parse_with_minimum(input, 0)
+    }
+
+    #[allow(missing_docs)]
+    pub fn parse_positive(input: &mut Parser) -> Result<Integer, ()> {
+        Integer::parse_with_minimum(input, 1)
+    }
+}
+
+impl ToComputedValue for Integer {
+    type ComputedValue = i32;
+
+    #[inline]
+    fn to_computed_value(&self, _: &Context) -> i32 { self.0 }
+
+    #[inline]
+    fn from_computed_value(computed: &i32) -> Self {
+        Integer(*computed)
+    }
+}
+
+impl ToCss for Integer {
+    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+        write!(dest, "{}", self.0)
+    }
+}
+
+/// <integer> | auto
+pub type IntegerOrAuto = Either<Integer, Auto>;
+
+impl IntegerOrAuto {
+    #[allow(missing_docs)]
+    pub fn parse_positive(context: &ParserContext, input: &mut Parser) -> Result<IntegerOrAuto, ()> {
+        match IntegerOrAuto::parse(context, input) {
+            Ok(Either::First(Integer(value))) if value <= 0 => Err(()),
+            result => result,
+        }
+    }
+}
+
 #[allow(missing_docs)]
 pub type UrlOrNone = Either<SpecifiedUrl, None_>;
 

tests/unit/style/properties/serialization.rs

@@ -557,13 +557,12 @@ mod shorthand_serialization {
 
     #[test]
     fn columns_should_serialize_correctly() {
-        use style::properties::longhands::column_count::SpecifiedValue as ColumnCount;
         use style::values::{Auto, Either};
 
         let mut properties = Vec::new();
 
         let width = Either::Second(Auto);
-        let count = ColumnCount::Auto;
+        let count = Either::Second(Auto);
 
         properties.push(PropertyDeclaration::ColumnWidth(width));
         properties.push(PropertyDeclaration::ColumnCount(count));