Commit 99589ad5 authored by Rune Lillesveen's avatar Rune Lillesveen Committed by Commit Bot

Remove sorting and de-duplication in media queries.

This was removed from the spec per resolution[1] and incompatible with
never media queries. Improves interop with Gecko which have not seen any
issues with the different serialization.

Removed fast/media test which is covered by existing wpt tests.

[1] https://github.com/w3c/csswg-drafts/issues/5627#issuecomment-712475204

Bug: 1138859
Change-Id: I1483008c81df90f8277dcad7e90c8036c5cc019b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2478992Reviewed-by: default avatarXiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819090}
parent ba2e367b
......@@ -69,10 +69,6 @@ String MediaQuery::Serialize() const {
return result.ToString();
}
static bool ExpressionCompare(const MediaQueryExp& a, const MediaQueryExp& b) {
return CodeUnitCompare(a.Serialize(), b.Serialize()) < 0;
}
std::unique_ptr<MediaQuery> MediaQuery::CreateNotAll() {
return std::make_unique<MediaQuery>(MediaQuery::kNot, media_type_names::kAll,
ExpressionHeapVector());
......@@ -83,20 +79,7 @@ MediaQuery::MediaQuery(RestrictorType restrictor,
ExpressionHeapVector expressions)
: restrictor_(restrictor),
media_type_(AttemptStaticStringCreation(media_type.LowerASCII())),
expressions_(std::move(expressions)) {
std::sort(expressions_.begin(), expressions_.end(), ExpressionCompare);
// Remove all duplicated expressions.
MediaQueryExp key = MediaQueryExp::Invalid();
for (int i = expressions_.size() - 1; i >= 0; --i) {
MediaQueryExp exp = expressions_.at(i);
CHECK(exp.IsValid());
if (exp == key)
expressions_.EraseAt(i);
else
key = exp;
}
}
expressions_(std::move(expressions)) {}
MediaQuery::MediaQuery(const MediaQuery& o)
: restrictor_(o.restrictor_),
......
......@@ -59,8 +59,7 @@ TEST(MediaQuerySetTest, Basic) {
{"(example, all,), speech", "not all, speech"},
{"&test, screen", "not all, screen"},
{"print and (min-width: 25cm)", nullptr},
{"screen and (min-width: 400px) and (max-width: 700px)",
"screen and (max-width: 700px) and (min-width: 400px)"},
{"screen and (min-width: 400px) and (max-width: 700px)", nullptr},
{"screen and (device-width: 800px)", nullptr},
{"screen and (device-height: 60em)", nullptr},
{"screen and (device-height: 60rem)", nullptr},
......
......@@ -27,8 +27,7 @@ TEST(MediaConditionParserTest, Basic) {
{"(min-width:500px)", "(min-width: 500px)"},
{"(min-width: -100px)", "not all"},
{"(min-width: 100px) and print", "not all"},
{"(min-width: 100px) and (max-width: 900px)",
"(max-width: 900px) and (min-width: 100px)"},
{"(min-width: 100px) and (max-width: 900px)", nullptr},
{"(min-width: [100px) and (max-width: 900px)", "not all"},
{"not (min-width: 900px)", "not all and (min-width: 900px)"},
{"not (blabla)", "not all"},
......@@ -44,7 +43,9 @@ TEST(MediaConditionParserTest, Basic) {
nullptr);
ASSERT_EQ(media_condition_query_set->QueryVector().size(), (unsigned)1);
String query_text = media_condition_query_set->QueryVector()[0]->CssText();
ASSERT_EQ(test_cases[i].output, query_text);
const char* expected_text =
test_cases[i].output ? test_cases[i].output : test_cases[i].input;
ASSERT_EQ(expected_text, query_text);
}
}
......
......@@ -40,19 +40,17 @@
mediaList = styleSheet.media;
}
// First explicit example input (first column) and output (second column) in specification.
test(function() {
setupMedia('not screen and (min-WIDTH:5px) AND (max-width:40px )');
assert_equals(mediaList.mediaText, "not screen and (max-width: 40px) and (min-width: 5px)");
assert_equals(mediaList.mediaText, "not screen and (min-width: 5px) and (max-width: 40px)");
}, "mediatest_mediaquery_serialize_1");
// Second explicit example input (first column) and output (second column) in specification.
test(function() {
setupMedia('all and (color) and (color) ');
assert_equals(mediaList.mediaText, "(color)");
assert_equals(mediaList.mediaText, "(color) and (color)");
}, "mediatest_mediaquery_serialize_2");
......
......@@ -132,9 +132,9 @@ test(function(t) {
);
assert_equals(
sheet.cssRules[1].cssText,
'@media screen and (color) and (max-width: 0px) {\n}'
'@media screen and (max-width: 0px) and (color) {\n}'
);
}, 'features - lexicographical sorting');
}, 'features - no lexicographical sorting');
test(function(t) {
var sheet = makeSheet(t);
......
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
<html>
<head>
<link rel="help" href="http://dev.w3.org/csswg/cssom/" />
<style type="text/css" media="NOT braille, tv and (orientation: landscape) AND (min-WIDTH:100px) and (max-width: 200px ), all and (color) and (color)">
</style>
<script src="../../resources/js-test.js"></script>
</head>
<body>
<script>
description(
'Test media query serialization. <a href="https://bugs.webkit.org/show_bug.cgi?id=39220">https://bugs.webkit.org/show_bug.cgi?id=39220</a>'
);
var expected = "not braille, tv and (max-width: 200px) and (min-width: 100px) and (orientation: landscape), (color)";
shouldBe("document.styleSheets[0].media.mediaText", "expected");
</script>
</body>
</html>
......@@ -33,22 +33,6 @@ mediaRule #1
unit: em
computed length: 1152
mediaRule #2
text: (max-width: 10px) and (min-width: 20px)
source: mediaRule
range: {"endColumn":46,"endLine":13,"startColumn":7,"startLine":13}
computedText: (min-width: 20px) and (max-width: 10px)
mediaQuery #0 active: false
mediaExpression #0
feature: max-width
value: 10
unit: px
computed length: 10
mediaExpression #1
feature: min-width
value: 20
unit: px
computed length: 20
mediaRule #3
text: (max-width: 200px) and (min-width: 100px)
source: mediaRule
range: {"endColumn":52,"endLine":6,"startColumn":11,"startLine":6}
......@@ -64,7 +48,7 @@ mediaRule #3
value: 100
unit: px
computed length: 100
mediaRule #4
mediaRule #3
text: (min-monochrome: 8)
source: mediaRule
range: {"endColumn":38,"endLine":22,"startColumn":11,"startLine":22}
......@@ -74,7 +58,7 @@ mediaRule #4
feature: min-monochrome
value: 8
unit:
mediaRule #5
mediaRule #4
text: (min-width: 100px)
source: mediaRule
range: {"endColumn":25,"endLine":4,"startColumn":7,"startLine":4}
......@@ -85,7 +69,7 @@ mediaRule #5
value: 100
unit: px
computed length: 100
mediaRule #6
mediaRule #5
text: (min-width: 100px)
source: mediaRule
range: {"endColumn":25,"endLine":4,"startColumn":7,"startLine":4}
......@@ -96,7 +80,7 @@ mediaRule #6
value: 100
unit: px
computed length: 100
mediaRule #7
mediaRule #6
text: (min-width: 100px)
source: mediaRule
range: {"endColumn":25,"endLine":4,"startColumn":7,"startLine":4}
......@@ -107,7 +91,7 @@ mediaRule #7
value: 100
unit: px
computed length: 100
mediaRule #8
mediaRule #7
text: (min-width: 1px), (max-width: 1000em)
source: mediaRule
range: {"endColumn":1,"endLine":9,"startColumn":7,"startLine":1}
......@@ -124,6 +108,22 @@ mediaRule #8
value: 1000
unit: em
computed length: 16000
mediaRule #8
text: (min-width: 20px) and (max-width: 10px)
source: mediaRule
range: {"endColumn":46,"endLine":13,"startColumn":7,"startLine":13}
computedText: (min-width: 20px) and (max-width: 10px)
mediaQuery #0 active: false
mediaExpression #0
feature: min-width
value: 20
unit: px
computed length: 20
mediaExpression #1
feature: max-width
value: 10
unit: px
computed length: 10
mediaRule #9
text: (orientation: landscape), handheld and (max-resolution: 3dppx)
source: importRule
......@@ -186,22 +186,6 @@ mediaRule #15
computedText: screen and (device-aspect-ratio: 16/9), screen and (device-aspect-ratio: 16/10)
mediaList is empty
mediaRule #16
text: screen and (max-height: 4000px) and (min-width: 10px)
source: importRule
range: {"endColumn":42,"endLine":1,"startColumn":37,"startLine":0}
computedText: screen and\n(min-width: 10px) and (max-height: 4000px)
mediaQuery #0
mediaExpression #0
feature: max-height
value: 4000
unit: px
computed length: 4000
mediaExpression #1
feature: min-width
value: 10
unit: px
computed length: 10
mediaRule #17
text: screen and (min-resolution: 2dppx)
source: mediaRule
range: {"endColumn":41,"endLine":7,"startColumn":7,"startLine":7}
......@@ -211,4 +195,20 @@ mediaRule #17
feature: min-resolution
value: 2
unit: dppx
mediaRule #17
text: screen and (min-width: 10px) and (max-height: 4000px)
source: importRule
range: {"endColumn":42,"endLine":1,"startColumn":37,"startLine":0}
computedText: screen and\n(min-width: 10px) and (max-height: 4000px)
mediaQuery #0
mediaExpression #0
feature: min-width
value: 10
unit: px
computed length: 10
mediaExpression #1
feature: max-height
value: 4000
unit: px
computed length: 4000
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment