From b4caebae69379c2d8cc756d8f025c5483eca1af7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 26 Dec 2016 22:13:43 +0100 Subject: [PATCH 1/4] style: Cleanup and refactor how media types are represented. This is necessary for the upcoming work in Stylo, given I plan to make MediaType different for Servo and Gecko, and the Unknown variant doesn't fit in MediaType to be fair, the device is never going to have any unknown media type. --- components/style/media_queries.rs | 45 ++++++++++++++++++------------- tests/unit/style/media_queries.rs | 36 ++++++++++++------------- 2 files changed, 45 insertions(+), 36 deletions(-) diff --git a/components/style/media_queries.rs b/components/style/media_queries.rs index a450995c565..29b72f5b0aa 100644 --- a/components/style/media_queries.rs +++ b/components/style/media_queries.rs @@ -94,6 +94,12 @@ pub struct MediaQuery { } impl MediaQuery { + /// Return a media query that never matches, used for when we fail to parse + /// a given media query. + fn never_matching() -> Self { + Self::new(Some(Qualifier::Not), MediaQueryType::All, vec![]) + } + pub fn new(qualifier: Option, media_type: MediaQueryType, expressions: Vec) -> MediaQuery { MediaQuery { @@ -115,9 +121,9 @@ impl ToCss for MediaQuery { let mut type_ = String::new(); match self.media_type { MediaQueryType::All => try!(write!(type_, "all")), - MediaQueryType::MediaType(MediaType::Screen) => try!(write!(type_, "screen")), - MediaQueryType::MediaType(MediaType::Print) => try!(write!(type_, "print")), - MediaQueryType::MediaType(MediaType::Unknown(ref desc)) => try!(write!(type_, "{}", desc)), + MediaQueryType::Known(MediaType::Screen) => try!(write!(type_, "screen")), + MediaQueryType::Known(MediaType::Print) => try!(write!(type_, "print")), + MediaQueryType::Unknown(ref desc) => try!(write!(type_, "{}", desc)), }; if self.expressions.is_empty() { return write!(dest, "{}", type_) @@ -148,7 +154,18 @@ impl ToCss for MediaQuery { #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub enum MediaQueryType { All, // Always true - MediaType(MediaType), + Known(MediaType), + Unknown(Atom), +} + +impl MediaQueryType { + fn matches(&self, other: &MediaType) -> bool { + match *self { + MediaQueryType::All => true, + MediaQueryType::Known(ref known_type) => known_type == other, + MediaQueryType::Unknown(..) => false, + } + } } #[derive(PartialEq, Eq, Clone, Debug)] @@ -156,7 +173,6 @@ pub enum MediaQueryType { pub enum MediaType { Screen, Print, - Unknown(Atom), } #[derive(Debug)] @@ -220,10 +236,10 @@ impl MediaQuery { let media_type; if let Ok(ident) = input.try(|input| input.expect_ident()) { media_type = match_ignore_ascii_case! { ident, - "screen" => MediaQueryType::MediaType(MediaType::Screen), - "print" => MediaQueryType::MediaType(MediaType::Print), + "screen" => MediaQueryType::Known(MediaType::Screen), + "print" => MediaQueryType::Known(MediaType::Print), "all" => MediaQueryType::All, - _ => MediaQueryType::MediaType(MediaType::Unknown(Atom::from(&*ident))) + _ => MediaQueryType::Unknown(Atom::from(&*ident)) } } else { // Media type is only optional if qualifier is not specified. @@ -253,10 +269,8 @@ pub fn parse_media_query_list(input: &mut Parser) -> MediaList { let mut media_queries = vec![]; loop { media_queries.push( - input.parse_until_before(Delimiter::Comma, MediaQuery::parse) - .unwrap_or(MediaQuery::new(Some(Qualifier::Not), - MediaQueryType::All, - vec!()))); + input.parse_until_before(Delimiter::Comma, MediaQuery::parse).ok() + .unwrap_or_else(MediaQuery::never_matching)); match input.next() { Ok(Token::Comma) => {}, Ok(_) => unreachable!(), @@ -275,12 +289,7 @@ impl MediaList { // Check if it is an empty media query list or any queries match (OR condition) // https://drafts.csswg.org/mediaqueries-4/#mq-list self.media_queries.is_empty() || self.media_queries.iter().any(|mq| { - // Check if media matches. Unknown media never matches. - let media_match = match mq.media_type { - MediaQueryType::MediaType(MediaType::Unknown(_)) => false, - MediaQueryType::MediaType(ref media_type) => *media_type == device.media_type, - MediaQueryType::All => true, - }; + let media_match = mq.media_type.matches(&device.media_type); // Check if all conditions match (AND condition) let query_match = media_match && mq.expressions.iter().all(|expression| { diff --git a/tests/unit/style/media_queries.rs b/tests/unit/style/media_queries.rs index ac5a9b501c2..e393fd552b7 100644 --- a/tests/unit/style/media_queries.rs +++ b/tests/unit/style/media_queries.rs @@ -74,7 +74,7 @@ fn test_mq_screen() { assert!(list.media_queries.len() == 1, css.to_owned()); let q = &list.media_queries[0]; assert!(q.qualifier == None, css.to_owned()); - assert!(q.media_type == MediaQueryType::MediaType(MediaType::Screen), css.to_owned()); + assert!(q.media_type == MediaQueryType::Known(MediaType::Screen), css.to_owned()); assert!(q.expressions.len() == 0, css.to_owned()); }); @@ -82,7 +82,7 @@ fn test_mq_screen() { assert!(list.media_queries.len() == 1, css.to_owned()); let q = &list.media_queries[0]; assert!(q.qualifier == Some(Qualifier::Only), css.to_owned()); - assert!(q.media_type == MediaQueryType::MediaType(MediaType::Screen), css.to_owned()); + assert!(q.media_type == MediaQueryType::Known(MediaType::Screen), css.to_owned()); assert!(q.expressions.len() == 0, css.to_owned()); }); @@ -90,7 +90,7 @@ fn test_mq_screen() { assert!(list.media_queries.len() == 1, css.to_owned()); let q = &list.media_queries[0]; assert!(q.qualifier == Some(Qualifier::Not), css.to_owned()); - assert!(q.media_type == MediaQueryType::MediaType(MediaType::Screen), css.to_owned()); + assert!(q.media_type == MediaQueryType::Known(MediaType::Screen), css.to_owned()); assert!(q.expressions.len() == 0, css.to_owned()); }); } @@ -101,7 +101,7 @@ fn test_mq_print() { assert!(list.media_queries.len() == 1, css.to_owned()); let q = &list.media_queries[0]; assert!(q.qualifier == None, css.to_owned()); - assert!(q.media_type == MediaQueryType::MediaType(MediaType::Print), css.to_owned()); + assert!(q.media_type == MediaQueryType::Known(MediaType::Print), css.to_owned()); assert!(q.expressions.len() == 0, css.to_owned()); }); @@ -109,7 +109,7 @@ fn test_mq_print() { assert!(list.media_queries.len() == 1, css.to_owned()); let q = &list.media_queries[0]; assert!(q.qualifier == Some(Qualifier::Only), css.to_owned()); - assert!(q.media_type == MediaQueryType::MediaType(MediaType::Print), css.to_owned()); + assert!(q.media_type == MediaQueryType::Known(MediaType::Print), css.to_owned()); assert!(q.expressions.len() == 0, css.to_owned()); }); @@ -117,7 +117,7 @@ fn test_mq_print() { assert!(list.media_queries.len() == 1, css.to_owned()); let q = &list.media_queries[0]; assert!(q.qualifier == Some(Qualifier::Not), css.to_owned()); - assert!(q.media_type == MediaQueryType::MediaType(MediaType::Print), css.to_owned()); + assert!(q.media_type == MediaQueryType::Known(MediaType::Print), css.to_owned()); assert!(q.expressions.len() == 0, css.to_owned()); }); } @@ -128,7 +128,7 @@ fn test_mq_unknown() { assert!(list.media_queries.len() == 1, css.to_owned()); let q = &list.media_queries[0]; assert!(q.qualifier == None, css.to_owned()); - assert!(q.media_type == MediaQueryType::MediaType(MediaType::Unknown(Atom::from("fridge"))), css.to_owned()); + assert!(q.media_type == MediaQueryType::Unknown(Atom::from("fridge")), css.to_owned()); assert!(q.expressions.len() == 0, css.to_owned()); }); @@ -136,7 +136,7 @@ fn test_mq_unknown() { assert!(list.media_queries.len() == 1, css.to_owned()); let q = &list.media_queries[0]; assert!(q.qualifier == Some(Qualifier::Only), css.to_owned()); - assert!(q.media_type == MediaQueryType::MediaType(MediaType::Unknown(Atom::from("glass"))), css.to_owned()); + assert!(q.media_type == MediaQueryType::Unknown(Atom::from("glass")), css.to_owned()); assert!(q.expressions.len() == 0, css.to_owned()); }); @@ -144,7 +144,7 @@ fn test_mq_unknown() { assert!(list.media_queries.len() == 1, css.to_owned()); let q = &list.media_queries[0]; assert!(q.qualifier == Some(Qualifier::Not), css.to_owned()); - assert!(q.media_type == MediaQueryType::MediaType(MediaType::Unknown(Atom::from("wood"))), css.to_owned()); + assert!(q.media_type == MediaQueryType::Unknown(Atom::from("wood")), css.to_owned()); assert!(q.expressions.len() == 0, css.to_owned()); }); } @@ -182,12 +182,12 @@ fn test_mq_or() { assert!(list.media_queries.len() == 2, css.to_owned()); let q0 = &list.media_queries[0]; assert!(q0.qualifier == None, css.to_owned()); - assert!(q0.media_type == MediaQueryType::MediaType(MediaType::Screen), css.to_owned()); + assert!(q0.media_type == MediaQueryType::Known(MediaType::Screen), css.to_owned()); assert!(q0.expressions.len() == 0, css.to_owned()); let q1 = &list.media_queries[1]; assert!(q1.qualifier == None, css.to_owned()); - assert!(q1.media_type == MediaQueryType::MediaType(MediaType::Print), css.to_owned()); + assert!(q1.media_type == MediaQueryType::Known(MediaType::Print), css.to_owned()); assert!(q1.expressions.len() == 0, css.to_owned()); }); } @@ -225,7 +225,7 @@ fn test_mq_expressions() { assert!(list.media_queries.len() == 1, css.to_owned()); let q = &list.media_queries[0]; assert!(q.qualifier == None, css.to_owned()); - assert!(q.media_type == MediaQueryType::MediaType(MediaType::Screen), css.to_owned()); + assert!(q.media_type == MediaQueryType::Known(MediaType::Screen), css.to_owned()); assert!(q.expressions.len() == 1, css.to_owned()); match q.expressions[0] { Expression::Width(Range::Min(w)) => assert!(w == specified::Length::Absolute(Au::from_px(100))), @@ -237,7 +237,7 @@ fn test_mq_expressions() { assert!(list.media_queries.len() == 1, css.to_owned()); let q = &list.media_queries[0]; assert!(q.qualifier == None, css.to_owned()); - assert!(q.media_type == MediaQueryType::MediaType(MediaType::Print), css.to_owned()); + assert!(q.media_type == MediaQueryType::Known(MediaType::Print), css.to_owned()); assert!(q.expressions.len() == 1, css.to_owned()); match q.expressions[0] { Expression::Width(Range::Max(w)) => assert!(w == specified::Length::Absolute(Au::from_px(43))), @@ -249,7 +249,7 @@ fn test_mq_expressions() { assert!(list.media_queries.len() == 1, css.to_owned()); let q = &list.media_queries[0]; assert!(q.qualifier == None, css.to_owned()); - assert!(q.media_type == MediaQueryType::MediaType(MediaType::Print), css.to_owned()); + assert!(q.media_type == MediaQueryType::Known(MediaType::Print), css.to_owned()); assert!(q.expressions.len() == 1, css.to_owned()); match q.expressions[0] { Expression::Width(Range::Eq(w)) => assert!(w == specified::Length::Absolute(Au::from_px(43))), @@ -261,7 +261,7 @@ fn test_mq_expressions() { assert!(list.media_queries.len() == 1, css.to_owned()); let q = &list.media_queries[0]; assert!(q.qualifier == None, css.to_owned()); - assert!(q.media_type == MediaQueryType::MediaType(MediaType::Unknown(Atom::from("fridge"))), css.to_owned()); + assert!(q.media_type == MediaQueryType::Unknown(Atom::from("fridge")), css.to_owned()); assert!(q.expressions.len() == 1, css.to_owned()); match q.expressions[0] { Expression::Width(Range::Max(w)) => assert!(w == specified::Length::Absolute(Au::from_px(52))), @@ -302,7 +302,7 @@ fn test_mq_multiple_expressions() { assert!(list.media_queries.len() == 1, css.to_owned()); let q = &list.media_queries[0]; assert!(q.qualifier == Some(Qualifier::Not), css.to_owned()); - assert!(q.media_type == MediaQueryType::MediaType(MediaType::Screen), css.to_owned()); + assert!(q.media_type == MediaQueryType::Known(MediaType::Screen), css.to_owned()); assert!(q.expressions.len() == 2, css.to_owned()); match q.expressions[0] { Expression::Width(Range::Min(w)) => assert!(w == specified::Length::Absolute(Au::from_px(100))), @@ -377,7 +377,7 @@ fn test_mq_malformed_expressions() { assert!(q0.expressions.len() == 0, css.to_owned()); let q1 = &list.media_queries[1]; assert!(q1.qualifier == None, css.to_owned()); - assert!(q1.media_type == MediaQueryType::MediaType(MediaType::Print), css.to_owned()); + assert!(q1.media_type == MediaQueryType::Known(MediaType::Print), css.to_owned()); assert!(q1.expressions.len() == 0, css.to_owned()); }); @@ -385,7 +385,7 @@ fn test_mq_malformed_expressions() { assert!(list.media_queries.len() == 2, css.to_owned()); let q0 = &list.media_queries[0]; assert!(q0.qualifier == None, css.to_owned()); - assert!(q0.media_type == MediaQueryType::MediaType(MediaType::Screen), css.to_owned()); + assert!(q0.media_type == MediaQueryType::Known(MediaType::Screen), css.to_owned()); assert!(q0.expressions.len() == 0, css.to_owned()); let q1 = &list.media_queries[1]; assert!(q1.qualifier == Some(Qualifier::Not), css.to_owned()); From 8120906a15c8b1747ea0b97fdae71abf7a7c1f2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 26 Dec 2016 22:25:46 +0100 Subject: [PATCH 2/4] style: Refactor media query serialization so it doesn't use an intermediate buffer. I plan to move MediaType to ToCss soon. --- components/style/media_queries.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/components/style/media_queries.rs b/components/style/media_queries.rs index 29b72f5b0aa..f005fc62729 100644 --- a/components/style/media_queries.rs +++ b/components/style/media_queries.rs @@ -11,7 +11,7 @@ use app_units::Au; use cssparser::{Delimiter, Parser, Token}; use euclid::size::{Size2D, TypedSize2D}; use serialize_comma_separated_list; -use std::fmt::{self, Write}; +use std::fmt; use style_traits::{ToCss, ViewportPx}; use values::computed::{self, ToComputedValue}; use values::specified; @@ -118,18 +118,21 @@ impl ToCss for MediaQuery { try!(write!(dest, "not ")); } - let mut type_ = String::new(); match self.media_type { - MediaQueryType::All => try!(write!(type_, "all")), - MediaQueryType::Known(MediaType::Screen) => try!(write!(type_, "screen")), - MediaQueryType::Known(MediaType::Print) => try!(write!(type_, "print")), - MediaQueryType::Unknown(ref desc) => try!(write!(type_, "{}", desc)), + MediaQueryType::All => try!(write!(dest, "all")), + MediaQueryType::Known(MediaType::Screen) => try!(write!(dest, "screen")), + MediaQueryType::Known(MediaType::Print) => try!(write!(dest, "print")), + MediaQueryType::Unknown(ref desc) => try!(write!(dest, "{}", desc)), }; + if self.expressions.is_empty() { - return write!(dest, "{}", type_) - } else if type_ != "all" || self.qualifier == Some(Qualifier::Not) { - try!(write!(dest, "{} and ", type_)); + return Ok(()); } + + if self.media_type != MediaQueryType::All || self.qualifier == Some(Qualifier::Not) { + try!(write!(dest, " and ")); + } + for (i, &e) in self.expressions.iter().enumerate() { try!(write!(dest, "(")); let (mm, l) = match e { From bb672f1a2c2481d628f80b00c1bfa66ace4a6c1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 26 Dec 2016 22:43:44 +0100 Subject: [PATCH 3/4] style: Simplify media-query parsing so it's easier to split out. --- components/style/media_queries.rs | 51 +++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/components/style/media_queries.rs b/components/style/media_queries.rs index f005fc62729..eca6f0a7cd8 100644 --- a/components/style/media_queries.rs +++ b/components/style/media_queries.rs @@ -11,6 +11,7 @@ use app_units::Au; use cssparser::{Delimiter, Parser, Token}; use euclid::size::{Size2D, TypedSize2D}; use serialize_comma_separated_list; +use std::ascii::AsciiExt; use std::fmt; use style_traits::{ToCss, ViewportPx}; use values::computed::{self, ToComputedValue}; @@ -162,6 +163,17 @@ pub enum MediaQueryType { } impl MediaQueryType { + fn parse(ident: &str) -> Self { + if ident.eq_ignore_ascii_case("all") { + return MediaQueryType::All; + } + + match MediaType::parse(ident) { + Some(media_type) => MediaQueryType::Known(media_type), + None => MediaQueryType::Unknown(Atom::from(ident)), + } + } + fn matches(&self, other: &MediaType) -> bool { match *self { MediaQueryType::All => true, @@ -178,6 +190,16 @@ pub enum MediaType { Print, } +impl MediaType { + fn parse(name: &str) -> Option { + Some(match_ignore_ascii_case! { name, + "screen" => MediaType::Screen, + "print" => MediaType::Print, + _ => return None + }) + } +} + #[derive(Debug)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub struct Device { @@ -236,23 +258,20 @@ impl MediaQuery { None }; - let media_type; - if let Ok(ident) = input.try(|input| input.expect_ident()) { - media_type = match_ignore_ascii_case! { ident, - "screen" => MediaQueryType::Known(MediaType::Screen), - "print" => MediaQueryType::Known(MediaType::Print), - "all" => MediaQueryType::All, - _ => MediaQueryType::Unknown(Atom::from(&*ident)) + let media_type = match input.try(|input| input.expect_ident()) { + Ok(ident) => MediaQueryType::parse(&*ident), + Err(()) => { + // Media type is only optional if qualifier is not specified. + if qualifier.is_some() { + return Err(()) + } + + // Without a media type, require at least one expression. + expressions.push(try!(Expression::parse(input))); + + MediaQueryType::All } - } else { - // Media type is only optional if qualifier is not specified. - if qualifier.is_some() { - return Err(()) - } - media_type = MediaQueryType::All; - // Without a media type, require at least one expression - expressions.push(try!(Expression::parse(input))); - } + }; // Parse any subsequent expressions loop { From c04c3c60c3cb02f0af98b5b559e64e09309c1f6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 27 Dec 2016 01:00:12 +0100 Subject: [PATCH 4/4] style: Serialize also "only" qualifier, and fix serialization of all media expressions without explicit qualifiers. --- components/style/media_queries.rs | 38 +++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/components/style/media_queries.rs b/components/style/media_queries.rs index eca6f0a7cd8..5762effb4c2 100644 --- a/components/style/media_queries.rs +++ b/components/style/media_queries.rs @@ -86,6 +86,17 @@ pub enum Qualifier { Not, } +impl ToCss for Qualifier { + fn to_css(&self, dest: &mut W) -> fmt::Result + where W: fmt::Write + { + match *self { + Qualifier::Not => write!(dest, "not"), + Qualifier::Only => write!(dest, "only"), + } + } +} + #[derive(Clone, Debug, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub struct MediaQuery { @@ -115,22 +126,32 @@ impl ToCss for MediaQuery { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - if self.qualifier == Some(Qualifier::Not) { - try!(write!(dest, "not ")); + if let Some(qual) = self.qualifier { + try!(qual.to_css(dest)); + try!(write!(dest, " ")); } match self.media_type { - MediaQueryType::All => try!(write!(dest, "all")), + 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() { + try!(write!(dest, "all")); + } + }, MediaQueryType::Known(MediaType::Screen) => try!(write!(dest, "screen")), MediaQueryType::Known(MediaType::Print) => try!(write!(dest, "print")), MediaQueryType::Unknown(ref desc) => try!(write!(dest, "{}", desc)), - }; + } if self.expressions.is_empty() { return Ok(()); } - if self.media_type != MediaQueryType::All || self.qualifier == Some(Qualifier::Not) { + if self.media_type != MediaQueryType::All || self.qualifier.is_some() { try!(write!(dest, " and ")); } @@ -143,10 +164,9 @@ impl ToCss for MediaQuery { }; try!(write!(dest, "{}width: ", mm)); try!(l.to_css(dest)); - if i == self.expressions.len() - 1 { - try!(write!(dest, ")")); - } else { - try!(write!(dest, ") and ")); + try!(write!(dest, ")")); + if i != self.expressions.len() - 1 { + try!(write!(dest, " and ")); } } Ok(())