style: Allow parsing operators in media feature expressions.

The only bit from the spec which I haven't implemented to my knowledge is the
bit that allows you to swap the position of the media feature and the value,
because it unnecessarily complicates parsing (we parse the value in terms of the
feature), and I don't think it's useful given how easy it is to switch from,
e.g., `(500px > width)` to `(width <= 500px)`.

I filed https://github.com/w3c/csswg-drafts/issues/2791 about it.

Bug: 1422225
Reviewed-by: xidorn
MozReview-Commit-ID: 6xrdVl87S9X

components/style/gecko/media_queries.rs

@@ -7,7 +7,7 @@
 use app_units::AU_PER_PX;
 use app_units::Au;
 use context::QuirksMode;
-use cssparser::{BasicParseErrorKind, Parser, RGBA};
+use cssparser::{BasicParseErrorKind, Parser, RGBA, Token};
 use euclid::Size2D;
 use euclid::TypedScale;
 use gecko::values::{convert_nscolor_to_rgba, convert_rgba_to_nscolor};
@@ -255,8 +255,41 @@ pub enum Range {
     Min,
     /// At most the specified value.
     Max,
-    /// Exactly the specified value.
+}
+
+/// The operator that was specified in this media feature.
+#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq)]
+enum Operator {
     Equal,
+    GreaterThan,
+    GreaterThanEqual,
+    LessThan,
+    LessThanEqual,
+}
+
+impl ToCss for Operator {
+    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
+    where
+        W: fmt::Write,
+    {
+        dest.write_str(match *self {
+            Operator::Equal => "=",
+            Operator::LessThan => "<",
+            Operator::LessThanEqual => "<=",
+            Operator::GreaterThan => ">",
+            Operator::GreaterThanEqual => ">=",
+        })
+    }
+}
+
+/// Either a `Range` or an `Operator`.
+///
+/// Ranged media features are not allowed with operations (that'd make no
+/// sense).
+#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq)]
+enum RangeOrOperator {
+    Range(Range),
+    Operator(Operator),
 }
 
 /// A expression for gecko contains a reference to the media feature, the value
@@ -265,7 +298,7 @@ pub enum Range {
 pub struct Expression {
     feature: &'static nsMediaFeature,
     value: Option<MediaExpressionValue>,
-    range: Range,
+    range_or_operator: Option<RangeOrOperator>,
 }
 
 impl ToCss for Expression {
@@ -279,10 +312,12 @@ impl ToCss for Expression {
         {
             dest.write_str("-webkit-")?;
         }
-        match self.range {
-            Range::Min => dest.write_str("min-")?,
-            Range::Max => dest.write_str("max-")?,
-            Range::Equal => {},
+
+        if let Some(RangeOrOperator::Range(range)) = self.range_or_operator {
+            match range {
+                Range::Min => dest.write_str("min-")?,
+                Range::Max => dest.write_str("max-")?,
+            }
         }
 
         // NB: CssStringWriter not needed, feature names are under control.
@@ -290,8 +325,15 @@ impl ToCss for Expression {
             Atom::from_static(*self.feature.mName)
         })?;
 
-        if let Some(ref val) = self.value {
+        if let Some(RangeOrOperator::Operator(op)) = self.range_or_operator {
+            dest.write_char(' ')?;
+            op.to_css(dest)?;
+            dest.write_char(' ')?;
+        } else if self.value.is_some() {
             dest.write_str(": ")?;
+        }
+
+        if let Some(ref val) = self.value {
             val.to_css(dest, self)?;
         }
 
@@ -302,7 +344,7 @@ impl ToCss for Expression {
 impl PartialEq for Expression {
     fn eq(&self, other: &Expression) -> bool {
         self.feature.mName == other.feature.mName && self.value == other.value &&
-            self.range == other.range
+            self.range_or_operator == other.range_or_operator
     }
 }
 
@@ -537,17 +579,53 @@ fn parse_feature_value<'i, 't>(
     Ok(value)
 }
 
+/// Consumes an operation or a colon, or returns an error.
+fn consume_operation_or_colon(
+    input: &mut Parser,
+) -> Result<Option<Operator>, ()> {
+    let first_delim = {
+        let next_token = match input.next() {
+            Ok(t) => t,
+            Err(..) => return Err(()),
+        };
+
+        match *next_token {
+            Token::Colon => return Ok(None),
+            Token::Delim(oper) => oper,
+            _ => return Err(()),
+        }
+    };
+    Ok(Some(match first_delim {
+        '=' => Operator::Equal,
+        '>' => {
+            if input.try(|i| i.expect_delim('=')).is_ok() {
+                Operator::GreaterThanEqual
+            } else {
+                Operator::GreaterThan
+            }
+        }
+        '<' => {
+            if input.try(|i| i.expect_delim('=')).is_ok() {
+                Operator::LessThanEqual
+            } else {
+                Operator::LessThan
+            }
+        }
+        _ => return Err(()),
+    }))
+}
+
 impl Expression {
     /// Trivially construct a new expression.
     fn new(
         feature: &'static nsMediaFeature,
         value: Option<MediaExpressionValue>,
-        range: Range,
+        range_or_operator: Option<RangeOrOperator>,
     ) -> Self {
         Self {
             feature,
             value,
-            range,
+            range_or_operator,
         }
     }
 
@@ -608,12 +686,12 @@ impl Expression {
 
                     let range = if starts_with_ignore_ascii_case(feature_name, "min-") {
                         feature_name = &feature_name[4..];
-                        Range::Min
+                        Some(Range::Min)
                     } else if starts_with_ignore_ascii_case(feature_name, "max-") {
                         feature_name = &feature_name[4..];
-                        Range::Max
+                        Some(Range::Max)
                     } else {
-                        Range::Equal
+                        None
                     };
 
                     let atom = Atom::from(string_as_ascii_lowercase(feature_name));
@@ -641,7 +719,7 @@ impl Expression {
                     ));
                 }
 
-                if range != Range::Equal &&
+                if range.is_some() &&
                     feature.mRangeType != nsMediaFeature_RangeType::eMinMaxAllowed
                 {
                     return Err(location.new_custom_error(
@@ -650,17 +728,56 @@ impl Expression {
                 }
             }
 
-            // If there's no colon, this is a media query of the form
-            // '(<feature>)', that is, there's no value specified.
-            //
-            // Gecko doesn't allow ranged expressions without a value, so just
-            // reject them here too.
-            if input.try(|i| i.expect_colon()).is_err() {
-                if range != Range::Equal {
-                    return Err(input.new_custom_error(StyleParseErrorKind::RangedExpressionWithNoValue));
+            let feature_allows_ranges =
+                feature.mRangeType == nsMediaFeature_RangeType::eMinMaxAllowed;
+
+            let operator = input.try(consume_operation_or_colon);
+            let operator = match operator {
+                Err(..) => {
+                    // If there's no colon, this is a media query of the
+                    // form '(<feature>)', that is, there's no value
+                    // specified.
+                    //
+                    // Gecko doesn't allow ranged expressions without a
+                    // value, so just reject them here too.
+                    if range.is_some() {
+                        return Err(input.new_custom_error(
+                            StyleParseErrorKind::RangedExpressionWithNoValue
+                        ));
+                    }
+
+                    return Ok(Expression::new(
+                        feature,
+                        None,
+                        None,
+                    ));
                 }
-                return Ok(Expression::new(feature, None, range));
-            }
+                Ok(operator) => operator,
+            };
+
+            let range_or_operator = match range {
+                Some(range) => {
+                    if operator.is_some() {
+                        return Err(input.new_custom_error(
+                            StyleParseErrorKind::MediaQueryExpectedFeatureValue
+                        ));
+                    }
+                    Some(RangeOrOperator::Range(range))
+                }
+                None => {
+                    match operator {
+                        Some(operator) => {
+                            if !feature_allows_ranges {
+                                return Err(input.new_custom_error(
+                                    StyleParseErrorKind::MediaQueryExpectedFeatureValue
+                                ));
+                            }
+                            Some(RangeOrOperator::Operator(operator))
+                        }
+                        None => None,
+                    }
+                }
+            };
 
             let value =
                 parse_feature_value(feature, feature.mValueType, context, input).map_err(|err| {
@@ -668,7 +785,7 @@ impl Expression {
                         .new_custom_error(StyleParseErrorKind::MediaQueryExpectedFeatureValue)
                 })?;
 
-            Ok(Expression::new(feature, Some(value), range))
+            Ok(Expression::new(feature, Some(value), range_or_operator))
         })
     }
 
@@ -704,8 +821,8 @@ impl Expression {
         use std::cmp::Ordering;
 
         debug_assert!(
-            self.range == Range::Equal ||
-                self.feature.mRangeType == nsMediaFeature_RangeType::eMinMaxAllowed,
+            self.feature.mRangeType == nsMediaFeature_RangeType::eMinMaxAllowed ||
+            self.range_or_operator.is_none(),
             "Whoops, wrong range"
         );
 
@@ -730,7 +847,7 @@ impl Expression {
         };
 
         // FIXME(emilio): Handle the possible floating point errors?
-        let cmp = match (required_value, actual_value) {
+        let cmp = match (actual_value, required_value) {
             (&Length(ref one), &Length(ref other)) => {
                 computed::Context::for_media_query_evaluation(device, quirks_mode, |context| {
                     one.to_computed_value(&context)
@@ -750,11 +867,11 @@ impl Expression {
                     if (*device.pres_context).mOverrideDPPX > 0.0 {
                         self::Resolution::Dppx((*device.pres_context).mOverrideDPPX).to_dpi()
                     } else {
-                        other.to_dpi()
+                        one.to_dpi()
                     }
                 };
 
-                one.to_dpi().partial_cmp(&actual_dpi).unwrap()
+                actual_dpi.partial_cmp(&other.to_dpi()).unwrap()
             },
             (&Ident(ref one), &Ident(ref other)) => {
                 debug_assert_ne!(
@@ -773,10 +890,31 @@ impl Expression {
             _ => unreachable!(),
         };
 
-        cmp == Ordering::Equal || match self.range {
-            Range::Min => cmp == Ordering::Less,
-            Range::Equal => false,
-            Range::Max => cmp == Ordering::Greater,
+        let range_or_op = match self.range_or_operator {
+            Some(r) => r,
+            None => return cmp == Ordering::Equal,
+        };
+
+        match range_or_op {
+            RangeOrOperator::Range(range) => {
+                cmp == Ordering::Equal || match range {
+                    Range::Min => cmp == Ordering::Greater,
+                    Range::Max => cmp == Ordering::Less,
+                }
+            }
+            RangeOrOperator::Operator(op) => {
+                match op {
+                    Operator::Equal => cmp == Ordering::Equal,
+                    Operator::GreaterThan => cmp == Ordering::Greater,
+                    Operator::GreaterThanEqual => {
+                        cmp == Ordering::Equal || cmp == Ordering::Greater
+                    }
+                    Operator::LessThan => cmp == Ordering::Less,
+                    Operator::LessThanEqual => {
+                        cmp == Ordering::Equal || cmp == Ordering::Less
+                    }
+                }
+            }
         }
     }
 }