Auto merge of #15941 - emilio:media, r=heycam

style: Fix media query parsing when invalid queries are present.

Fixes [bug 1347273](https://bugzil.la/1347273).

<!-- 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/15941)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-03-15 09:56:40 -07:00 committed by GitHub
commit 304cafe57d
4 changed files with 34 additions and 17 deletions

View file

@ -243,19 +243,13 @@ pub fn parse_media_query_list(input: &mut Parser) -> MediaList {
} }
let mut media_queries = vec![]; let mut media_queries = vec![];
let mut found_invalid = false;
loop { loop {
match input.parse_until_before(Delimiter::Comma, MediaQuery::parse) { match input.parse_until_before(Delimiter::Comma, MediaQuery::parse) {
Ok(mq) => if !found_invalid { Ok(mq) => {
media_queries.push(mq); media_queries.push(mq);
}, },
Err(..) => if !found_invalid { Err(..) => {
media_queries.clear();
media_queries.push(MediaQuery::never_matching()); media_queries.push(MediaQuery::never_matching());
// Consume the rest of the input as if they were valid
// expressions (they might be, they might not), but ignore the
// result, this allows correctly parsing invalid media queries.
found_invalid = true;
}, },
} }
@ -266,8 +260,6 @@ pub fn parse_media_query_list(input: &mut Parser) -> MediaList {
} }
} }
debug_assert!(!found_invalid || media_queries.len() == 1);
MediaList { MediaList {
media_queries: media_queries, media_queries: media_queries,
} }

View file

@ -321,11 +321,12 @@ fn test_mq_multiple_expressions() {
#[test] #[test]
fn test_mq_malformed_expressions() { fn test_mq_malformed_expressions() {
fn check_malformed_expr(list: &MediaList, css: &str) { fn check_malformed_expr(list: &MediaList, css: &str) {
assert!(list.media_queries.len() == 1, css.to_owned()); assert!(!list.media_queries.is_empty(), css.to_owned());
let q = &list.media_queries[0]; for mq in &list.media_queries {
assert!(q.qualifier == Some(Qualifier::Not), css.to_owned()); assert!(mq.qualifier == Some(Qualifier::Not), css.to_owned());
assert!(q.media_type == MediaQueryType::All, css.to_owned()); assert!(mq.media_type == MediaQueryType::All, css.to_owned());
assert!(q.expressions.len() == 0, css.to_owned()); assert!(mq.expressions.is_empty(), css.to_owned());
}
} }
for rule in &[ for rule in &[
@ -335,8 +336,6 @@ fn test_mq_malformed_expressions() {
"@media not {}", "@media not {}",
"@media not (min-width: 300px) {}", "@media not (min-width: 300px) {}",
"@media , {}", "@media , {}",
"@media screen 4px, print {}",
"@media screen, {}",
] { ] {
test_media_rule(rule, check_malformed_expr); test_media_rule(rule, check_malformed_expr);
} }

View file

@ -11066,6 +11066,12 @@
{} {}
] ]
], ],
"css/media_bogus_query_sequence.html": [
[
"/_mozilla/css/media_bogus_query_sequence.html",
{}
]
],
"css/media_calc_crash.html": [ "css/media_calc_crash.html": [
[ [
"/_mozilla/css/media_calc_crash.html", "/_mozilla/css/media_calc_crash.html",
@ -22295,6 +22301,10 @@
"477c77a8ca848cf57b440ea7d74cc0edc97aa2cb", "477c77a8ca848cf57b440ea7d74cc0edc97aa2cb",
"support" "support"
], ],
"css/media_bogus_query_sequence.html": [
"88fb8ef33c4870966a1691f1dd992362ebe5f24e",
"testharness"
],
"css/media_calc_crash.html": [ "css/media_calc_crash.html": [
"14034fed9727ceb0c17b17b33fbc1552bc7c949e", "14034fed9727ceb0c17b17b33fbc1552bc7c949e",
"testharness" "testharness"

View file

@ -0,0 +1,16 @@
<!doctype html>
<meta charset="utf-8">
<title>CSS Test: Media query with a bogus query doesn't turn the whole media query list invalid.</title>
<style>
@media not all and (bogus), all {
#test { display: inline }
}
</style>
<p id="test"></p>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
test(function() {
assert_equals(getComputedStyle(document.getElementById('test')).display, "inline");
}, "Media query with a bogus query doesn't turn the whole media query list invalid");
</script>