Bug 1422225: Evaluate MediaConditions, and glue it all together. r?xidorn draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sat, 16 Jun 2018 00:59:04 -0700
changeset 809286 3520a9328ae5dbd74c4cffa1f202bfdde65c7d44
parent 809285 a32a8bf9e4eeb2bc4f1fb8f49fa28ff9de82a1d8
child 809287 02977a9a6d6348d1ccba55612e13b15341a7fd71
push id113617
push userbmo:emilio@crisal.io
push dateThu, 21 Jun 2018 17:44:14 +0000
reviewersxidorn
bugs1422225
milestone62.0a1
Bug 1422225: Evaluate MediaConditions, and glue it all together. r?xidorn MozReview-Commit-ID: 3MThE2FvfDf
servo/components/style/media_queries/media_condition.rs
servo/components/style/media_queries/media_list.rs
servo/components/style/media_queries/media_query.rs
servo/components/style/values/specified/source_size_list.rs
--- a/servo/components/style/media_queries/media_condition.rs
+++ b/servo/components/style/media_queries/media_condition.rs
@@ -2,32 +2,41 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! A media query condition:
 //!
 //! https://drafts.csswg.org/mediaqueries-4/#typedef-media-condition
 
 use cssparser::{Parser, Token};
+use context::QuirksMode;
 use parser::ParserContext;
 use std::fmt::{self, Write};
-use style_traits::{CssWriter, ParseError, ToCss};
+use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
 
-use super::MediaFeatureExpression;
+use super::{Device, MediaFeatureExpression};
 
 
 /// A binary `and` or `or` operator.
 #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, Parse, ToCss)]
 #[allow(missing_docs)]
 pub enum Operator {
     And,
     Or,
 }
 
+/// Whether to allow an `or` condition or not during parsing.
+#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, Parse, ToCss)]
+enum AllowOr {
+    Yes,
+    No,
+}
+
 /// Represents a media condition.
+#[derive(Clone, Debug, MallocSizeOf, PartialEq)]
 pub enum MediaCondition {
     /// A simple media feature expression, implicitly parenthesized.
     Feature(MediaFeatureExpression),
     /// A negation of a condition.
     Not(Box<MediaCondition>),
     /// A set of joint operations.
     Operation(Box<[MediaCondition]>, Operator),
     /// A condition wrapped in parenthesis.
@@ -68,16 +77,34 @@ impl ToCss for MediaCondition {
 }
 
 impl MediaCondition {
     /// Parse a single media condition.
     pub fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
+        Self::parse_internal(context, input, AllowOr::Yes)
+    }
+
+    /// Parse a single media condition, disallowing `or` expressions.
+    ///
+    /// To be used from the legacy media query syntax.
+    pub fn parse_disallow_or<'i, 't>(
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Self, ParseError<'i>> {
+        Self::parse_internal(context, input, AllowOr::No)
+    }
+
+    fn parse_internal<'i, 't>(
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+        allow_or: AllowOr,
+    ) -> Result<Self, ParseError<'i>> {
         let location = input.current_source_location();
 
         // FIXME(emilio): This can be cleaner with nll.
         let is_negation = match *input.next()? {
             Token::ParenthesisBlock => false,
             Token::Ident(ref ident) if ident.eq_ignore_ascii_case("not") => true,
             ref t => {
                 return Err(location.new_unexpected_token_error(t.clone()))
@@ -91,16 +118,20 @@ impl MediaCondition {
 
         // ParenthesisBlock.
         let first_condition = Self::parse_paren_block(context, input)?;
         let operator = match input.try(Operator::parse) {
             Ok(op) => op,
             Err(..) => return Ok(first_condition),
         };
 
+        if allow_or == AllowOr::No && operator == Operator::Or {
+            return Err(location.new_custom_error(StyleParseErrorKind::UnspecifiedError));
+        }
+
         let mut conditions = vec![];
         conditions.push(first_condition);
         conditions.push(Self::parse_in_parens(context, input)?);
 
         let delim = match operator {
             Operator::And => "and",
             Operator::Or => "or",
         };
@@ -135,9 +166,29 @@ impl MediaCondition {
             if let Ok(expr) = input.try(|i| MediaFeatureExpression::parse_in_parenthesis_block(context, i)) {
                 return Ok(MediaCondition::Feature(expr));
             }
 
             let inner = Self::parse(context, input)?;
             Ok(MediaCondition::InParens(Box::new(inner)))
         })
     }
+
+    /// Whether this condition matches the device and quirks mode.
+    pub fn matches(&self, device: &Device, quirks_mode: QuirksMode) -> bool {
+        match *self {
+            MediaCondition::Feature(ref f) => f.matches(device, quirks_mode),
+            MediaCondition::InParens(ref c) => c.matches(device, quirks_mode),
+            MediaCondition::Not(ref c) => !c.matches(device, quirks_mode),
+            MediaCondition::Operation(ref conditions, op) => {
+                let mut iter = conditions.iter();
+                match op {
+                    Operator::And => {
+                        iter.all(|c| c.matches(device, quirks_mode))
+                    }
+                    Operator::Or => {
+                        iter.any(|c| c.matches(device, quirks_mode))
+                    }
+                }
+            }
+        }
+    }
 }
--- a/servo/components/style/media_queries/media_list.rs
+++ b/servo/components/style/media_queries/media_list.rs
@@ -68,26 +68,24 @@ impl MediaList {
     pub fn empty() -> Self {
         MediaList {
             media_queries: vec![],
         }
     }
 
     /// Evaluate a whole `MediaList` against `Device`.
     pub fn evaluate(&self, device: &Device, quirks_mode: QuirksMode) -> bool {
-        // Check if it is an empty media query list or any queries match (OR condition)
+        // Check if it is an empty media query list or any queries match.
         // https://drafts.csswg.org/mediaqueries-4/#mq-list
         self.media_queries.is_empty() || self.media_queries.iter().any(|mq| {
             let media_match = mq.media_type.matches(device.media_type());
 
-            // Check if all conditions match (AND condition)
+            // Check if the media condition match.
             let query_match = media_match &&
-                mq.expressions
-                    .iter()
-                    .all(|expression| expression.matches(&device, quirks_mode));
+                mq.condition.as_ref().map_or(true, |c| c.matches(device, quirks_mode));
 
             // Apply the logical NOT qualifier to the result
             match mq.qualifier {
                 Some(Qualifier::Not) => !query_match,
                 _ => query_match,
             }
         })
     }
--- a/servo/components/style/media_queries/media_query.rs
+++ b/servo/components/style/media_queries/media_query.rs
@@ -7,20 +7,21 @@
 //! https://drafts.csswg.org/mediaqueries/#typedef-media-query
 
 use Atom;
 use cssparser::Parser;
 use parser::ParserContext;
 use selectors::parser::SelectorParseErrorKind;
 use std::fmt::{self, Write};
 use str::string_as_ascii_lowercase;
-use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
-use super::MediaFeatureExpression;
+use style_traits::{CssWriter, ParseError, ToCss};
+use super::media_condition::MediaCondition;
 use values::CustomIdent;
 
+
 /// <https://drafts.csswg.org/mediaqueries/#mq-prefix>
 #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, Parse, PartialEq, ToCss)]
 pub enum Qualifier {
     /// Hide a media query from legacy UAs:
     /// <https://drafts.csswg.org/mediaqueries/#mq-only>
     Only,
     /// Negate a media query:
     /// <https://drafts.csswg.org/mediaqueries/#mq-not>
@@ -60,18 +61,19 @@ impl MediaType {
 ///
 /// [mq]: https://drafts.csswg.org/mediaqueries/
 #[derive(Clone, Debug, MallocSizeOf, PartialEq)]
 pub struct MediaQuery {
     /// The qualifier for this query.
     pub qualifier: Option<Qualifier>,
     /// The media type for this query, that can be known, unknown, or "all".
     pub media_type: MediaQueryType,
-    /// The set of range expressions that this media query contains.
-    pub expressions: Vec<MediaFeatureExpression>,
+    /// The condition that this media query contains. This cannot have `or`
+    /// in the first level.
+    pub condition: Option<MediaCondition>,
 }
 
 impl ToCss for MediaQuery {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
         W: Write,
     {
         if let Some(qual) = self.qualifier {
@@ -81,93 +83,82 @@ impl ToCss for MediaQuery {
 
         match self.media_type {
             MediaQueryType::All => {
                 // We need to print "all" if there's a qualifier, or there's
                 // just an empty list of expressions.
                 //
                 // Otherwise, we'd serialize media queries like "(min-width:
                 // 40px)" in "all (min-width: 40px)", which is unexpected.
-                if self.qualifier.is_some() || self.expressions.is_empty() {
+                if self.qualifier.is_some() || self.condition.is_none() {
                     dest.write_str("all")?;
                 }
             },
             MediaQueryType::Concrete(MediaType(ref desc)) => desc.to_css(dest)?,
         }
 
-        if self.expressions.is_empty() {
-            return Ok(());
-        }
+        let condition = match self.condition {
+            Some(ref c) => c,
+            None => return Ok(()),
+        };
 
         if self.media_type != MediaQueryType::All || self.qualifier.is_some() {
             dest.write_str(" and ")?;
         }
 
-        self.expressions[0].to_css(dest)?;
-
-        for expr in self.expressions.iter().skip(1) {
-            dest.write_str(" and ")?;
-            expr.to_css(dest)?;
-        }
-        Ok(())
+        condition.to_css(dest)
     }
 }
 
 impl MediaQuery {
     /// Return a media query that never matches, used for when we fail to parse
     /// a given media query.
     pub fn never_matching() -> Self {
         Self {
             qualifier: Some(Qualifier::Not),
             media_type: MediaQueryType::All,
-            expressions: vec![],
+            condition: None,
         }
     }
 
     /// Parse a media query given css input.
     ///
     /// Returns an error if any of the expressions is unknown.
     pub fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<MediaQuery, ParseError<'i>> {
-        let mut expressions = vec![];
+        if let Ok(condition) = input.try(|i| MediaCondition::parse(context, i)) {
+            return Ok(Self {
+                qualifier: None,
+                media_type: MediaQueryType::All,
+                condition: Some(condition),
+            })
+        }
 
         let qualifier = input.try(Qualifier::parse).ok();
-        let media_type = match input.try(|i| i.expect_ident_cloned()) {
-            Ok(ident) => MediaQueryType::parse(&*ident).map_err(|()| {
-                input.new_custom_error(SelectorParseErrorKind::UnexpectedIdent(ident.clone()))
-            })?,
-            Err(_) => {
-                // Media type is only optional if qualifier is not specified.
-                if qualifier.is_some() {
-                    return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
-                }
-
-                // Without a media type, require at least one expression.
-                expressions.push(MediaFeatureExpression::parse(context, input)?);
-
-                MediaQueryType::All
-            },
+        let media_type = {
+            let location = input.current_source_location();
+            let ident = input.expect_ident()?;
+            match MediaQueryType::parse(&ident) {
+                Ok(t) => t,
+                Err(..) => return Err(location.new_custom_error(
+                    SelectorParseErrorKind::UnexpectedIdent(ident.clone())
+                ))
+            }
         };
 
-        // Parse any subsequent expressions
-        loop {
-            if input
-                .try(|input| input.expect_ident_matching("and"))
-                .is_err()
-            {
-                return Ok(MediaQuery {
-                    qualifier,
-                    media_type,
-                    expressions,
-                });
-            }
-            expressions.push(MediaFeatureExpression::parse(context, input)?)
-        }
+        let condition =
+            if input.try(|i| i.expect_ident_matching("and")).is_ok() {
+                Some(MediaCondition::parse_disallow_or(context, input)?)
+            } else {
+                None
+            };
+
+        Ok(Self { qualifier, media_type, condition })
     }
 }
 
 /// <http://dev.w3.org/csswg/mediaqueries-3/#media0>
 #[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq)]
 pub enum MediaQueryType {
     /// A media type that matches every device.
     All,
--- a/servo/components/style/values/specified/source_size_list.rs
+++ b/servo/components/style/values/specified/source_size_list.rs
@@ -3,39 +3,37 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! https://html.spec.whatwg.org/multipage/#source-size-list
 
 use app_units::Au;
 use cssparser::{Delimiter, Parser, Token};
 #[cfg(feature = "gecko")]
 use gecko_bindings::sugar::ownership::{HasBoxFFI, HasFFI, HasSimpleFFI};
-use media_queries::{Device, MediaFeatureExpression};
+use media_queries::{Device, MediaCondition};
 use parser::{Parse, ParserContext};
 use selectors::context::QuirksMode;
 use style_traits::ParseError;
 use values::computed::{self, ToComputedValue};
 use values::specified::{Length, NoCalcLength, ViewportPercentageLength};
 
 /// A value for a `<source-size>`:
 ///
 /// https://html.spec.whatwg.org/multipage/#source-size
 pub struct SourceSize {
-    // FIXME(emilio): This should be a `MediaCondition`, and support `and` and
-    // `or`.
-    condition: MediaFeatureExpression,
+    condition: MediaCondition,
     value: Length,
 }
 
 impl Parse for SourceSize {
     fn parse<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Self, ParseError<'i>> {
-        let condition = MediaFeatureExpression::parse(context, input)?;
+        let condition = MediaCondition::parse(context, input)?;
         let value = Length::parse_non_negative(context, input)?;
 
         Ok(Self { condition, value })
     }
 }
 
 /// A value for a `<source-size-list>`:
 ///