Bug 1635939 - Replace AspectRatio with computed::position::Ratio in media-queries. r=emilio

Also, we drop the pref, layout.css.aspect-ratio-number.enabled, becacuse
the spec of css-sizing-4 uses Number now.

Differential Revision: https://phabricator.services.mozilla.com/D75233

layout/style/test/test_css_parse_error_smoketest.html

@@ -105,7 +105,7 @@
       error: "Found invalid value for media feature.",
     }, {
       css: "@media (min-aspect-ratio: 1 invalid) {}",
-      error: "Found invalid value for media feature.",
+      error: "Unexpected token ‘invalid’ in media list.",
     }, {
       css: "@media (min-aspect-ratio: 1 / invalid) {}",
       error: "Found invalid value for media feature.",

layout/style/test/test_media_queries.html

@@ -512,28 +512,20 @@ function run() {
     expression_should_be_parseable(feature + ": 1  /1");
     expression_should_be_parseable(feature + ": 1  / \t\n1");
     expression_should_be_parseable(feature + ": 1/\r1");
-    if (SpecialPowers.getBoolPref('layout.css.aspect-ratio-number.enabled')) {
-      expression_should_be_parseable(feature + ": 1");
-      expression_should_be_parseable(feature + ": 0.5");
-      expression_should_be_parseable(feature + ": 1.0/1");
-      expression_should_be_parseable(feature + ": 1/1.0");
-      expression_should_be_parseable(feature + ": 1.0/1.0");
-      expression_should_be_parseable(feature + ": 1.5/1.2");
-      expression_should_be_parseable(feature + ": 1.5");
-      expression_should_be_parseable(feature + ": calc(1.2 * 1.3)");
-      expression_should_be_parseable(feature + ": 1.1/calc(2.2 * 2.3)");
-      expression_should_be_parseable(feature + ": calc(1.2 * 1.3)/2.2");
-      expression_should_be_parseable(feature + ": calc(1.2 * 1.3)/calc(2.2 * 2.3)");
-    } else {
-      expression_should_not_be_parseable(feature + ": 1");
-      expression_should_not_be_parseable(feature + ": 0.5");
-      expression_should_not_be_parseable(feature + ": 1.0/1");
-      expression_should_not_be_parseable(feature + ": 1/1.0");
-      expression_should_not_be_parseable(feature + ": 1.0/1.0");
-    }
-    expression_should_not_be_parseable(feature + ": 0/1");
-    expression_should_not_be_parseable(feature + ": 1/0");
-    expression_should_not_be_parseable(feature + ": 0/0");
+    expression_should_be_parseable(feature + ": 1");
+    expression_should_be_parseable(feature + ": 0.5");
+    expression_should_be_parseable(feature + ": 1.0/1");
+    expression_should_be_parseable(feature + ": 1/1.0");
+    expression_should_be_parseable(feature + ": 1.0/1.0");
+    expression_should_be_parseable(feature + ": 1.5/1.2");
+    expression_should_be_parseable(feature + ": 1.5");
+    expression_should_be_parseable(feature + ": calc(1.2 * 1.3)");
+    expression_should_be_parseable(feature + ": 1.1/calc(2.2 * 2.3)");
+    expression_should_be_parseable(feature + ": calc(1.2 * 1.3)/2.2");
+    expression_should_be_parseable(feature + ": calc(1.2 * 1.3)/calc(2.2 * 2.3)");
+    expression_should_be_parseable(feature + ": 0/1");
+    expression_should_be_parseable(feature + ": 1/0");
+    expression_should_be_parseable(feature + ": 0/0");
     expression_should_not_be_parseable(feature + ": -1/1");
     expression_should_not_be_parseable(feature + ": 1/-1");
     expression_should_not_be_parseable(feature + ": -1/-1");

modules/libpref/init/StaticPrefList.yaml

@@ -5503,14 +5503,6 @@
   mirror: always
   rust: true
 
-# Allow <number> and <number>/<number> both for <aspect-ratio>
-# https://github.com/w3c/csswg-drafts/issues/3757
-- name: layout.css.aspect-ratio-number.enabled
-  type: RelaxedAtomicBool
-  value: false
-  mirror: always
-  rust: true
-
 # Is the codepath for using cached scrollbar styles enabled?
 - name: layout.css.cached-scrollbar-styles.enabled
   type: bool

servo/components/style/gecko/media_features.rs

@@ -8,8 +8,9 @@ use crate::gecko_bindings::bindings;
 use crate::gecko_bindings::structs;
 use crate::media_queries::media_feature::{AllowsRanges, ParsingRequirements};
 use crate::media_queries::media_feature::{Evaluator, MediaFeatureDescription};
-use crate::media_queries::media_feature_expression::{AspectRatio, RangeOrOperator};
+use crate::media_queries::media_feature_expression::RangeOrOperator;
 use crate::media_queries::{Device, MediaType};
+use crate::values::computed::position::Ratio;
 use crate::values::computed::CSSPixelLength;
 use crate::values::computed::Resolution;
 use crate::Atom;
@@ -91,7 +92,7 @@ fn eval_device_height(
 
 fn eval_aspect_ratio_for<F>(
     device: &Device,
-    query_value: Option<AspectRatio>,
+    query_value: Option<Ratio>,
     range_or_operator: Option<RangeOrOperator>,
     get_size: F,
 ) -> bool
@@ -104,14 +105,14 @@ where
     };
 
     let size = get_size(device);
-    let value = AspectRatio(size.width.0 as f32, size.height.0 as f32);
+    let value = Ratio::new(size.width.0 as f32, size.height.0 as f32);
     RangeOrOperator::evaluate_with_query_value(range_or_operator, query_value, value)
 }
 
 /// https://drafts.csswg.org/mediaqueries-4/#aspect-ratio
 fn eval_aspect_ratio(
     device: &Device,
-    query_value: Option<AspectRatio>,
+    query_value: Option<Ratio>,
     range_or_operator: Option<RangeOrOperator>,
 ) -> bool {
     eval_aspect_ratio_for(device, query_value, range_or_operator, viewport_size)
@@ -120,7 +121,7 @@ fn eval_aspect_ratio(
 /// https://drafts.csswg.org/mediaqueries-4/#device-aspect-ratio
 fn eval_device_aspect_ratio(
     device: &Device,
-    query_value: Option<AspectRatio>,
+    query_value: Option<Ratio>,
     range_or_operator: Option<RangeOrOperator>,
 ) -> bool {
     eval_aspect_ratio_for(device, query_value, range_or_operator, device_size)

servo/components/style/media_queries/media_feature.rs

@@ -4,9 +4,10 @@
 
 //! Media features.
 
-use super::media_feature_expression::{AspectRatio, RangeOrOperator};
+use super::media_feature_expression::RangeOrOperator;
 use super::Device;
 use crate::parser::ParserContext;
+use crate::values::computed::position::Ratio;
 use crate::values::computed::{CSSPixelLength, Resolution};
 use crate::Atom;
 use cssparser::Parser;
@@ -45,7 +46,7 @@ pub enum Evaluator {
     Float(MediaFeatureEvaluator<f32>),
     BoolInteger(MediaFeatureEvaluator<bool>),
     /// A non-negative number ratio, such as the one from device-pixel-ratio.
-    NumberRatio(MediaFeatureEvaluator<AspectRatio>),
+    NumberRatio(MediaFeatureEvaluator<Ratio>),
     /// A resolution.
     Resolution(MediaFeatureEvaluator<Resolution>),
     /// A keyword value.

servo/components/style/media_queries/media_feature_expression.rs

@@ -15,9 +15,8 @@ use crate::parser::{Parse, ParserContext};
 #[cfg(feature = "servo")]
 use crate::servo::media_queries::MEDIA_FEATURES;
 use crate::str::{starts_with_ignore_ascii_case, string_as_ascii_lowercase};
+use crate::values::computed::position::Ratio;
 use crate::values::computed::{self, ToComputedValue};
-#[cfg(feature = "gecko")]
-use crate::values::specified::NonNegativeNumber;
 use crate::values::specified::{Integer, Length, Number, Resolution};
 use crate::values::{serialize_atom_identifier, CSSFloat};
 use crate::{Atom, Zero};
@@ -26,30 +25,6 @@ use std::cmp::{Ordering, PartialOrd};
 use std::fmt::{self, Write};
 use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
 
-/// An aspect ratio, with a numerator and denominator.
-#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, ToShmem)]
-pub struct AspectRatio(pub CSSFloat, pub CSSFloat);
-
-impl ToCss for AspectRatio {
-    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
-    where
-        W: fmt::Write,
-    {
-        self.0.to_css(dest)?;
-        dest.write_str(" / ")?;
-        self.1.to_css(dest)
-    }
-}
-
-impl PartialOrd for AspectRatio {
-    fn partial_cmp(&self, other: &AspectRatio) -> Option<Ordering> {
-        f64::partial_cmp(
-            &(self.0 as f64 * other.1 as f64),
-            &(self.1 as f64 * other.0 as f64),
-        )
-    }
-}
-
 /// The kind of matching that should be performed on a media feature value.
 #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToShmem)]
 pub enum Range {
@@ -460,7 +435,7 @@ pub enum MediaExpressionValue {
     BoolInteger(bool),
     /// A single non-negative number or two non-negative numbers separated by '/',
     /// with optional whitespace on either side of the '/'.
-    NumberRatio(AspectRatio),
+    NumberRatio(Ratio),
     /// A resolution.
     Resolution(Resolution),
     /// An enumerated value, defined by the variant keyword table in the
@@ -517,24 +492,14 @@ impl MediaExpressionValue {
                 MediaExpressionValue::Float(number.get())
             },
             Evaluator::NumberRatio(..) => {
-                #[cfg(feature = "gecko")]
-                {
-                    if static_prefs::pref!("layout.css.aspect-ratio-number.enabled") {
-                        let a = NonNegativeNumber::parse(context, input)?.0.get();
-                        let b = match input.try_parse(|input| input.expect_delim('/')) {
-                            Ok(()) => NonNegativeNumber::parse(context, input)?.0.get(),
-                            _ => 1.0,
-                        };
-                        return Ok(MediaExpressionValue::NumberRatio(AspectRatio(a, b)));
-                    }
-                }
+                use crate::values::generics::position::Ratio as GenericRatio;
+                use crate::values::generics::NonNegative;
+                use crate::values::specified::position::Ratio;
 
-                let a = Integer::parse_positive(context, input)?;
-                input.expect_delim('/')?;
-                let b = Integer::parse_positive(context, input)?;
-                MediaExpressionValue::NumberRatio(AspectRatio(
-                    a.value() as CSSFloat,
-                    b.value() as CSSFloat,
+                let ratio = Ratio::parse(context, input)?;
+                MediaExpressionValue::NumberRatio(GenericRatio(
+                    NonNegative(ratio.0.get()),
+                    NonNegative(ratio.1.get()),
                 ))
             },
             Evaluator::Resolution(..) => {

servo/components/style/values/computed/position.rs

@@ -12,9 +12,11 @@ use crate::values::generics::position::AspectRatio as GenericAspectRatio;
 use crate::values::generics::position::Position as GenericPosition;
 use crate::values::generics::position::PositionComponent as GenericPositionComponent;
 use crate::values::generics::position::PositionOrAuto as GenericPositionOrAuto;
+use crate::values::generics::position::Ratio as GenericRatio;
 use crate::values::generics::position::ZIndex as GenericZIndex;
 pub use crate::values::specified::position::{GridAutoFlow, GridTemplateAreas, MasonryAutoFlow};
 use crate::Zero;
+use std::cmp::{Ordering, PartialOrd};
 use std::fmt::{self, Write};
 use style_traits::{CssWriter, ToCss};
 
@@ -70,5 +72,24 @@ impl GenericPositionComponent for LengthPercentage {
 /// A computed value for the `z-index` property.
 pub type ZIndex = GenericZIndex<Integer>;
 
+/// A computed <ratio> value.
+pub type Ratio = GenericRatio<NonNegativeNumber>;
+
+impl PartialOrd for Ratio {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        f64::partial_cmp(
+            &((self.0).0 as f64 * (other.1).0 as f64),
+            &((self.1).0 as f64 * (other.0).0 as f64),
+        )
+    }
+}
+
+impl Ratio {
+    /// Returns a new Ratio.
+    pub fn new(a: f32, b: f32) -> Self {
+        GenericRatio(a.into(), b.into())
+    }
+}
+
 /// A computed value for the `aspect-ratio` property.
 pub type AspectRatio = GenericAspectRatio<NonNegativeNumber>;

servo/components/style/values/generics/position.rs

@@ -156,7 +156,6 @@ impl<Integer> ZIndex<Integer> {
 }
 
 /// A generic value for the `<ratio>` value.
-// FIXME: Use this for aspect-ratio in both css-sizing-4 and media-queries in the following patch.
 #[derive(
     Animate,
     Clone,

servo/components/style/values/specified/mod.rs

@@ -383,7 +383,7 @@ impl One for NonNegativeNumber {
 
     #[inline]
     fn is_one(&self) -> bool {
-        self.0.get() == 1.0
+        self.get() == 1.0
     }
 }
 
@@ -392,6 +392,12 @@ impl NonNegativeNumber {
     pub fn new(val: CSSFloat) -> Self {
         NonNegative::<Number>(Number::new(val.max(0.)))
     }
+
+    /// Returns the numeric value.
+    #[inline]
+    pub fn get(&self) -> f32 {
+        self.0.get()
+    }
 }
 
 /// A Number which is >= 1.0.

servo/components/style/values/specified/position.rs

@@ -899,8 +899,11 @@ impl Parse for AspectRatio {
     }
 }
 
+/// A specified value for the `aspect-ratio` property.
+pub type Ratio = GenericRatio<NonNegativeNumber>;
+
 // https://drafts.csswg.org/css-values-4/#ratios
-impl Parse for GenericRatio<NonNegativeNumber> {
+impl Parse for Ratio {
     fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,

testing/web-platform/meta/css/mediaqueries/aspect-ratio-005.html.ini

@@ -1,2 +0,0 @@
-[aspect-ratio-005.html]
-  prefs: [layout.css.aspect-ratio-number.enabled:true]

testing/web-platform/meta/css/mediaqueries/aspect-ratio-006.html.ini

@@ -1,2 +0,0 @@
-[aspect-ratio-006.html]
-  prefs: [layout.css.aspect-ratio-number.enabled:true]

testing/web-platform/tests/css/mediaqueries/aspect-ratio-002.html

@@ -6,7 +6,7 @@
 <link rel="help" title="4.6. aspect-ratio" href="http://www.w3.org/TR/css3-mediaqueries/#aspect-ratio">
 <link rel="match" href="../reference/ref-filled-green-100px-square.xht">
 <meta name="flags" content="">
-<meta name="assert" content="The 'aspect-ratio' property with prefix 'min' set '0/0' is invalid that means the style sheet specified by 'min-aspect-ratio' will not be applied.">
+<meta name="assert" content="The 'aspect-ratio' property with prefix 'min' set '0/0' (which is converted into '1/0') is infinite that means the style sheet specified by 'min-aspect-ratio' will not be applied.">
 <style>
   div {
     background-color: green;

testing/web-platform/tests/css/mediaqueries/aspect-ratio-004.html

@@ -6,16 +6,16 @@
 <link rel="help" title="4.6. aspect-ratio" href="http://www.w3.org/TR/css3-mediaqueries/#aspect-ratio">
 <link rel="match" href="../reference/ref-filled-green-100px-square.xht">
 <meta name="flags" content="">
-<meta name="assert" content="The 'aspect-ratio' property with prefix 'max' set '0/0' is invalid that means the style sheet specified by 'max-aspect-ratio' will not be applied.">
+<meta name="assert" content="The 'aspect-ratio' property with prefix 'max' set '0/0' (which is converted into '1/0') is infinite that means the style sheet specified by 'max-aspect-ratio' will be applied.">
 <style>
   div {
-    background-color: green;
+    background-color: red;
     height: 100px;
     width: 100px;
   }
   @media screen and (max-aspect-ratio: 0/0) {
     div {
-      background-color: red;
+      background-color: green;
     }
   }
 </style>

testing/web-platform/tests/css/mediaqueries/device-aspect-ratio-002.html

@@ -6,16 +6,16 @@
 <link rel="help" title="4.7. device-aspect-ratio" href="http://www.w3.org/TR/css3-mediaqueries/#device-aspect-ratio">
 <link rel="match" href="../reference/ref-filled-green-100px-square.xht">
 <meta name="flags" content="">
-<meta name="assert" content="The 'device-aspect-ratio' property with prefix 'max' set '0/0' is invalid that means the style sheet specified by 'max-device-aspect-ratio' will not be applied.">
+<meta name="assert" content="The 'device-aspect-ratio' property with prefix 'max' set '0/0' (which is converted into '1/0') is infinite that means the style sheet specified by 'max-device-aspect-ratio' will be applied.">
 <style>
   div {
-    background-color: green;
+    background-color: red;
     height: 100px;
     width: 100px;
   }
   @media screen and (max-device-aspect-ratio: 0/0) {
     div {
-      background-color: red;
+      background-color: green;
     }
   }
 </style>

testing/web-platform/tests/css/mediaqueries/device-aspect-ratio-004.html

@@ -6,7 +6,7 @@
 <link rel="help" title="4.7. device-aspect-ratio" href="http://www.w3.org/TR/css3-mediaqueries/#device-aspect-ratio">
 <link rel="match" href="../reference/ref-filled-green-100px-square.xht">
 <meta name="flags" content="">
-<meta name="assert" content="The 'device-aspect-ratio' property with prefix 'min' set '0/0' is invalid that means the style sheet specified by 'min-device-aspect-ratio' will not be applied.">
+<meta name="assert" content="The 'device-aspect-ratio' property with prefix 'min' set '0/0' (which is converted into '1/0') is infinite that means the style sheet specified by 'min-device-aspect-ratio' will not be applied.">
 <style>
   div {
     background-color: green;

testing/web-platform/tests/css/mediaqueries/test_media_queries.html

@@ -307,14 +307,14 @@ function run() {
       expression_should_be_parseable(feature + ": 1  /1");
       expression_should_be_parseable(feature + ": 1  / \t\n1");
       expression_should_be_parseable(feature + ": 1/\r1");
-      expression_should_not_be_parseable(feature + ": 1");
-      expression_should_not_be_parseable(feature + ": 0.5");
-      expression_should_not_be_parseable(feature + ": 1.0/1");
-      expression_should_not_be_parseable(feature + ": 1/1.0");
-      expression_should_not_be_parseable(feature + ": 1.0/1.0");
-      expression_should_not_be_parseable(feature + ": 0/1");
-      expression_should_not_be_parseable(feature + ": 1/0");
-      expression_should_not_be_parseable(feature + ": 0/0");
+      expression_should_be_parseable(feature + ": 1");
+      expression_should_be_parseable(feature + ": 0.5");
+      expression_should_be_parseable(feature + ": 1.0/1");
+      expression_should_be_parseable(feature + ": 1/1.0");
+      expression_should_be_parseable(feature + ": 1.0/1.0");
+      expression_should_be_parseable(feature + ": 0/1");
+      expression_should_be_parseable(feature + ": 1/0");
+      expression_should_be_parseable(feature + ": 0/0");
       expression_should_not_be_parseable(feature + ": -1/1");
       expression_should_not_be_parseable(feature + ": 1/-1");
       expression_should_not_be_parseable(feature + ": -1/-1");