From 45ba167030f7bf746b61e61eae57a89b528ff6c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 6 Feb 2018 22:19:12 +0100 Subject: [PATCH 1/5] style: Make MozLength / MaxLength a bit more clear and do a bit less work. --- components/style/values/computed/length.rs | 1 + components/style/values/specified/length.rs | 26 ++++++++++++++++----- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/components/style/values/computed/length.rs b/components/style/values/computed/length.rs index 49db7201a39..b130b68eb98 100644 --- a/components/style/values/computed/length.rs +++ b/components/style/values/computed/length.rs @@ -888,6 +888,7 @@ pub enum MozLength { impl MozLength { /// Returns the `auto` value. + #[inline] pub fn auto() -> Self { MozLength::LengthOrPercentageOrAuto(LengthOrPercentageOrAuto::Auto) } diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index 9ac32c208d9..b1d7a19e2c5 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -1131,9 +1131,16 @@ impl MozLength { input: &mut Parser<'i, 't>, allow_quirks: AllowQuirks, ) -> Result> { - input.try(ExtremumLength::parse).map(MozLength::ExtremumLength) - .or_else(|_| input.try(|i| LengthOrPercentageOrAuto::parse_non_negative_quirky(context, i, allow_quirks)) - .map(MozLength::LengthOrPercentageOrAuto)) + if let Ok(l) = input.try(ExtremumLength::parse) { + return Ok(MozLength::ExtremumLength(l)); + } + + let length = LengthOrPercentageOrAuto::parse_non_negative_quirky( + context, + input, + allow_quirks, + )?; + Ok(MozLength::LengthOrPercentageOrAuto(length)) } } @@ -1158,8 +1165,15 @@ impl MaxLength { input: &mut Parser<'i, 't>, allow_quirks: AllowQuirks, ) -> Result> { - input.try(ExtremumLength::parse).map(MaxLength::ExtremumLength) - .or_else(|_| input.try(|i| LengthOrPercentageOrNone::parse_non_negative_quirky(context, i, allow_quirks)) - .map(MaxLength::LengthOrPercentageOrNone)) + if let Ok(l) = input.try(ExtremumLength::parse) { + return Ok(MaxLength::ExtremumLength(l)); + } + + let length = LengthOrPercentageOrNone::parse_non_negative_quirky( + context, + input, + allow_quirks, + )?; + Ok(MaxLength::LengthOrPercentageOrNone(length)) } } From a0d07699986a12fb1a93c4ec58622a44c2923045 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 6 Feb 2018 22:30:46 +0100 Subject: [PATCH 2/5] style: Move flex-basis out of gecko_size_type. --- components/style/properties/helpers.mako.rs | 6 ------ .../style/properties/longhand/position.mako.rs | 17 ++++++++++++----- components/style/values/specified/length.rs | 13 +++++++++++++ 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index cfb7fbbfa45..5e90fc06394 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -930,12 +930,6 @@ use values::specified::length::LengthOrPercentageOrAuto; SpecifiedValue(MozLength::LengthOrPercentageOrAuto(LengthOrPercentageOrAuto::zero())) } - - /// Returns a value representing a `0%` length. - pub fn zero_percent() -> Self { - use values::specified::length::LengthOrPercentageOrAuto; - SpecifiedValue(MozLength::LengthOrPercentageOrAuto(LengthOrPercentageOrAuto::zero_percent())) - } } % endif diff --git a/components/style/properties/longhand/position.mako.rs b/components/style/properties/longhand/position.mako.rs index cfc54d2d00c..795aa7513e4 100644 --- a/components/style/properties/longhand/position.mako.rs +++ b/components/style/properties/longhand/position.mako.rs @@ -167,11 +167,18 @@ ${helpers.predefined_type("order", "Integer", "0", // and should be converted to just LengthOrPercentage. % if product == "gecko": // FIXME: Gecko doesn't support content value yet. - ${helpers.gecko_size_type("flex-basis", "MozLength", "auto()", - logical=False, - spec="https://drafts.csswg.org/css-flexbox/#flex-basis-property", - extra_prefixes="webkit", - animation_value_type="MozLength")} + // + // FIXME(emilio): I suspect this property shouldn't allow quirks, and this + // was just a mistake. + ${helpers.predefined_type( + "flex-basis", + "MozLength", + "computed::MozLength::auto()", + extra_prefixes="webkit", + animation_value_type="ComputedValue", + allow_quirks=True, + spec="https://drafts.csswg.org/css-flexbox/#flex-basis-property" + )} % else: // FIXME: This property should be animatable. ${helpers.predefined_type("flex-basis", diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index b1d7a19e2c5..1e669e5b8c8 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -933,6 +933,7 @@ impl LengthOrPercentageOrAuto { } /// Returns a value representing `0%`. + #[inline] pub fn zero_percent() -> Self { LengthOrPercentageOrAuto::Percentage(computed::Percentage::zero()) } @@ -1142,6 +1143,18 @@ impl MozLength { )?; Ok(MozLength::LengthOrPercentageOrAuto(length)) } + + /// Returns `auto`. + #[inline] + pub fn auto() -> Self { + MozLength::LengthOrPercentageOrAuto(LengthOrPercentageOrAuto::auto()) + } + + /// Returns `0%`. + #[inline] + pub fn zero_percent() -> Self { + MozLength::LengthOrPercentageOrAuto(LengthOrPercentageOrAuto::zero_percent()) + } } /// A value suitable for a `max-width` or `max-height` property. From 100f4cd7c346b383726054f9ff1d285701cd4841 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 6 Feb 2018 22:40:46 +0100 Subject: [PATCH 3/5] style: "%s" % foo is pointless. --- components/style/properties/longhand/position.mako.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/style/properties/longhand/position.mako.rs b/components/style/properties/longhand/position.mako.rs index 795aa7513e4..4fac4560c19 100644 --- a/components/style/properties/longhand/position.mako.rs +++ b/components/style/properties/longhand/position.mako.rs @@ -196,7 +196,7 @@ ${helpers.predefined_type("order", "Integer", "0", %> % if product == "gecko": // width, height, block-size, inline-size - ${helpers.gecko_size_type("%s" % size, "MozLength", "auto()", + ${helpers.gecko_size_type(size, "MozLength", "auto()", logical, spec=spec % size, animation_value_type="MozLength")} @@ -212,7 +212,7 @@ ${helpers.predefined_type("order", "Integer", "0", animation_value_type="MaxLength")} % else: // servo versions (no keyword support) - ${helpers.predefined_type("%s" % size, + ${helpers.predefined_type(size, "LengthOrPercentageOrAuto", "computed::LengthOrPercentageOrAuto::Auto", "parse_non_negative", 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 4/5] 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, From ed2ba30261726a7922225c990fc2c7bd2ad5bcad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 7 Feb 2018 03:11:14 +0100 Subject: [PATCH 5/5] style: Sprinkle some comments. --- components/style/properties/longhand/position.mako.rs | 3 ++- components/style/values/computed/length.rs | 10 +++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/components/style/properties/longhand/position.mako.rs b/components/style/properties/longhand/position.mako.rs index f89bb95368f..818899ba7cb 100644 --- a/components/style/properties/longhand/position.mako.rs +++ b/components/style/properties/longhand/position.mako.rs @@ -164,7 +164,8 @@ ${helpers.predefined_type("order", "Integer", "0", // FIXME: Gecko doesn't support content value yet. // // FIXME(emilio): I suspect this property shouldn't allow quirks, and this - // was just a mistake. + // was just a mistake, it's kind of justificable to support it given the + // spec grammar is just `content | `, but other browsers don't... ${helpers.predefined_type( "flex-basis", "MozLength", diff --git a/components/style/values/computed/length.rs b/components/style/values/computed/length.rs index 7dc4f6dc7a1..a9d211a235e 100644 --- a/components/style/values/computed/length.rs +++ b/components/style/values/computed/length.rs @@ -891,9 +891,15 @@ pub enum ExtremumLength { impl ExtremumLength { /// Returns whether this size keyword can be used for the given writing-mode /// and property. + /// + /// TODO: After these values are supported for both axes (and maybe + /// unprefixed, see bug 1322780) all this complexity can go away, and + /// everything can be derived (no need for uncacheable stuff). fn valid_for(&self, wm: WritingMode, longhand: LonghandId) -> bool { // We only make sense on the inline axis. match longhand { + // FIXME(emilio): The flex-basis thing is not quite clear... + LonghandId::FlexBasis | LonghandId::MinWidth | LonghandId::MaxWidth | LonghandId::Width => !wm.is_vertical(), @@ -919,7 +925,9 @@ impl ExtremumLength { } } -/// A value suitable for a `min-width`, `min-height`, `width` or `height` property. +/// A value suitable for a `min-width`, `min-height`, `width` or `height` +/// property. +/// /// See values/specified/length.rs for more details. #[allow(missing_docs)] #[cfg_attr(feature = "servo", derive(MallocSizeOf))]