style: Make content distribution parsing know the axis it's parsed for. draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 24 Jan 2018 13:02:42 +0100
changeset 747862 e33105723cb17b21533073b5a199971ca5d7b2ac
parent 747861 e009270cd439c6d1ff86007cffae666104d500a5
child 747863 e99521508635c48527df4f7ffe887f8444cc8f23
push id97026
push userbmo:emilio@crisal.io
push dateFri, 26 Jan 2018 23:13:24 +0000
milestone60.0a1
style: Make content distribution parsing know the axis it's parsed for. MozReview-Commit-ID: LMPXVtKU1mq
servo/components/style/properties/longhand/position.mako.rs
servo/components/style/properties/shorthand/position.mako.rs
servo/components/style/values/computed/align.rs
servo/components/style/values/computed/mod.rs
servo/components/style/values/specified/align.rs
servo/components/style/values/specified/mod.rs
--- a/servo/components/style/properties/longhand/position.mako.rs
+++ b/servo/components/style/properties/longhand/position.mako.rs
@@ -64,18 +64,18 @@ macro_rules! impl_align_conversions {
 % if product == "servo":
     // FIXME: Update Servo to support the same Syntax as Gecko.
     ${helpers.single_keyword("justify-content", "flex-start stretch flex-end center space-between space-around",
                              extra_prefixes="webkit",
                              spec="https://drafts.csswg.org/css-align/#propdef-justify-content",
                              animation_value_type="discrete")}
 % else:
     ${helpers.predefined_type(name="justify-content",
-                              type="ContentDistribution",
-                              initial_value="specified::ContentDistribution::normal()",
+                              type="JustifyContent",
+                              initial_value="specified::JustifyContent(specified::ContentDistribution::normal())",
                               spec="https://drafts.csswg.org/css-align/#propdef-justify-content",
                               extra_prefixes="webkit",
                               animation_value_type="discrete")}
 % endif
 
 % if product == "servo":
     // FIXME: Update Servo to support the same Syntax as Gecko.
     ${helpers.single_keyword("align-content", "stretch flex-start flex-end center space-between space-around",
@@ -85,18 +85,18 @@ macro_rules! impl_align_conversions {
 
     ${helpers.single_keyword("align-items",
                              "stretch flex-start flex-end center baseline",
                              extra_prefixes="webkit",
                              spec="https://drafts.csswg.org/css-flexbox/#align-items-property",
                              animation_value_type="discrete")}
 % else:
     ${helpers.predefined_type(name="align-content",
-                              type="ContentDistribution",
-                              initial_value="specified::ContentDistribution::normal()",
+                              type="AlignContent",
+                              initial_value="specified::AlignContent(specified::ContentDistribution::normal())",
                               spec="https://drafts.csswg.org/css-align/#propdef-align-content",
                               extra_prefixes="webkit",
                               animation_value_type="discrete")}
 
     ${helpers.predefined_type(name="align-items",
                               type="AlignItems",
                               initial_value="specified::AlignItems::normal()",
                               spec="https://drafts.csswg.org/css-align/#propdef-align-items",
--- a/servo/components/style/properties/shorthand/position.mako.rs
+++ b/servo/components/style/properties/shorthand/position.mako.rs
@@ -610,43 +610,54 @@
             Ok(())
         }
     }
 </%helpers:shorthand>
 
 <%helpers:shorthand name="place-content" sub_properties="align-content justify-content"
                     spec="https://drafts.csswg.org/css-align/#propdef-place-content"
                     products="gecko">
-    use values::specified::align::{ContentDistribution, FallbackAllowed};
+    use values::specified::align::{AlignContent, JustifyContent, ContentDistribution, FallbackAllowed, AxisDirection};
 
     pub fn parse_value<'i, 't>(
         _: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Longhands, ParseError<'i>> {
-        let align = ContentDistribution::parse_with_fallback(input, FallbackAllowed::No)?;
-        if align.has_extra_flags() {
-            return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
-        }
-        let justify =
-            input.try(|input| ContentDistribution::parse_with_fallback(input, FallbackAllowed::No))
-                .unwrap_or(align);
-        if justify.has_extra_flags() {
+        let align_content =
+            ContentDistribution::parse(
+                input,
+                FallbackAllowed::No,
+                AxisDirection::Inline,
+            )?;
+
+        let justify_content = input.try(|input| {
+            ContentDistribution::parse(
+                input,
+                FallbackAllowed::No,
+                AxisDirection::Block,
+            )
+        }).unwrap_or(align_content);
+
+        // NOTE(emilio): align-content parsing is more restrictive than
+        // justify-content parsing, so no need to do any extra check here for
+        // that.
+        if align_content.has_extra_flags() || justify_content.has_extra_flags() {
             return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
         }
 
         Ok(expanded! {
-            align_content: align,
-            justify_content: justify,
+            align_content: AlignContent(align_content),
+            justify_content: JustifyContent(justify_content),
         })
     }
 
     impl<'a> ToCss for LonghandsToSerialize<'a> {
         fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
             self.align_content.to_css(dest)?;
-            if self.align_content != self.justify_content {
+            if self.align_content.0 != self.justify_content.0 {
                 dest.write_str(" ")?;
                 self.justify_content.to_css(dest)?;
             }
             Ok(())
         }
     }
 </%helpers:shorthand>
 
--- a/servo/components/style/values/computed/align.rs
+++ b/servo/components/style/values/computed/align.rs
@@ -6,17 +6,17 @@
 //!
 //! https://drafts.csswg.org/css-align/
 
 use std::fmt;
 use style_traits::{CssWriter, ToCss};
 use values::computed::{Context, ToComputedValue};
 use values::specified;
 
-pub use super::specified::{AlignItems, ContentDistribution, SelfAlignment};
+pub use super::specified::{AlignContent, JustifyContent, AlignItems, SelfAlignment};
 
 /// The computed value for the `justify-items` property.
 ///
 /// Need to carry around both the specified and computed value to handle the
 /// special legacy keyword. Sigh.
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 pub struct JustifyItems {
     /// The specified value for the property. Can contain `auto`.
--- a/servo/components/style/values/computed/mod.rs
+++ b/servo/components/style/values/computed/mod.rs
@@ -27,17 +27,17 @@ use super::generics::{GreaterThanOrEqual
 use super::generics::grid::{GridLine as GenericGridLine, TrackBreadth as GenericTrackBreadth};
 use super::generics::grid::{TrackSize as GenericTrackSize, TrackList as GenericTrackList};
 use super::generics::grid::GridTemplateComponent as GenericGridTemplateComponent;
 use super::specified;
 
 pub use app_units::Au;
 pub use properties::animated_properties::TransitionProperty;
 #[cfg(feature = "gecko")]
-pub use self::align::{AlignItems, ContentDistribution, SelfAlignment, JustifyItems};
+pub use self::align::{AlignItems, AlignContent, JustifyContent, SelfAlignment, JustifyItems};
 pub use self::angle::Angle;
 pub use self::background::{BackgroundSize, BackgroundRepeat};
 pub use self::border::{BorderImageSlice, BorderImageWidth, BorderImageSideWidth};
 pub use self::border::{BorderRadius, BorderCornerRadius, BorderSpacing};
 pub use self::font::{FontSize, FontSizeAdjust, FontSynthesis, FontWeight, FontVariantAlternates};
 pub use self::font::{FontFamily, FontLanguageOverride, FontVariantSettings, FontVariantEastAsian};
 pub use self::font::{FontVariantLigatures, FontVariantNumeric, FontFeatureSettings};
 pub use self::font::{MozScriptLevel, MozScriptMinSize, MozScriptSizeMultiplier, XTextZoom, XLang};
--- a/servo/components/style/values/specified/align.rs
+++ b/servo/components/style/values/specified/align.rs
@@ -105,40 +105,50 @@ impl ToCss for AlignFlags {
     }
 }
 
 /// Mask for a single AlignFlags value.
 const ALIGN_ALL_BITS: u16 = structs::NS_STYLE_ALIGN_ALL_BITS as u16;
 /// Number of bits to shift a fallback alignment.
 const ALIGN_ALL_SHIFT: u32 = structs::NS_STYLE_ALIGN_ALL_SHIFT;
 
-/// Value of the `align-content` or `justify-content` property.
-///
-/// <https://drafts.csswg.org/css-align/#content-distribution>
-#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToComputedValue)]
-#[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
-pub struct ContentDistribution {
-    primary: AlignFlags,
-    fallback: AlignFlags,
+/// An axis direction, either inline (for the `justify` properties) or block,
+/// (for the `align` properties).
+#[derive(Clone, Copy, PartialEq)]
+pub enum AxisDirection {
+    /// Block direction.
+    Block,
+    /// Inline direction.
+    Inline,
 }
 
 /// Whether fallback is allowed in align-content / justify-content parsing.
 ///
 /// This is used for the place-content shorthand, until the resolutions from [1]
 /// are specified.
 ///
 /// [1]: https://github.com/w3c/csswg-drafts/issues/1002
 #[derive(Clone, Copy, PartialEq)]
 pub enum FallbackAllowed {
     /// Allow fallback alignment.
     Yes,
     /// Don't allow fallback alignment.
     No,
 }
 
+/// Shared value for the `align-content` and `justify-content` properties.
+///
+/// <https://drafts.csswg.org/css-align/#content-distribution>
+#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToComputedValue)]
+#[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
+pub struct ContentDistribution {
+    primary: AlignFlags,
+    fallback: AlignFlags,
+}
+
 impl ContentDistribution {
     /// The initial value 'normal'
     #[inline]
     pub fn normal() -> Self {
         Self::new(AlignFlags::NORMAL)
     }
 
     /// Construct a value with no fallback.
@@ -150,16 +160,29 @@ impl ContentDistribution {
     /// Construct a value including a fallback alignment.
     ///
     /// <https://drafts.csswg.org/css-align/#fallback-alignment>
     #[inline]
     pub fn with_fallback(primary: AlignFlags, fallback: AlignFlags) -> Self {
         Self { primary, fallback }
     }
 
+    fn from_bits(bits: u16) -> Self {
+        let primary =
+            AlignFlags::from_bits_truncate((bits & ALIGN_ALL_BITS) as u8);
+        let fallback =
+            AlignFlags::from_bits_truncate((bits >> ALIGN_ALL_SHIFT) as u8);
+        Self::with_fallback(primary, fallback)
+    }
+
+    fn as_bits(&self) -> u16 {
+        self.primary.bits() as u16 |
+        ((self.fallback.bits() as u16) << ALIGN_ALL_SHIFT)
+    }
+
     /// The primary alignment
     #[inline]
     pub fn primary(self) -> AlignFlags {
         self.primary
     }
 
     /// The fallback alignment
     #[inline]
@@ -171,19 +194,20 @@ impl ContentDistribution {
     #[inline]
     pub fn has_extra_flags(self) -> bool {
         self.primary().intersects(AlignFlags::FLAG_BITS) ||
         self.fallback().intersects(AlignFlags::FLAG_BITS)
     }
 
     /// Parse a value for align-content / justify-content, optionally allowing
     /// fallback.
-    pub fn parse_with_fallback<'i, 't>(
+    pub fn parse<'i, 't>(
         input: &mut Parser<'i, 't>,
         fallback_allowed: FallbackAllowed,
+        _axis: AxisDirection,
     ) -> Result<Self, ParseError<'i>> {
         // normal | <baseline-position>
         if let Ok(value) = input.try(|input| parse_normal_or_baseline(input)) {
             return Ok(ContentDistribution::new(value))
         }
 
         // <content-distribution> followed by optional <*-position>
         if let Ok(value) = input.try(|input| parse_content_distribution(input)) {
@@ -219,22 +243,79 @@ impl ToCss for ContentDistribution {
                 dest.write_str(" ")?;
                 fallback.to_css(dest)?;
             }
         }
         Ok(())
     }
 }
 
+/// Value for the `align-content` property.
+///
+/// <https://drafts.csswg.org/css-align/#propdef-align-content>
+#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToComputedValue, ToCss)]
+pub struct AlignContent(pub ContentDistribution);
 
-impl Parse for ContentDistribution {
-    // normal | <baseline-position> |
-    // [ <content-distribution> || [ <overflow-position>? && <content-position> ] ]
-    fn parse<'i, 't>(_: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
-        Self::parse_with_fallback(input, FallbackAllowed::Yes)
+impl Parse for AlignContent {
+    fn parse<'i, 't>(
+        _: &ParserContext,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Self, ParseError<'i>> {
+        Ok(AlignContent(ContentDistribution::parse(
+            input,
+            FallbackAllowed::Yes,
+            AxisDirection::Block,
+        )?))
+    }
+}
+
+#[cfg(feature = "gecko")]
+impl From<u16> for AlignContent {
+    fn from(bits: u16) -> Self {
+        AlignContent(ContentDistribution::from_bits(bits))
+    }
+}
+
+#[cfg(feature = "gecko")]
+impl From<AlignContent> for u16 {
+    fn from(v: AlignContent) -> u16 {
+        v.0.as_bits()
+    }
+}
+
+/// Value for the `justify-content` property.
+///
+/// <https://drafts.csswg.org/css-align/#propdef-align-content>
+#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToComputedValue, ToCss)]
+pub struct JustifyContent(pub ContentDistribution);
+
+impl Parse for JustifyContent {
+    fn parse<'i, 't>(
+        _: &ParserContext,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Self, ParseError<'i>> {
+        Ok(JustifyContent(ContentDistribution::parse(
+            input,
+            FallbackAllowed::Yes,
+            AxisDirection::Inline,
+        )?))
+    }
+}
+
+#[cfg(feature = "gecko")]
+impl From<u16> for JustifyContent {
+    fn from(bits: u16) -> Self {
+        JustifyContent(ContentDistribution::from_bits(bits))
+    }
+}
+
+#[cfg(feature = "gecko")]
+impl From<JustifyContent> for u16 {
+    fn from(v: JustifyContent) -> u16 {
+        v.0.as_bits()
     }
 }
 
 /// Value of the `align-self` or `justify-self` property.
 ///
 /// <https://drafts.csswg.org/css-align/#self-alignment>
 #[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
 #[derive(Clone, Copy, Debug, Eq, PartialEq, ToComputedValue, ToCss)]
@@ -344,35 +425,16 @@ impl Parse for JustifyItems {
         if let Ok(value) = input.try(parse_legacy) {
             return Ok(JustifyItems(value))
         }
         // [ <overflow-position>? && <self-position> ]
         Ok(JustifyItems(parse_overflow_self_position(input)?))
     }
 }
 
-#[cfg(feature = "gecko")]
-impl From<u16> for ContentDistribution {
-    fn from(bits: u16) -> ContentDistribution {
-        let primary =
-            AlignFlags::from_bits_truncate((bits & ALIGN_ALL_BITS) as u8);
-        let fallback =
-            AlignFlags::from_bits_truncate((bits >> ALIGN_ALL_SHIFT) as u8);
-        ContentDistribution::with_fallback(primary, fallback)
-    }
-}
-
-#[cfg(feature = "gecko")]
-impl From<ContentDistribution> for u16 {
-    fn from(v: ContentDistribution) -> u16 {
-        v.primary().bits() as u16 |
-        ((v.fallback().bits() as u16) << ALIGN_ALL_SHIFT)
-    }
-}
-
 // auto | normal | stretch | <baseline-position>
 fn parse_auto_normal_stretch_baseline<'i, 't>(
     input: &mut Parser<'i, 't>,
 ) -> Result<AlignFlags, ParseError<'i>> {
     if let Ok(baseline) = input.try(parse_baseline) {
         return Ok(baseline);
     }
 
--- a/servo/components/style/values/specified/mod.rs
+++ b/servo/components/style/values/specified/mod.rs
@@ -21,17 +21,17 @@ use super::computed::{Context, ToCompute
 use super::generics::{GreaterThanOrEqualToOne, NonNegative};
 use super::generics::grid::{GridLine as GenericGridLine, TrackBreadth as GenericTrackBreadth};
 use super::generics::grid::{TrackSize as GenericTrackSize, TrackList as GenericTrackList};
 use values::specified::calc::CalcNode;
 
 pub use properties::animated_properties::TransitionProperty;
 pub use self::angle::Angle;
 #[cfg(feature = "gecko")]
-pub use self::align::{AlignItems, ContentDistribution, SelfAlignment, JustifyItems};
+pub use self::align::{AlignContent, JustifyContent, AlignItems, ContentDistribution, SelfAlignment, JustifyItems};
 pub use self::background::{BackgroundRepeat, BackgroundSize};
 pub use self::border::{BorderCornerRadius, BorderImageSlice, BorderImageWidth};
 pub use self::border::{BorderImageSideWidth, BorderRadius, BorderSideWidth, BorderSpacing};
 pub use self::font::{FontSize, FontSizeAdjust, FontSynthesis, FontWeight, FontVariantAlternates};
 pub use self::font::{FontFamily, FontLanguageOverride, FontVariantSettings, FontVariantEastAsian};
 pub use self::font::{FontVariantLigatures, FontVariantNumeric, FontFeatureSettings};
 pub use self::font::{MozScriptLevel, MozScriptMinSize, MozScriptSizeMultiplier, XTextZoom, XLang};
 pub use self::box_::{AnimationIterationCount, AnimationName, Display, OverscrollBehavior, Contain};