Auto merge of #20571 - emilio:align-shorthands-again, r=mats

style: Don't restrict alignment shorthand parsing now that's not ambiguous.

Bug: 1339656
Reviewed-by: mats
MozReview-Commit-ID: AnYBt6zwnPl

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20571)
<!-- Reviewable:end -->

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

@@ -52,15 +52,19 @@
     use values::specified::NonNegativeNumber;
     use properties::longhands::flex_basis::SpecifiedValue as FlexBasis;
 
-    fn parse_flexibility<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
+    fn parse_flexibility<'i, 't>(
-                                 -> Result<(NonNegativeNumber, Option<NonNegativeNumber>),ParseError<'i>> {
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<(NonNegativeNumber, Option<NonNegativeNumber>),ParseError<'i>> {
         let grow = NonNegativeNumber::parse(context, input)?;
         let shrink = input.try(|i| NonNegativeNumber::parse(context, i)).ok();
         Ok((grow, shrink))
     }
 
-    pub fn parse_value<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
+    pub fn parse_value<'i, 't>(
-                               -> Result<Longhands, ParseError<'i>> {
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Longhands, ParseError<'i>> {
         let mut grow = None;
         let mut shrink = None;
         let mut basis = None;
@@ -144,8 +148,10 @@
     // NOTE: Since both the shorthands have the same code, we should (re-)use code from one to implement
     // the other. This might not be a big deal for now, but we should consider looking into this in the future
     // to limit the amount of code generated.
-    pub fn parse_value<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
+    pub fn parse_value<'i, 't>(
-                               -> Result<Longhands, ParseError<'i>> {
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Longhands, ParseError<'i>> {
         let start = input.try(|i| GridLine::parse(context, i))?;
         let end = if input.try(|i| i.expect_delim('/')).is_ok() {
             GridLine::parse(context, input)?
@@ -182,8 +188,10 @@
     use parser::Parse;
 
     // The code is the same as `grid-{row,column}` except that this can have four values at most.
-    pub fn parse_value<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
+    pub fn parse_value<'i, 't>(
-                               -> Result<Longhands, ParseError<'i>> {
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Longhands, ParseError<'i>> {
         fn line_with_ident_from(other: &GridLine) -> GridLine {
             let mut this = GridLine::auto();
             if other.line_num.is_none() && !other.is_span {
@@ -253,10 +261,10 @@
     use values::specified::position::{TemplateAreas, TemplateAreasArc};
 
     /// Parsing for `<grid-template>` shorthand (also used by `grid` shorthand).
-    pub fn parse_grid_template<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
+    pub fn parse_grid_template<'i, 't>(
-                                       -> Result<(GridTemplateComponent,
+        context: &ParserContext,
-                                                  GridTemplateComponent,
+        input: &mut Parser<'i, 't>,
-                                                  Either<TemplateAreasArc, None_>), ParseError<'i>> {
+    ) -> Result<(GridTemplateComponent, GridTemplateComponent, Either<TemplateAreasArc, None_>), ParseError<'i>> {
         // Other shorthand sub properties also parse `none` and `subgrid` keywords and this
         // shorthand should know after these keywords there is nothing to parse. Otherwise it
         // gets confused and rejects the sub properties that contains `none` or `subgrid`.
@@ -354,8 +362,10 @@
     }
 
     #[inline]
-    pub fn parse_value<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
+    pub fn parse_value<'i, 't>(
-                               -> Result<Longhands, ParseError<'i>> {
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Longhands, ParseError<'i>> {
         let (rows, columns, areas) = parse_grid_template(context, input)?;
         Ok(expanded! {
             grid_template_rows: rows,
@@ -481,8 +491,10 @@
     use values::specified::{GenericGridTemplateComponent, TrackSize};
     use values::specified::position::{AutoFlow, GridAutoFlow};
 
-    pub fn parse_value<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
+    pub fn parse_value<'i, 't>(
-                               -> Result<Longhands, ParseError<'i>> {
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Longhands, ParseError<'i>> {
         let mut temp_rows = GridTemplateComponent::None;
         let mut temp_cols = GridTemplateComponent::None;
         let mut temp_areas = Either::Second(None_);
@@ -490,8 +502,10 @@
         let mut auto_cols = TrackSize::default();
         let mut flow = grid_auto_flow::get_initial_value();
 
-        fn parse_auto_flow<'i, 't>(input: &mut Parser<'i, 't>, is_row: bool)
+        fn parse_auto_flow<'i, 't>(
-                                   -> Result<GridAutoFlow, ParseError<'i>> {
+            input: &mut Parser<'i, 't>,
+            is_row: bool,
+        ) -> Result<GridAutoFlow, ParseError<'i>> {
             let mut auto_flow = None;
             let mut dense = false;
             for _ in 0..2 {
@@ -637,19 +651,22 @@
 
         let justify_content = match justify_content {
             Ok(v) => v,
-            Err(err) => {
+            Err(..) => {
-                if !align_content.is_valid_on_both_axes() {
+                // https://drafts.csswg.org/css-align-3/#place-content:
-                    return Err(err);
+                //
+                //   The second value is assigned to justify-content; if
+                //   omitted, it is copied from the first value, unless that
+                //   value is a <baseline-position> in which case it is
+                //   defaulted to start.
+                //
+                if !align_content.is_baseline_position() {
+                    align_content
+                } else {
+                    ContentDistribution::start()
                 }
-
-                align_content
             }
         };
 
-        if align_content.has_extra_flags() || justify_content.has_extra_flags() {
-            return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
-        }
-
         Ok(expanded! {
             align_content: AlignContent(align_content),
             justify_content: JustifyContent(justify_content),
@@ -682,18 +699,12 @@
 
         let justify = match justify {
             Ok(v) => v,
-            Err(err) => {
+            Err(..) => {
-                if !align.is_valid_on_both_axes() {
+                debug_assert!(align.is_valid_on_both_axes());
-                    return Err(err);
-                }
                 align
             }
         };
 
-        if justify.has_extra_flags() || align.has_extra_flags() {
-            return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
-        }
-
         Ok(expanded! {
             align_self: AlignSelf(align),
             justify_self: JustifySelf(justify),
@@ -729,14 +740,9 @@
         input: &mut Parser<'i, 't>,
     ) -> Result<Longhands, ParseError<'i>> {
         let align = AlignItems::parse(context, input)?;
-        if align.has_extra_flags() {
+        let justify =
-            return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
+            input.try(|input| JustifyItems::parse(context, input))
-        }
+                 .unwrap_or_else(|_| JustifyItems::from(align));
-        let justify = input.try(|input| JustifyItems::parse(context, input))
-                           .unwrap_or(JustifyItems::from(align));
-        if justify.has_extra_flags() {
-            return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
-        }
 
         Ok(expanded! {
             align_items: align,
@@ -746,11 +752,6 @@
 
     impl<'a> ToCss for LonghandsToSerialize<'a> {
         fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
-            if self.align_items.has_extra_flags() ||
-               self.justify_items.has_extra_flags() {
-                return Ok(());
-            }
-
             self.align_items.to_css(dest)?;
             if self.align_items.0 != self.justify_items.0 {
                 dest.write_str(" ")?;

components/style/values/specified/align.rs

@@ -139,6 +139,12 @@ impl ContentDistribution {
         Self::new(AlignFlags::NORMAL)
     }
 
+    /// `start`
+    #[inline]
+    pub fn start() -> Self {
+        Self::new(AlignFlags::START)
+    }
+
     /// The initial value 'normal'
     #[inline]
     pub fn new(primary: AlignFlags) -> Self {
@@ -155,19 +161,12 @@ impl ContentDistribution {
         self.primary.bits() as u16
     }
 
-    /// Returns whether this value is valid for both axis directions.
+    /// Returns whether this value is a <baseline-position>.
-    pub fn is_valid_on_both_axes(&self) -> bool {
+    pub fn is_baseline_position(&self) -> bool {
-        match self.primary.value() {
+        matches!(
-            // <baseline-position> is only allowed on the block axis.
+            self.primary.value(),
-            AlignFlags::BASELINE |
+            AlignFlags::BASELINE | AlignFlags::LAST_BASELINE
-            AlignFlags::LAST_BASELINE => false,
+        )
-
-            // left | right are only allowed on the inline axis.
-            AlignFlags::LEFT |
-            AlignFlags::RIGHT => false,
-
-            _ => true,
-        }
     }
 
     /// The primary alignment
@@ -176,12 +175,6 @@ impl ContentDistribution {
         self.primary
     }
 
-    /// Whether this value has extra flags.
-    #[inline]
-    pub fn has_extra_flags(self) -> bool {
-        self.primary().intersects(AlignFlags::FLAG_BITS)
-    }
-
     /// Parse a value for align-content / justify-content.
     pub fn parse<'i, 't>(
         input: &mut Parser<'i, 't>,
@@ -309,12 +302,6 @@ impl SelfAlignment {
         }
     }
 
-    /// Whether this value has extra flags.
-    #[inline]
-    pub fn has_extra_flags(self) -> bool {
-        self.0.intersects(AlignFlags::FLAG_BITS)
-    }
-
     /// Parse a self-alignment value on one of the axis.
     pub fn parse<'i, 't>(
         input: &mut Parser<'i, 't>,
@@ -402,12 +389,6 @@ impl AlignItems {
     pub fn normal() -> Self {
         AlignItems(AlignFlags::NORMAL)
     }
-
-    /// Whether this value has extra flags.
-    #[inline]
-    pub fn has_extra_flags(self) -> bool {
-        self.0.intersects(AlignFlags::FLAG_BITS)
-    }
 }
 
 
@@ -449,12 +430,6 @@ impl JustifyItems {
     pub fn normal() -> Self {
         JustifyItems(AlignFlags::NORMAL)
     }
-
-    /// Whether this value has extra flags.
-    #[inline]
-    pub fn has_extra_flags(self) -> bool {
-        self.0.intersects(AlignFlags::FLAG_BITS)
-    }
 }
 
 impl Parse for JustifyItems {