From 3a92fd1cfc1fa87b36d1c1a594ce5394245e06e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 16 Jun 2018 00:59:04 -0700 Subject: [PATCH] style: Evaluate MediaConditions, and glue it all together. Bug: 1422225 Reviewed-by: xidorn MozReview-Commit-ID: 3MThE2FvfDf --- .../style/media_queries/media_condition.rs | 55 +++++++++++- components/style/media_queries/media_list.rs | 8 +- components/style/media_queries/media_query.rs | 83 +++++++++---------- .../values/specified/source_size_list.rs | 8 +- 4 files changed, 96 insertions(+), 58 deletions(-) diff --git a/components/style/media_queries/media_condition.rs b/components/style/media_queries/media_condition.rs index 091d7049599..c2c29c129ca 100644 --- a/components/style/media_queries/media_condition.rs +++ b/components/style/media_queries/media_condition.rs @@ -7,11 +7,12 @@ //! 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. @@ -22,7 +23,15 @@ pub enum Operator { 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), @@ -72,6 +81,24 @@ impl MediaCondition { pub fn parse<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, + ) -> Result> { + 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::parse_internal(context, input, AllowOr::No) + } + + fn parse_internal<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + allow_or: AllowOr, ) -> Result> { let location = input.current_source_location(); @@ -96,6 +123,10 @@ impl MediaCondition { 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)?); @@ -140,4 +171,24 @@ impl MediaCondition { 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)) + } + } + } + } + } } diff --git a/components/style/media_queries/media_list.rs b/components/style/media_queries/media_list.rs index a5604b6dc19..f8d15df7257 100644 --- a/components/style/media_queries/media_list.rs +++ b/components/style/media_queries/media_list.rs @@ -73,16 +73,14 @@ impl MediaList { /// 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 { diff --git a/components/style/media_queries/media_query.rs b/components/style/media_queries/media_query.rs index 03e1e2de108..a2f4f9bea70 100644 --- a/components/style/media_queries/media_query.rs +++ b/components/style/media_queries/media_query.rs @@ -12,10 +12,11 @@ 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; + /// #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, Parse, PartialEq, ToCss)] pub enum Qualifier { @@ -65,8 +66,9 @@ pub struct MediaQuery { pub qualifier: Option, /// 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, + /// The condition that this media query contains. This cannot have `or` + /// in the first level. + pub condition: Option, } impl ToCss for MediaQuery { @@ -86,28 +88,23 @@ impl ToCss for MediaQuery { // // 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) } } @@ -118,7 +115,7 @@ impl MediaQuery { Self { qualifier: Some(Qualifier::Not), media_type: MediaQueryType::All, - expressions: vec![], + condition: None, } } @@ -129,40 +126,34 @@ impl MediaQuery { context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - 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 }) } } diff --git a/components/style/values/specified/source_size_list.rs b/components/style/values/specified/source_size_list.rs index f0a0a47e4a9..d41fc283bcc 100644 --- a/components/style/values/specified/source_size_list.rs +++ b/components/style/values/specified/source_size_list.rs @@ -8,7 +8,7 @@ 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; @@ -19,9 +19,7 @@ use values::specified::{Length, NoCalcLength, ViewportPercentageLength}; /// /// 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, } @@ -30,7 +28,7 @@ impl Parse for SourceSize { context: &ParserContext, input: &mut Parser<'i, 't>, ) -> Result> { - let condition = MediaFeatureExpression::parse(context, input)?; + let condition = MediaCondition::parse(context, input)?; let value = Length::parse_non_negative(context, input)?; Ok(Self { condition, value })