Bug 45017 – CSS MediaQuery tests fail

• Status: RESOLVED
• Resolution: CONFIGURATION CHANGED
• Component: CSS

[Simon Fraser (smfr)]

180 of the 190 tests in the url fail. Firefox passes them all.

[Simon Fraser (smfr)]

Created attachment 66503 [details]
Reduced testcase

It seems that sheet.cssRules does not include the @media at-rules in WebKit.

[Simon Fraser (smfr)]

There are several issues here.

First, we don't keep around empty CSSRules, so an empty @media rule doesn't show in sheet.cssRules. This patch fixes that:


diff --git a/WebCore/css/CSSMediaRule.cpp b/WebCore/css/CSSMediaRule.cpp
index d1c220b..9f8bb4a 100644
--- a/WebCore/css/CSSMediaRule.cpp
+++ b/WebCore/css/CSSMediaRule.cpp
@@ -40,9 +40,11 @@ CSSMediaRule::~CSSMediaRule()
     if (m_lstMedia)
         m_lstMedia->setParent(0);
 
-    int length = m_lstCSSRules->length();
-    for (int i = 0; i < length; i++)
-        m_lstCSSRules->item(i)->setParent(0);
+    if (m_lstCSSRules) {
+        int length = m_lstCSSRules->length();
+        for (int i = 0; i < length; i++)
+            m_lstCSSRules->item(i)->setParent(0);
+    }
 }
 
 unsigned CSSMediaRule::append(CSSRule* rule)
diff --git a/WebCore/css/CSSParser.cpp b/WebCore/css/CSSParser.cpp
index e20537a..4b5f620 100644
--- a/WebCore/css/CSSParser.cpp
+++ b/WebCore/css/CSSParser.cpp
@@ -5344,7 +5344,7 @@ CSSRule* CSSParser::createImportRule(const CSSParserString& url, MediaList* medi
 
 CSSRule* CSSParser::createMediaRule(MediaList* media, CSSRuleList* rules)
 {
-    if (!media || !rules || !m_styleSheet)
+    if (!media || !m_styleSheet)
         return 0;
     m_allowImportRules = m_allowNamespaceDeclarations = m_allowVariablesRules = false;
     RefPtr<CSSMediaRule> rule = CSSMediaRule::create(m_styleSheet, media, rules);


Secondly, the error handling described here:
http://dev.w3.org/csswg/css3-mediaqueries/#error-handling
is not implemented.

Even with that, I don't understand the test. It seems to test that:

@media screen, not(orientation)

(which does not parse)
should result in @media screen, but I don't see how that is correct behavior, from the spec.

[Alexey Proskuryakov]

> First, we don't keep around empty CSSRules, so an empty @media rule doesn't show in sheet.cssRules.

I think that's bug 15649.

[Simon Fraser (smfr)]

Bug 15649 is about empty sheets, not empty @ rules.

[Daniel Glazman]

> @media screen, not(orientation)
>
> (which does not parse)
> should result in @media screen, but I don't see how that is correct behavior, > from the spec.

The prose says the error handling cares about media queries. Here you
have a group of two media queries... So not(orientation) is invalid and
thrown away and screen is valid and preserved.
This was also discussed recently inside the CSS WG.

[Anne van Kesteren]

It is turned into "not all" actually. See the bit about "Malformed media query" under error handling.

[Simon Fraser (smfr)]

*** Bug 77958 has been marked as a duplicate of this bug. ***

[Rune Lillesveen]

I'm working on a patch that's for Media Queries error handling that's nearly finished. The Media Queries tests suite is using the media attribute which is currently split at commas, supporting HTML4 error handling, before fed into the CSS parser, so that needs to be fixed too.

[Ahmad Saleem]

I tested attached test case with Safari 15.5 on macOS 12.4 and it shows dialog box with "sheet has 1 rules" and all other browsers (Chrome Canary 104 and Firefox Nightly 103) also show same behavior. I think it is now fixed since 2010 and can be marked as "RESOLVED CONFIGURATION CHANGED". If I am incorrect, please ignore. Thanks!

[Alexey Proskuryakov]

AFAICT passing this test does mean that the issue is fixed. Thank you for testing!