From 32c409d5aa636c590769e35d883d27bf4c6f5cdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 6 Feb 2018 23:01:50 +0100 Subject: [PATCH] style: Get rid of gecko_size_type. This also fixes a style sharing issue with MaxLength, anecdotally. --- components/style/gecko/values.rs | 4 +- components/style/properties/data.py | 2 - components/style/properties/helpers.mako.rs | 107 ------------------ .../properties/longhand/position.mako.rs | 62 ++++++---- .../style/properties/properties.mako.rs | 2 +- components/style/values/computed/length.rs | 88 ++++++++++++-- components/style/values/computed/mod.rs | 2 +- components/style/values/mod.rs | 11 -- components/style/values/specified/length.rs | 26 ++++- ports/geckolib/glue.rs | 23 ++-- 10 files changed, 154 insertions(+), 173 deletions(-) diff --git a/components/style/gecko/values.rs b/components/style/gecko/values.rs index d24e9ea33ac..0695f1e5144 100644 --- a/components/style/gecko/values.rs +++ b/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}; diff --git a/components/style/properties/data.py b/components/style/properties/data.py index 1d46ff9481b..ac7f9e427f7 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -149,7 +149,6 @@ class Longhand(object): 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 @@ class Longhand(object): 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 inside of accepts any CSS property diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index 5e90fc06394..a3279a68b98 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -894,110 +894,3 @@ return "nsCSSPropertyID::eCSSProperty_%s" % ident %> - -// 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> { - % 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::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)) - } - } - - - diff --git a/components/style/properties/longhand/position.mako.rs b/components/style/properties/longhand/position.mako.rs index 4fac4560c19..f89bb95368f 100644 --- a/components/style/properties/longhand/position.mako.rs +++ b/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, diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 4ddd4cb7941..ee35b7bf4f4 100644 --- a/components/style/properties/properties.mako.rs +++ b/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) diff --git a/components/style/values/computed/length.rs b/components/style/values/computed/length.rs index b130b68eb98..7dc4f6dc7a1 100644 --- a/components/style/values/computed/length.rs +++ b/components/style/values/computed/length.rs @@ -5,13 +5,15 @@ //! `` 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; /// Either a computed NonNegativeLength or a NonNegativeNumber value. pub type NonNegativeLengthOrNumber = Either; +/// 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,16 +944,22 @@ 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) + } } } } @@ -916,11 +967,14 @@ impl ToComputedValue for specified::MozLength { #[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) + } } } } diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index e883cc79a89..004e44cbbcc 100644 --- a/components/style/values/computed/mod.rs +++ b/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; diff --git a/components/style/values/mod.rs b/components/style/values/mod.rs index d75400d9e7e..48efdd64d40 100644 --- a/components/style/values/mod.rs +++ b/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, -} diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index 1e669e5b8c8..85c400e6472 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -18,8 +18,8 @@ use style_traits::{ParseError, StyleParseErrorKind}; use style_traits::values::specified::AllowedNumericType; use stylesheets::CssRuleType; use super::{AllowQuirks, Number, ToComputedValue, Percentage}; -use values::{Auto, CSSFloat, Either, ExtremumLength, None_, Normal}; -use values::computed::{self, CSSPixelLength, Context}; +use values::{Auto, CSSFloat, Either, None_, Normal}; +use values::computed::{self, CSSPixelLength, Context, ExtremumLength}; use values::generics::NonNegative; use values::specified::NonNegativeNumber; use values::specified::calc::CalcNode; @@ -1126,6 +1126,17 @@ impl Parse for MozLength { } impl MozLength { + /// Parses, without quirks, and disallowing ExtremumLength values. + /// + /// Used for logical props in the block direction. + pub fn parse_disallow_keyword<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + let length = LengthOrPercentageOrAuto::parse_non_negative(context, input)?; + Ok(MozLength::LengthOrPercentageOrAuto(length)) + } + /// Parses, with quirks. pub fn parse_quirky<'i, 't>( context: &ParserContext, @@ -1172,6 +1183,17 @@ impl Parse for MaxLength { } impl MaxLength { + /// Parses, without quirks, and disallowing ExtremumLength values. + /// + /// Used for logical props in the block direction. + pub fn parse_disallow_keyword<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + ) -> Result> { + let length = LengthOrPercentageOrNone::parse_non_negative(context, input)?; + Ok(MaxLength::LengthOrPercentageOrNone(length)) + } + /// Parses, with quirks. pub fn parse_quirky<'i, 't>( context: &ParserContext, diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 224f3a3798e..968c28bb523 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -3138,8 +3138,6 @@ pub extern "C" fn Servo_DeclarationBlock_SetPixelValue( ) { use style::properties::{PropertyDeclaration, LonghandId}; use style::properties::longhands::border_spacing::SpecifiedValue as BorderSpacing; - use style::properties::longhands::height::SpecifiedValue as Height; - use style::properties::longhands::width::SpecifiedValue as Width; use style::values::specified::{BorderSideWidth, MozLength, BorderCornerRadius}; use style::values::specified::length::{NoCalcLength, NonNegativeLength, LengthOrPercentage}; @@ -3147,8 +3145,8 @@ pub extern "C" fn Servo_DeclarationBlock_SetPixelValue( let nocalc = NoCalcLength::from_px(value); let prop = match_wrap_declared! { long, - Height => Height(MozLength::LengthOrPercentageOrAuto(nocalc.into())), - Width => Width(MozLength::LengthOrPercentageOrAuto(nocalc.into())), + Height => MozLength::LengthOrPercentageOrAuto(nocalc.into()), + Width => MozLength::LengthOrPercentageOrAuto(nocalc.into()), BorderTopWidth => BorderSideWidth::Length(nocalc.into()), BorderRightWidth => BorderSideWidth::Length(nocalc.into()), BorderBottomWidth => BorderSideWidth::Length(nocalc.into()), @@ -3197,7 +3195,6 @@ pub extern "C" fn Servo_DeclarationBlock_SetLengthValue( ) { use style::properties::{PropertyDeclaration, LonghandId}; use style::properties::longhands::_moz_script_min_size::SpecifiedValue as MozScriptMinSize; - use style::properties::longhands::width::SpecifiedValue as Width; use style::values::specified::MozLength; use style::values::specified::length::{AbsoluteLength, FontRelativeLength}; use style::values::specified::length::{LengthOrPercentage, NoCalcLength}; @@ -3217,7 +3214,7 @@ pub extern "C" fn Servo_DeclarationBlock_SetLengthValue( }; let prop = match_wrap_declared! { long, - Width => Width(MozLength::LengthOrPercentageOrAuto(nocalc.into())), + Width => MozLength::LengthOrPercentageOrAuto(nocalc.into()), FontSize => LengthOrPercentage::from(nocalc).into(), MozScriptMinSize => MozScriptMinSize(nocalc), }; @@ -3255,8 +3252,6 @@ pub extern "C" fn Servo_DeclarationBlock_SetPercentValue( value: f32, ) { use style::properties::{PropertyDeclaration, LonghandId}; - use style::properties::longhands::height::SpecifiedValue as Height; - use style::properties::longhands::width::SpecifiedValue as Width; use style::values::computed::Percentage; use style::values::specified::MozLength; use style::values::specified::length::LengthOrPercentage; @@ -3265,8 +3260,8 @@ pub extern "C" fn Servo_DeclarationBlock_SetPercentValue( let pc = Percentage(value); let prop = match_wrap_declared! { long, - Height => Height(MozLength::LengthOrPercentageOrAuto(pc.into())), - Width => Width(MozLength::LengthOrPercentageOrAuto(pc.into())), + Height => MozLength::LengthOrPercentageOrAuto(pc.into()), + Width => MozLength::LengthOrPercentageOrAuto(pc.into()), MarginTop => pc.into(), MarginRight => pc.into(), MarginBottom => pc.into(), @@ -3284,16 +3279,14 @@ pub extern "C" fn Servo_DeclarationBlock_SetAutoValue( property: nsCSSPropertyID, ) { use style::properties::{PropertyDeclaration, LonghandId}; - use style::properties::longhands::height::SpecifiedValue as Height; - use style::properties::longhands::width::SpecifiedValue as Width; - use style::values::specified::LengthOrPercentageOrAuto; + use style::values::specified::{LengthOrPercentageOrAuto, MozLength}; let long = get_longhand_from_id!(property); let auto = LengthOrPercentageOrAuto::Auto; let prop = match_wrap_declared! { long, - Height => Height::auto(), - Width => Width::auto(), + Height => MozLength::auto(), + Width => MozLength::auto(), MarginTop => auto, MarginRight => auto, MarginBottom => auto,