REGRESSION (257227@main): initial whitespace breaks the query in window.matchMedia()

anttijk · anttijk · commit 738835521b50 · 2023-01-25T07:53:21.000-08:00
https://bugs.webkit.org/show_bug.cgi?id=251147 rdar://104104606 Reviewed by Alan Baradlay. * LayoutTests/imported/w3c/web-platform-tests/css/mediaqueries/match-media-parsing-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/mediaqueries/match-media-parsing.html: Added. * Source/WebCore/css/query/MediaQueryParser.cpp: (WebCore::MQ::MediaQueryParser::consumeMediaQueryList): Consume initial whitespace. Parsing functions expect to start with a non-whitespace token. This was not an issue with the regular media queries because stylesheet parsing had already consumed any initial whitespace. Also fix the loop so that commas generate "not all" list items even for empty strings "foo," -> "foo, not all". Canonical link: https://commits.webkit.org/259357@main
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/mediaqueries/match-media-parsing-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/css/mediaqueries/match-media-parsing-expected.txt
@@ -0,0 +1,23 @@
+
+PASS Test parsing '' with matchMedia
+PASS Test parsing ' ' with matchMedia
+PASS Test parsing 'all' with matchMedia
+PASS Test parsing ' all' with matchMedia
+PASS Test parsing '   all   ' with matchMedia
+PASS Test parsing 'all,all' with matchMedia
+PASS Test parsing ' all , all ' with matchMedia
+PASS Test parsing '(color)' with matchMedia
+PASS Test parsing '(color' with matchMedia
+PASS Test parsing ' (color)' with matchMedia
+PASS Test parsing ' ( color  )  ' with matchMedia
+PASS Test parsing ' ( color   ' with matchMedia
+PASS Test parsing 'color)' with matchMedia
+PASS Test parsing '  color)' with matchMedia
+PASS Test parsing '  color ), ( color' with matchMedia
+PASS Test parsing ' foo ' with matchMedia
+PASS Test parsing ',' with matchMedia
+PASS Test parsing ' , ' with matchMedia
+PASS Test parsing ',,' with matchMedia
+PASS Test parsing '  ,  ,  ' with matchMedia
+PASS Test parsing ' foo,' with matchMedia
+
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/mediaqueries/match-media-parsing.html b/LayoutTests/imported/w3c/web-platform-tests/css/mediaqueries/match-media-parsing.html
@@ -0,0 +1,34 @@
+<!DOCTYPE html>
+<link rel="help" href="https://drafts.csswg.org/css-syntax-3/#parse-comma-separated-list-of-component-values">
+<script type="text/javascript" src="/resources/testharness.js"></script>
+<script type="text/javascript" src="/resources/testharnessreport.js"></script>
+
+<script>
+function test_parsing(query, expected) {
+    test(() => {
+        const match = window.matchMedia(query);
+        assert_equals(match.media, expected)
+    }, "Test parsing '" + query + "' with matchMedia");
+}
+test_parsing("", "");
+test_parsing(" ", "");
+test_parsing("all", "all");
+test_parsing(" all", "all");
+test_parsing("   all   ", "all");
+test_parsing("all,all", "all, all");
+test_parsing(" all , all ", "all, all");
+test_parsing("(color)", "(color)");
+test_parsing("(color", "(color)");
+test_parsing(" (color)", "(color)");
+test_parsing(" ( color  )  ", "(color)");
+test_parsing(" ( color   ", "(color)");
+test_parsing("color)", "not all");
+test_parsing("  color)", "not all");
+test_parsing("  color ), ( color", "not all, (color)");
+test_parsing(" foo ", "foo");
+test_parsing(",", "not all, not all");
+test_parsing(" , ", "not all, not all");
+test_parsing(",,", "not all, not all, not all");
+test_parsing("  ,  ,  ", "not all, not all, not all");
+test_parsing(" foo,", "foo, not all");
+</script>
diff --git a/Source/WebCore/css/query/MediaQueryParser.cpp b/Source/WebCore/css/query/MediaQueryParser.cpp
@@ -79,18 +79,20 @@ std::optional<MediaQuery> MediaQueryParser::parseCondition(CSSParserTokenRange r
 
 MediaQueryList MediaQueryParser::consumeMediaQueryList(CSSParserTokenRange& range)
 {
+    range.consumeWhitespace();
+
+    if (range.atEnd())
+        return { };
+
     MediaQueryList list;
 
-    while (!range.atEnd()) {
+    while (true) {
         auto begin = range.begin();
         while (!range.atEnd() && range.peek().type() != CommaToken)
             range.consumeComponentValue();
 
         auto subrange = range.makeSubRange(begin, &range.peek());
 
-        if (!range.atEnd())
-            range.consumeIncludingWhitespace();
-
         auto consumeMediaQueryOrNotAll = [&] {
             if (auto query = consumeMediaQuery(subrange))
                 return *query;
@@ -99,6 +101,10 @@ MediaQueryList MediaQueryParser::consumeMediaQueryList(CSSParserTokenRange& rang
         };
 
         list.append(consumeMediaQueryOrNotAll());
+
+        if (range.atEnd())
+            break;
+        range.consumeIncludingWhitespace();
     }
 
     return list;
