From fd47a93b9c3330b6e5f1385bc2342a277f58041d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 24 Jan 2018 13:23:19 +0100 Subject: [PATCH] style: Restrict and depending on the axis in content distribution properties. This fixes https://bugzilla.mozilla.org/show_bug.cgi?id=1430817, and updates us to the current version of the css-align spec. MozReview-Commit-ID: LtBcdqYJeK --- .../properties/shorthand/position.mako.rs | 16 +++- components/style/values/specified/align.rs | 74 +++++++++++-------- 2 files changed, 57 insertions(+), 33 deletions(-) diff --git a/components/style/properties/shorthand/position.mako.rs b/components/style/properties/shorthand/position.mako.rs index 06549a71891..bb6af5d29fe 100644 --- a/components/style/properties/shorthand/position.mako.rs +++ b/components/style/properties/shorthand/position.mako.rs @@ -634,11 +634,19 @@ FallbackAllowed::No, AxisDirection::Block, ) - }).unwrap_or(align_content); + }); + + let justify_content = match justify_content { + Ok(v) => v, + Err(err) => { + if !align_content.is_valid_on_both_axes() { + return Err(err); + } + + 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)); } diff --git a/components/style/values/specified/align.rs b/components/style/values/specified/align.rs index e0faf23f864..03957399b7e 100644 --- a/components/style/values/specified/align.rs +++ b/components/style/values/specified/align.rs @@ -178,6 +178,23 @@ impl ContentDistribution { ((self.fallback.bits() as u16) << ALIGN_ALL_SHIFT) } + /// Returns whether this value is valid for both axis directions. + pub fn is_valid_on_both_axes(&self) -> bool { + if self.primary.intersects(AlignFlags::BASELINE | AlignFlags::LAST_BASELINE) || + self.fallback.intersects(AlignFlags::BASELINE | AlignFlags::LAST_BASELINE) { + // is only allowed on the block axis. + return false; + } + + if self.primary.intersects(AlignFlags::LEFT | AlignFlags::RIGHT) || + self.fallback.intersects(AlignFlags::LEFT | AlignFlags::RIGHT) { + // left | right are only allowed on the inline axis. + return false; + } + + true + } + /// The primary alignment #[inline] pub fn primary(self) -> AlignFlags { @@ -202,17 +219,24 @@ impl ContentDistribution { pub fn parse<'i, 't>( input: &mut Parser<'i, 't>, fallback_allowed: FallbackAllowed, - _axis: AxisDirection, + axis: AxisDirection, ) -> Result> { - // normal | - if let Ok(value) = input.try(|input| parse_normal_or_baseline(input)) { - return Ok(ContentDistribution::new(value)) + // Try to parse normal first + if input.try(|i| i.expect_ident_matching("normal")).is_ok() { + return Ok(ContentDistribution::normal()); + } + + // Parse , but only on the block axis. + if axis == AxisDirection::Block { + if let Ok(value) = input.try(parse_baseline) { + return Ok(ContentDistribution::new(value)); + } } // followed by optional <*-position> if let Ok(value) = input.try(|input| parse_content_distribution(input)) { if fallback_allowed == FallbackAllowed::Yes { - if let Ok(fallback) = input.try(|input| parse_overflow_content_position(input)) { + if let Ok(fallback) = input.try(|input| parse_overflow_content_position(input, axis)) { return Ok(ContentDistribution::with_fallback(value, fallback)) } } @@ -220,7 +244,7 @@ impl ContentDistribution { } // <*-position> followed by optional - let fallback = parse_overflow_content_position(input)?; + let fallback = parse_overflow_content_position(input, axis)?; if fallback_allowed == FallbackAllowed::Yes { if let Ok(value) = input.try(|input| parse_content_distribution(input)) { return Ok(ContentDistribution::with_fallback(value, fallback)) @@ -335,7 +359,6 @@ impl SelfAlignment { } } - impl Parse for SelfAlignment { // auto | normal | stretch | | // [ ? && ] @@ -457,19 +480,8 @@ fn parse_normal_stretch_baseline<'i, 't>(input: &mut Parser<'i, 't>) -> Result -fn parse_normal_or_baseline<'i, 't>(input: &mut Parser<'i, 't>) -> Result> { - if let Ok(baseline) = input.try(parse_baseline) { - return Ok(baseline); - } - - input.expect_ident_matching("normal")?; - Ok(AlignFlags::NORMAL) -} - // fn parse_baseline<'i, 't>(input: &mut Parser<'i, 't>) -> Result> { - // FIXME: remove clone() when lifetimes are non-lexical try_match_ident_ignore_ascii_case! { input, "baseline" => Ok(AlignFlags::BASELINE), "first" => { @@ -494,33 +506,37 @@ fn parse_content_distribution<'i, 't>(input: &mut Parser<'i, 't>) -> Result? && ] -fn parse_overflow_content_position<'i, 't>(input: &mut Parser<'i, 't>) -> Result> { +fn parse_overflow_content_position<'i, 't>( + input: &mut Parser<'i, 't>, + axis: AxisDirection, +) -> Result> { // followed by optional - if let Ok(mut content) = input.try(parse_content_position) { + if let Ok(mut content) = input.try(|input| parse_content_position(input, axis)) { if let Ok(overflow) = input.try(parse_overflow_position) { content |= overflow; } return Ok(content) } + // followed by required - if let Ok(overflow) = parse_overflow_position(input) { - if let Ok(content) = parse_content_position(input) { - return Ok(overflow | content) - } - } - return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) + let overflow = parse_overflow_position(input)?; + let content = parse_content_position(input, axis)?; + Ok(overflow | content) } // -fn parse_content_position<'i, 't>(input: &mut Parser<'i, 't>) -> Result> { +fn parse_content_position<'i, 't>( + input: &mut Parser<'i, 't>, + axis: AxisDirection, +) -> Result> { try_match_ident_ignore_ascii_case! { input, "start" => Ok(AlignFlags::START), "end" => Ok(AlignFlags::END), "flex-start" => Ok(AlignFlags::FLEX_START), "flex-end" => Ok(AlignFlags::FLEX_END), "center" => Ok(AlignFlags::CENTER), - "left" => Ok(AlignFlags::LEFT), - "right" => Ok(AlignFlags::RIGHT), + "left" if axis == AxisDirection::Inline => Ok(AlignFlags::LEFT), + "right" if axis == AxisDirection::Inline => Ok(AlignFlags::RIGHT), } }