From 81ae588ec94bd7112aebe623873448966a51ffd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 3 Nov 2022 14:07:02 +0000 Subject: [PATCH] style: Fix container query evaluation on unsupported axis We were falling back to viewport size, which is not what the spec says. Differential Revision: https://phabricator.services.mozilla.com/D161132 --- components/style/queries/feature.rs | 2 + .../style/queries/feature_expression.rs | 21 ++++++- .../style/stylesheets/container_rule.rs | 59 ++++++++----------- 3 files changed, 47 insertions(+), 35 deletions(-) diff --git a/components/style/queries/feature.rs b/components/style/queries/feature.rs index 19340f8e240..23a12713baa 100644 --- a/components/style/queries/feature.rs +++ b/components/style/queries/feature.rs @@ -34,11 +34,13 @@ pub type KeywordParser = for<'a, 'i, 't> fn( #[allow(missing_docs)] pub enum Evaluator { Length(QueryFeatureGetter), + OptionalLength(QueryFeatureGetter>), Integer(QueryFeatureGetter), Float(QueryFeatureGetter), BoolInteger(QueryFeatureGetter), /// A non-negative number ratio, such as the one from device-pixel-ratio. NumberRatio(QueryFeatureGetter), + OptionalNumberRatio(QueryFeatureGetter>), /// A resolution. Resolution(QueryFeatureGetter), /// A keyword value. diff --git a/components/style/queries/feature_expression.rs b/components/style/queries/feature_expression.rs index e798986cd2f..8bea95cb2e0 100644 --- a/components/style/queries/feature_expression.rs +++ b/components/style/queries/feature_expression.rs @@ -591,6 +591,14 @@ impl QueryFeatureExpression { self.kind .evaluate(v, |v| expect!(Length, v).to_computed_value(context)) }, + Evaluator::OptionalLength(eval) => { + let v = match eval(context) { + Some(v) => v, + None => return false, + }; + self.kind + .evaluate(v, |v| expect!(Length, v).to_computed_value(context)) + }, Evaluator::Integer(eval) => { let v = eval(context); self.kind.evaluate(v, |v| *expect!(Integer, v)) @@ -608,6 +616,15 @@ impl QueryFeatureExpression { self.kind .evaluate(ratio, |v| expect!(NumberRatio, v).used_value()) }, + Evaluator::OptionalNumberRatio(eval) => { + let ratio = match eval(context) { + Some(v) => v, + None => return false, + }; + // See above for subtleties here. + self.kind + .evaluate(ratio, |v| expect!(NumberRatio, v).used_value()) + }, Evaluator::Resolution(eval) => { let v = eval(context).dppx(); self.kind.evaluate(v, |v| { @@ -686,7 +703,7 @@ impl QueryExpressionValue { input: &mut Parser<'i, 't>, ) -> Result> { Ok(match for_feature.evaluator { - Evaluator::Length(..) => { + Evaluator::OptionalLength(..) | Evaluator::Length(..) => { let length = Length::parse_non_negative(context, input)?; QueryExpressionValue::Length(length) }, @@ -706,7 +723,7 @@ impl QueryExpressionValue { let number = Number::parse(context, input)?; QueryExpressionValue::Float(number.get()) }, - Evaluator::NumberRatio(..) => { + Evaluator::OptionalNumberRatio(..) | Evaluator::NumberRatio(..) => { use crate::values::specified::Ratio as SpecifiedRatio; let ratio = SpecifiedRatio::parse(context, input)?; QueryExpressionValue::NumberRatio(Ratio::new(ratio.0.get(), ratio.1.get())) diff --git a/components/style/stylesheets/container_rule.rs b/components/style/stylesheets/container_rule.rs index 381025e5e0f..5df810b9be4 100644 --- a/components/style/stylesheets/container_rule.rs +++ b/components/style/stylesheets/container_rule.rs @@ -261,51 +261,44 @@ pub struct ContainerInfo { wm: WritingMode, } -fn get_container(context: &Context) -> ContainerInfo { - if let Some(ref info) = context.container_info { - return info.clone(); - } - ContainerInfo { - size: context.device().au_viewport_size(), - wm: WritingMode::horizontal_tb(), - } +fn eval_width(context: &Context) -> Option { + let info = context.container_info.as_ref()?; + Some(CSSPixelLength::new(info.size.width.to_f32_px())) } -fn eval_width(context: &Context) -> CSSPixelLength { - let info = get_container(context); - CSSPixelLength::new(info.size.width.to_f32_px()) +fn eval_height(context: &Context) -> Option { + let info = context.container_info.as_ref()?; + Some(CSSPixelLength::new(info.size.height.to_f32_px())) } -fn eval_height(context: &Context) -> CSSPixelLength { - let info = get_container(context); - CSSPixelLength::new(info.size.height.to_f32_px()) -} - -fn eval_inline_size(context: &Context) -> CSSPixelLength { - let info = get_container(context); - CSSPixelLength::new( +fn eval_inline_size(context: &Context) -> Option { + let info = context.container_info.as_ref()?; + Some(CSSPixelLength::new( LogicalSize::from_physical(info.wm, info.size) .inline .to_f32_px(), - ) + )) } -fn eval_block_size(context: &Context) -> CSSPixelLength { - let info = get_container(context); - CSSPixelLength::new( +fn eval_block_size(context: &Context) -> Option { + let info = context.container_info.as_ref()?; + Some(CSSPixelLength::new( LogicalSize::from_physical(info.wm, info.size) .block .to_f32_px(), - ) + )) } -fn eval_aspect_ratio(context: &Context) -> Ratio { - let info = get_container(context); - Ratio::new(info.size.width.0 as f32, info.size.height.0 as f32) +fn eval_aspect_ratio(context: &Context) -> Option { + let info = context.container_info.as_ref()?; + Some(Ratio::new(info.size.width.0 as f32, info.size.height.0 as f32)) } fn eval_orientation(context: &Context, value: Option) -> bool { - let info = get_container(context); + let info = match context.container_info.as_ref() { + Some(info) => info, + None => return false, + }; Orientation::eval(info.size, value) } @@ -316,31 +309,31 @@ pub static CONTAINER_FEATURES: [QueryFeatureDescription; 6] = [ feature!( atom!("width"), AllowsRanges::Yes, - Evaluator::Length(eval_width), + Evaluator::OptionalLength(eval_width), FeatureFlags::CONTAINER_REQUIRES_WIDTH_AXIS, ), feature!( atom!("height"), AllowsRanges::Yes, - Evaluator::Length(eval_height), + Evaluator::OptionalLength(eval_height), FeatureFlags::CONTAINER_REQUIRES_HEIGHT_AXIS, ), feature!( atom!("inline-size"), AllowsRanges::Yes, - Evaluator::Length(eval_inline_size), + Evaluator::OptionalLength(eval_inline_size), FeatureFlags::CONTAINER_REQUIRES_INLINE_AXIS, ), feature!( atom!("block-size"), AllowsRanges::Yes, - Evaluator::Length(eval_block_size), + Evaluator::OptionalLength(eval_block_size), FeatureFlags::CONTAINER_REQUIRES_BLOCK_AXIS, ), feature!( atom!("aspect-ratio"), AllowsRanges::Yes, - Evaluator::NumberRatio(eval_aspect_ratio), + Evaluator::OptionalNumberRatio(eval_aspect_ratio), // XXX from_bits_truncate is const, but the pipe operator isn't, so this // works around it. FeatureFlags::from_bits_truncate(