diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 067d014442d..82e52853fea 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1806,7 +1806,7 @@ fn static_assert() { } pub fn set_computed_justify_items(&mut self, v: values::specified::JustifyItems) { - debug_assert_ne!(v.0, ::values::specified::align::AlignFlags::AUTO); + debug_assert_ne!(v.0, ::values::specified::align::AlignFlags::LEGACY); self.gecko.mJustifyItems = v.into(); } diff --git a/components/style/properties/longhand/position.mako.rs b/components/style/properties/longhand/position.mako.rs index 5d64159654c..bfda37e8a19 100644 --- a/components/style/properties/longhand/position.mako.rs +++ b/components/style/properties/longhand/position.mako.rs @@ -118,11 +118,13 @@ ${helpers.single_keyword("flex-wrap", "nowrap wrap wrap-reverse", #[cfg(feature = "gecko")] impl_align_conversions!(::values::specified::align::AlignItems); - ${helpers.predefined_type(name="justify-items", - type="JustifyItems", - initial_value="computed::JustifyItems::auto()", - spec="https://drafts.csswg.org/css-align/#propdef-justify-items", - animation_value_type="discrete")} + ${helpers.predefined_type( + name="justify-items", + type="JustifyItems", + initial_value="computed::JustifyItems::legacy()", + spec="https://drafts.csswg.org/css-align/#propdef-justify-items", + animation_value_type="discrete", + )} #[cfg(feature = "gecko")] impl_align_conversions!(::values::specified::align::JustifyItems); diff --git a/components/style/style_adjuster.rs b/components/style/style_adjuster.rs index b7270976ead..32bd68db52d 100644 --- a/components/style/style_adjuster.rs +++ b/components/style/style_adjuster.rs @@ -673,17 +673,15 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { } } - /// Resolves "justify-items: auto" based on the inherited style if needed to - /// comply with: + /// Resolves "justify-items: legacy" based on the inherited style if needed + /// to comply with: /// /// - /// - /// (Note that "auto" is being renamed to "legacy") #[cfg(feature = "gecko")] fn adjust_for_justify_items(&mut self) { use values::specified::align; let justify_items = self.style.get_position().clone_justify_items(); - if justify_items.specified.0 != align::AlignFlags::AUTO { + if justify_items.specified.0 != align::AlignFlags::LEGACY { return; } diff --git a/components/style/values/computed/align.rs b/components/style/values/computed/align.rs index 940e3f747b8..1a26e41fd24 100644 --- a/components/style/values/computed/align.rs +++ b/components/style/values/computed/align.rs @@ -20,10 +20,10 @@ pub use super::specified::{AlignSelf, JustifySelf}; /// In particular, `justify-items` is a reset property, so we ought to be able /// to share its computed representation across elements as long as they match /// the same rules. Except that it's not true if the specified value for -/// `justify-items` is `auto` and the computed value of the parent has the +/// `justify-items` is `legacy` and the computed value of the parent has the /// `legacy` modifier. /// -/// So instead of computing `auto` "normally" looking at get_parent_position(), +/// So instead of computing `legacy` "normally" looking at get_parent_position(), /// marking it as uncacheable, we carry the specified value around and handle /// the special case in `StyleAdjuster` instead, only when the result of the /// computation would vary. @@ -35,18 +35,21 @@ pub use super::specified::{AlignSelf, JustifySelf}; /// See the discussion in https://bugzil.la/1384542. #[derive(Clone, Copy, Debug, Eq, PartialEq, ToCss)] pub struct JustifyItems { - /// The specified value for the property. Can contain `auto`. + /// The specified value for the property. Can contain the bare `legacy` + /// keyword. #[css(skip)] pub specified: specified::JustifyItems, - /// The computed value for the property. Cannot contain `auto`. + /// The computed value for the property. Cannot contain the bare `legacy` + /// keyword, but note that it could contain it in combination with other + /// keywords like `left`, `right` or `center`. pub computed: specified::JustifyItems, } impl JustifyItems { - /// Returns the `auto` value. - pub fn auto() -> Self { + /// Returns the `legacy` value. + pub fn legacy() -> Self { Self { - specified: specified::JustifyItems::auto(), + specified: specified::JustifyItems::legacy(), computed: specified::JustifyItems::normal(), } } @@ -59,13 +62,13 @@ impl ToComputedValue for specified::JustifyItems { fn to_computed_value(&self, _context: &Context) -> JustifyItems { use values::specified::align; let specified = *self; - let computed = if self.0 != align::AlignFlags::AUTO { + let computed = if self.0 != align::AlignFlags::LEGACY { *self } else { // If the inherited value of `justify-items` includes the - // `legacy` keyword, `auto` computes to the inherited value, - // but we assume it computes to `normal`, and handle that - // special-case in StyleAdjuster. + // `legacy` keyword, `legacy` computes to the inherited value, but + // we assume it computes to `normal`, and handle that special-case + // in StyleAdjuster. Self::normal() }; diff --git a/components/style/values/specified/align.rs b/components/style/values/specified/align.rs index 918c4ddf8ed..7b9b003a50c 100644 --- a/components/style/values/specified/align.rs +++ b/components/style/values/specified/align.rs @@ -9,9 +9,8 @@ use cssparser::Parser; use gecko_bindings::structs; use parser::{Parse, ParserContext}; -use selectors::parser::SelectorParseErrorKind; use std::fmt::{self, Write}; -use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; +use style_traits::{CssWriter, ParseError, ToCss}; bitflags! { /// Constants shared by multiple CSS Box Alignment properties @@ -81,14 +80,26 @@ impl ToCss for AlignFlags { where W: Write, { - match *self & AlignFlags::FLAG_BITS { - AlignFlags::LEGACY => dest.write_str("legacy ")?, + let extra_flags = *self & AlignFlags::FLAG_BITS; + let value = self.value(); + + match extra_flags { + AlignFlags::LEGACY => { + dest.write_str("legacy")?; + if value.is_empty() { + return Ok(()); + } + dest.write_char(' ')?; + }, AlignFlags::SAFE => dest.write_str("safe ")?, // Don't serialize "unsafe", since it's the default. - _ => {}, + AlignFlags::UNSAFE => {}, + _ => { + debug_assert_eq!(extra_flags, AlignFlags::empty()); + }, } - dest.write_str(match self.value() { + dest.write_str(match value { AlignFlags::AUTO => "auto", AlignFlags::NORMAL => "normal", AlignFlags::START => "start", @@ -436,10 +447,10 @@ impl Parse for AlignItems { pub struct JustifyItems(pub AlignFlags); impl JustifyItems { - /// The initial value 'auto' + /// The initial value 'legacy' #[inline] - pub fn auto() -> Self { - JustifyItems(AlignFlags::AUTO) + pub fn legacy() -> Self { + JustifyItems(AlignFlags::LEGACY) } /// The value 'normal' @@ -462,24 +473,12 @@ impl Parse for JustifyItems { return Ok(JustifyItems(baseline)); } - // auto | normal | stretch - // - // FIXME(emilio): auto is no longer a keyword in the current spec, and - // has been renamed to legacy, but that needs different changes because - // right now it's the initial value for both style systems, and has that - // weird behavior of "inheriting" into descendants. - // - // Fix this in both. - // - // See also: - // https://bugs.webkit.org/show_bug.cgi?id=172711 - // https://bugs.chromium.org/p/chromium/issues/detail?id=726148 - // - if let Ok(value) = input.try(parse_auto_normal_stretch) { + // normal | stretch + if let Ok(value) = input.try(parse_normal_stretch) { return Ok(JustifyItems(value)); } - // [ legacy || [ left | right | center ] ] + // legacy | [ legacy && [ left | right | center ] ] if let Ok(value) = input.try(parse_legacy) { return Ok(JustifyItems(value)); } @@ -567,29 +566,30 @@ fn parse_self_position<'i, 't>( }) } -// [ legacy && [ left | right | center ] ] -fn parse_legacy<'i, 't>(input: &mut Parser<'i, 't>) -> Result> { - let a_location = input.current_source_location(); - let a = input.expect_ident()?.clone(); - let b_location = input.current_source_location(); - let b = input.expect_ident()?; - if a.eq_ignore_ascii_case("legacy") { - (match_ignore_ascii_case! { &b, - "left" => Ok(AlignFlags::LEGACY | AlignFlags::LEFT), - "right" => Ok(AlignFlags::LEGACY | AlignFlags::RIGHT), - "center" => Ok(AlignFlags::LEGACY | AlignFlags::CENTER), - _ => Err(()) - }).map_err(|()| { - b_location.new_custom_error(SelectorParseErrorKind::UnexpectedIdent(b.clone())) - }) - } else if b.eq_ignore_ascii_case("legacy") { - (match_ignore_ascii_case! { &a, - "left" => Ok(AlignFlags::LEGACY | AlignFlags::LEFT), - "right" => Ok(AlignFlags::LEGACY | AlignFlags::RIGHT), - "center" => Ok(AlignFlags::LEGACY | AlignFlags::CENTER), - _ => Err(()) - }).map_err(|()| a_location.new_custom_error(SelectorParseErrorKind::UnexpectedIdent(a))) - } else { - Err(a_location.new_custom_error(StyleParseErrorKind::UnspecifiedError)) - } +fn parse_left_right_center<'i, 't>( + input: &mut Parser<'i, 't>, +) -> Result> { + Ok(try_match_ident_ignore_ascii_case! { input, + "left" => AlignFlags::LEFT, + "right" => AlignFlags::RIGHT, + "center" => AlignFlags::CENTER, + }) +} + +// legacy | [ legacy && [ left | right | center ] ] +fn parse_legacy<'i, 't>(input: &mut Parser<'i, 't>) -> Result> { + let flags = try_match_ident_ignore_ascii_case! { input, + "legacy" => { + let flags = input.try(parse_left_right_center) + .unwrap_or(AlignFlags::empty()); + + return Ok(AlignFlags::LEGACY | flags) + } + "left" => AlignFlags::LEFT, + "right" => AlignFlags::RIGHT, + "center" => AlignFlags::CENTER, + }; + + input.expect_ident_matching("legacy")?; + Ok(AlignFlags::LEGACY | flags) }