style: Get rid of gecko_size_type.

This also fixes a style sharing issue with MaxLength, anecdotally.

components/style/gecko/values.rs

@@ -16,8 +16,8 @@ use gecko_bindings::sugar::ns_style_coord::{CoordData, CoordDataMut, CoordDataVa
 use media_queries::Device;
 use nsstring::{nsACString, nsCStr};
 use std::cmp::max;
-use values::{Auto, Either, ExtremumLength, None_, Normal};
-use values::computed::{Angle, Length, LengthOrPercentage, LengthOrPercentageOrAuto};
+use values::{Auto, Either, None_, Normal};
+use values::computed::{Angle, ExtremumLength, Length, LengthOrPercentage, LengthOrPercentageOrAuto};
 use values::computed::{LengthOrPercentageOrNone, Number, NumberOrPercentage};
 use values::computed::{MaxLength, MozLength, Percentage};
 use values::computed::{NonNegativeLength, NonNegativeLengthOrPercentage, NonNegativeNumber};

components/style/properties/data.py

@@ -149,7 +149,6 @@ def __init__(self, style_struct, name, spec=None, animation_value_type=None, key
                  predefined_type=None, servo_pref=None, gecko_pref=None,
                  enabled_in="content", need_index=False,
                  gecko_ffi_name=None,
-                 is_gecko_size_type_hack=False,
                  allowed_in_keyframe_block=True, cast_type='u8',
                  logical=False, alias=None, extra_prefixes=None, boxed=False,
                  flags=None, allowed_in_page_rule=False, allow_quirks=False, ignored_when_colors_disabled=False,
@@ -186,7 +185,6 @@ def __init__(self, style_struct, name, spec=None, animation_value_type=None, key
         self.allow_quirks = allow_quirks
         self.ignored_when_colors_disabled = ignored_when_colors_disabled
         self.is_vector = vector
-        self.is_gecko_size_type_hack = is_gecko_size_type_hack
 
         # https://drafts.csswg.org/css-animations/#keyframes
         # > The <declaration-list> inside of <keyframe-block> accepts any CSS property

components/style/properties/helpers.mako.rs

@@ -894,110 +894,3 @@
         return "nsCSSPropertyID::eCSSProperty_%s" % ident
     %>
 </%def>
-
-// Define property that supports prefixed intrinsic size keyword values for gecko.
-// E.g. -moz-max-content, -moz-min-content, etc.
-//
-// FIXME(emilio): This feels a lot like a huge hack, get rid of this.
-<%def name="gecko_size_type(name, length_type, initial_value, logical, **kwargs)">
-    <%call expr="longhand(name,
-                          predefined_type=length_type,
-                          logical=logical,
-                          is_gecko_size_type_hack=True,
-                          **kwargs)">
-        % if not logical:
-            use values::specified::AllowQuirks;
-        % endif
-        use values::specified::${length_type};
-
-        pub mod computed_value {
-            pub type T = ::values::computed::${length_type};
-        }
-
-        #[derive(Clone, Debug, MallocSizeOf, PartialEq, ToCss)]
-        pub struct SpecifiedValue(pub ${length_type});
-
-        % if length_type == "MozLength":
-        impl SpecifiedValue {
-            /// Returns the `auto` value.
-            pub fn auto() -> Self {
-                use values::specified::length::LengthOrPercentageOrAuto;
-                SpecifiedValue(MozLength::LengthOrPercentageOrAuto(LengthOrPercentageOrAuto::Auto))
-            }
-
-            /// Returns a value representing a `0` length.
-            pub fn zero() -> Self {
-                use values::specified::length::LengthOrPercentageOrAuto;
-                SpecifiedValue(MozLength::LengthOrPercentageOrAuto(LengthOrPercentageOrAuto::zero()))
-            }
-        }
-        % endif
-
-        #[inline]
-        pub fn get_initial_value() -> computed_value::T {
-            use values::computed::${length_type};
-            ${length_type}::${initial_value}
-        }
-
-        impl Parse for SpecifiedValue {
-            fn parse<'i, 't>(
-                context: &ParserContext,
-                input: &mut Parser<'i, 't>,
-            ) -> Result<SpecifiedValue, ParseError<'i>> {
-                % if logical:
-                let ret = ${length_type}::parse(context, input);
-                % else:
-                let ret = ${length_type}::parse_quirky(context, input, AllowQuirks::Yes);
-                % endif
-                // Keyword values don't make sense in the block direction; don't parse them
-                % if "block" in name:
-                    if let Ok(${length_type}::ExtremumLength(..)) = ret {
-                        return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
-                    }
-                % endif
-                ret.map(SpecifiedValue)
-            }
-        }
-
-        fn parse<'i, 't>(
-            context: &ParserContext,
-            input: &mut Parser<'i, 't>,
-        ) -> Result<SpecifiedValue, ParseError<'i>> {
-            SpecifiedValue::parse(context, input)
-        }
-
-        impl ToComputedValue for SpecifiedValue {
-            type ComputedValue = computed_value::T;
-            #[inline]
-            fn to_computed_value(&self, context: &Context) -> computed_value::T {
-                % if not logical or "block" in name:
-                    use values::computed::${length_type};
-                % endif
-                let computed = self.0.to_computed_value(context);
-
-                // filter out keyword values in the block direction
-                % if logical:
-                    % if "block" in name:
-                        if let ${length_type}::ExtremumLength(..) = computed {
-                            return get_initial_value()
-                        }
-                    % endif
-                % else:
-                    if let ${length_type}::ExtremumLength(..) = computed {
-                        <% is_height = "true" if "height" in name else "false" %>
-                        if ${is_height} != context.builder.writing_mode.is_vertical() {
-                            return get_initial_value()
-                        }
-                    }
-                % endif
-                computed
-            }
-
-            #[inline]
-            fn from_computed_value(computed: &computed_value::T) -> Self {
-                SpecifiedValue(ToComputedValue::from_computed_value(computed))
-            }
-        }
-    </%call>
-</%def>
-

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

@@ -160,11 +160,6 @@ ${helpers.predefined_type("order", "Integer", "0",
                           animation_value_type="ComputedValue",
                           spec="https://drafts.csswg.org/css-flexbox/#order-property")}
 
-// FIXME(emilio): All the sizes stuff, and the MozLength values should be
-// unified with Servo, or at least be less hacky.
-//
-// The block direction ones don't even accept extremum lengths during parsing,
-// and should be converted to just LengthOrPercentage.
 % if product == "gecko":
     // FIXME: Gecko doesn't support content value yet.
     //
@@ -190,26 +185,51 @@ ${helpers.predefined_type("order", "Integer", "0",
 % endif
 % for (size, logical) in ALL_SIZES:
     <%
-      spec = "https://drafts.csswg.org/css-box/#propdef-%s"
-      if logical:
-        spec = "https://drafts.csswg.org/css-logical-props/#propdef-%s"
+        spec = "https://drafts.csswg.org/css-box/#propdef-%s"
+        if logical:
+            spec = "https://drafts.csswg.org/css-logical-props/#propdef-%s"
     %>
+    // NOTE: Block-size doesn't support -moz-*-content keywords, since they make
+    // no sense on the block axis, but it simplifies things the have that it has
+    // the same type as the other properties, since otherwise we'd need to
+    // handle logical props where the types are different, which looks like a
+    // pain.
     % if product == "gecko":
+        <%
+            parse_function = "parse" if size != "block-size" else "parse_disallow_keyword"
+        %>
         // width, height, block-size, inline-size
-        ${helpers.gecko_size_type(size, "MozLength", "auto()",
-                                  logical,
-                                  spec=spec % size,
-                                  animation_value_type="MozLength")}
+        ${helpers.predefined_type(
+            size,
+            "MozLength",
+            "computed::MozLength::auto()",
+            parse_function,
+            logical=logical,
+            allow_quirks=not logical,
+            spec=spec % size,
+            animation_value_type="ComputedValues"
+        )}
         // min-width, min-height, min-block-size, min-inline-size,
-        // max-width, max-height, max-block-size, max-inline-size
-        ${helpers.gecko_size_type("min-%s" % size, "MozLength", "auto()",
-                                  logical,
-                                  spec=spec % size,
-                                  animation_value_type="MozLength")}
-        ${helpers.gecko_size_type("max-%s" % size, "MaxLength", "none()",
-                                  logical,
-                                  spec=spec % size,
-                                  animation_value_type="MaxLength")}
+        ${helpers.predefined_type(
+            "min-%s" % size,
+            "MozLength",
+            "computed::MozLength::auto()",
+            parse_function,
+            logical=logical,
+            allow_quirks=not logical,
+            spec=spec % size,
+            animation_value_type="ComputedValue"
+        )}
+        ${helpers.predefined_type(
+            "max-%s" % size,
+            "MaxLength",
+            "computed::MaxLength::none()",
+            parse_function,
+            logical=logical,
+            allow_quirks=not logical,
+            spec=spec % size,
+            animation_value_type="ComputedValue",
+        )}
     % else:
         // servo versions (no keyword support)
         ${helpers.predefined_type(size,

components/style/properties/properties.mako.rs

@@ -208,7 +208,7 @@ pub mod animated_properties {
 
     variants = []
     for property in data.longhands:
-        if property.predefined_type and not property.is_vector and not property.is_gecko_size_type_hack:
+        if property.predefined_type and not property.is_vector:
             ty = "::values::specified::{}".format(property.predefined_type)
         else:
             ty = "longhands::{}::SpecifiedValue".format(property.ident)

components/style/values/computed/length.rs

@@ -5,13 +5,15 @@
 //! `<length>` computed values, and related ones.
 
 use app_units::Au;
+use logical_geometry::WritingMode;
 use ordered_float::NotNaN;
+use properties::LonghandId;
 use std::fmt::{self, Write};
 use std::ops::{Add, Neg};
 use style_traits::{CssWriter, ToCss};
 use style_traits::values::specified::AllowedNumericType;
 use super::{Number, ToComputedValue, Context, Percentage};
-use values::{Auto, CSSFloat, Either, ExtremumLength, None_, Normal, specified};
+use values::{Auto, CSSFloat, Either, None_, Normal, specified};
 use values::animated::{Animate, Procedure, ToAnimatedZero};
 use values::computed::NonNegativeNumber;
 use values::distance::{ComputeSquaredDistance, SquaredDistance};
@@ -874,6 +876,49 @@ pub type NonNegativeLengthOrNormal = Either<NonNegativeLength, Normal>;
 /// Either a computed NonNegativeLength or a NonNegativeNumber value.
 pub type NonNegativeLengthOrNumber = Either<NonNegativeLength, NonNegativeNumber>;
 
+/// A type for possible values for min- and max- flavors of width, height,
+/// block-size, and inline-size.
+#[allow(missing_docs)]
+#[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
+#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, Parse, PartialEq, ToCss)]
+pub enum ExtremumLength {
+    MozMaxContent,
+    MozMinContent,
+    MozFitContent,
+    MozAvailable,
+}
+
+impl ExtremumLength {
+    /// Returns whether this size keyword can be used for the given writing-mode
+    /// and property.
+    fn valid_for(&self, wm: WritingMode, longhand: LonghandId) -> bool {
+        // We only make sense on the inline axis.
+        match longhand {
+            LonghandId::MinWidth |
+            LonghandId::MaxWidth |
+            LonghandId::Width => !wm.is_vertical(),
+
+            LonghandId::MinHeight |
+            LonghandId::MaxHeight |
+            LonghandId::Height => wm.is_vertical(),
+
+            LonghandId::MinInlineSize |
+            LonghandId::MaxInlineSize |
+            LonghandId::InlineSize => true,
+            // The block-* properties are rejected at parse-time, so they're
+            // unexpected here.
+            _ => {
+                debug_assert!(
+                    false,
+                    "Unexpected property using ExtremumLength: {:?}",
+                    longhand,
+                );
+                false
+            }
+        }
+    }
+}
+
 /// A value suitable for a `min-width`, `min-height`, `width` or `height` property.
 /// See values/specified/length.rs for more details.
 #[allow(missing_docs)]
@@ -899,28 +944,37 @@ impl ToComputedValue for specified::MozLength {
 
     #[inline]
     fn to_computed_value(&self, context: &Context) -> MozLength {
+        debug_assert!(
+            context.for_non_inherited_property.is_some(),
+            "Someone added a MozLength to an inherited property? Evil!"
+        );
         match *self {
             specified::MozLength::LengthOrPercentageOrAuto(ref lopoa) => {
                 MozLength::LengthOrPercentageOrAuto(lopoa.to_computed_value(context))
             }
-            specified::MozLength::ExtremumLength(ref ext) => {
-                debug_assert!(context.for_non_inherited_property.is_some(),
-                              "should check whether we're a non-inherited property");
+            specified::MozLength::ExtremumLength(ext) => {
                 context.rule_cache_conditions.borrow_mut()
                     .set_writing_mode_dependency(context.builder.writing_mode);
-                MozLength::ExtremumLength(ext.clone())
+                if !ext.valid_for(context.builder.writing_mode, context.for_non_inherited_property.unwrap()) {
+                    MozLength::auto()
+                } else {
+                    MozLength::ExtremumLength(ext)
+                }
             }
         }
     }
 
     #[inline]
     fn from_computed_value(computed: &MozLength) -> Self {
         match *computed {
-            MozLength::LengthOrPercentageOrAuto(ref lopoa) =>
+            MozLength::LengthOrPercentageOrAuto(ref lopoa) => {
                 specified::MozLength::LengthOrPercentageOrAuto(
-                    specified::LengthOrPercentageOrAuto::from_computed_value(&lopoa)),
-            MozLength::ExtremumLength(ref ext) =>
-                specified::MozLength::ExtremumLength(ext.clone()),
+                    specified::LengthOrPercentageOrAuto::from_computed_value(lopoa)
+                )
+            },
+            MozLength::ExtremumLength(ext) => {
+                specified::MozLength::ExtremumLength(ext)
+            }
         }
     }
 }
@@ -938,21 +992,33 @@ pub enum MaxLength {
 
 impl MaxLength {
     /// Returns the `none` value.
+    #[inline]
     pub fn none() -> Self {
         MaxLength::LengthOrPercentageOrNone(LengthOrPercentageOrNone::None)
     }
 }
+
 impl ToComputedValue for specified::MaxLength {
     type ComputedValue = MaxLength;
 
     #[inline]
     fn to_computed_value(&self, context: &Context) -> MaxLength {
+        debug_assert!(
+            context.for_non_inherited_property.is_some(),
+            "Someone added a MaxLength to an inherited property? Evil!"
+        );
         match *self {
             specified::MaxLength::LengthOrPercentageOrNone(ref lopon) => {
                 MaxLength::LengthOrPercentageOrNone(lopon.to_computed_value(context))
             }
-            specified::MaxLength::ExtremumLength(ref ext) => {
-                MaxLength::ExtremumLength(ext.clone())
+            specified::MaxLength::ExtremumLength(ext) => {
+                context.rule_cache_conditions.borrow_mut()
+                    .set_writing_mode_dependency(context.builder.writing_mode);
+                if !ext.valid_for(context.builder.writing_mode, context.for_non_inherited_property.unwrap()) {
+                    MaxLength::none()
+                } else {
+                    MaxLength::ExtremumLength(ext)
+                }
             }
         }
     }

components/style/values/computed/mod.rs

@@ -58,7 +58,7 @@ pub use super::{Auto, Either, None_};
 pub use super::specified::{BorderStyle, TextDecorationLine};
 pub use self::length::{CalcLengthOrPercentage, Length, LengthOrNone, LengthOrNumber, LengthOrPercentage};
 pub use self::length::{LengthOrPercentageOrAuto, LengthOrPercentageOrNone, MaxLength, MozLength};
-pub use self::length::{CSSPixelLength, NonNegativeLength, NonNegativeLengthOrPercentage};
+pub use self::length::{CSSPixelLength, ExtremumLength, NonNegativeLength, NonNegativeLengthOrPercentage};
 pub use self::list::{ListStyleImage, Quotes};
 #[cfg(feature = "gecko")]
 pub use self::list::ListStyleType;

components/style/values/mod.rs

@@ -195,14 +195,3 @@ impl ToCss for KeyframesName {
     }
 }
 
-/// A type for possible values for min- and max- flavors of width,
-/// height, block-size, and inline-size.
-#[allow(missing_docs)]
-#[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
-#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, Parse, PartialEq, ToCss)]
-pub enum ExtremumLength {
-    MozMaxContent,
-    MozMinContent,
-    MozFitContent,
-    MozAvailable,
-}