Skip to content

Commit

Permalink
Auto merge of #16342 - emilio:media-query-parsing, r=SimonSapin
Browse files Browse the repository at this point in the history
Fix a few media query parsing bugs.

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16342)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Apr 11, 2017
2 parents 2544c08 + 78a3408 commit df67977
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 13 deletions.
9 changes: 5 additions & 4 deletions components/style/gecko/media_queries.rs
Expand Up @@ -409,11 +409,12 @@ impl Expression {
// If there's no colon, this is a media query of the form
// '(<feature>)', that is, there's no value specified.
//
// FIXME(emilio): We need to check for range operators too here when
// we support them, see:
//
// https://drafts.csswg.org/mediaqueries/#mq-ranges
// 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 != nsMediaExpression_Range::eEqual {
return Err(())
}
return Ok(Expression::new(feature, None, range));
}

Expand Down
29 changes: 20 additions & 9 deletions components/style/media_queries.rs
Expand Up @@ -56,10 +56,10 @@ impl ToCss for Qualifier {
fn to_css<W>(&self, dest: &mut W) -> fmt::Result
where W: fmt::Write
{
match *self {
Qualifier::Not => write!(dest, "not"),
Qualifier::Only => write!(dest, "only"),
}
dest.write_str(match *self {
Qualifier::Not => "not",
Qualifier::Only => "only",
})
}
}

Expand Down Expand Up @@ -152,15 +152,26 @@ pub enum MediaQueryType {
}

impl MediaQueryType {
fn parse(ident: &str) -> Self {
fn parse(ident: &str) -> Result<Self, ()> {
if ident.eq_ignore_ascii_case("all") {
return MediaQueryType::All;
return Ok(MediaQueryType::All);
}

match MediaType::parse(ident) {
// From https://drafts.csswg.org/mediaqueries/#mq-syntax:
//
// The <media-type> production does not include the keywords only,
// not, and, and or.
if ident.eq_ignore_ascii_case("not") ||
ident.eq_ignore_ascii_case("or") ||
ident.eq_ignore_ascii_case("and") ||
ident.eq_ignore_ascii_case("only") {
return Err(())
}

Ok(match MediaType::parse(ident) {
Some(media_type) => MediaQueryType::Known(media_type),
None => MediaQueryType::Unknown(Atom::from(ident)),
}
})
}

fn matches(&self, other: MediaType) -> bool {
Expand Down Expand Up @@ -207,7 +218,7 @@ impl MediaQuery {
};

let media_type = match input.try(|input| input.expect_ident()) {
Ok(ident) => MediaQueryType::parse(&*ident),
Ok(ident) => try!(MediaQueryType::parse(&*ident)),
Err(()) => {
// Media type is only optional if qualifier is not specified.
if qualifier.is_some() {
Expand Down

0 comments on commit df67977

Please sign in to comment.