689319 - update to current rule for handling malformed media queries

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect, P4)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

There was a spec update to media queries that I managed to miss.  The rule for handling "Malformed media query" in section 3.1 (Error Handling) now describes an error handling behavior that invalidates the query rather than the query list.

I caught this because Arron pointed out the test was wrong in:
http://lists.w3.org/Archives/Public/public-css-testsuite/2011Sep/0042.html

Comment 1

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 562549 [details] [diff] [review]
patch

r=me

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6d1c0e678b61

Comment 3

[Matt Brubeck (:mbrubeck)]

Backed out: https://hg.mozilla.org/integration/mozilla-inbound/rev/c59e1d1d8615

Because of failures in browser_page_style_menu.js:
https://tbpl.mozilla.org/php/getParsedLog.php?id=6629138&tree=Mozilla-Inbound

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So the reason for the failures is that the patch is changing the behavior of items 15, 16, 17, and 20 in:
http://hg.mozilla.org/mozilla-central/file/d2241309d83c/browser/base/content/test/page_style_sample.html
with respect to the code here:
http://hg.mozilla.org/mozilla-central/file/d2241309d83c/browser/base/content/browser.js#l5835

Both before and after the patch, currentStyleSheet.media.mediaText is "not all".  The problem is that the patch changes currentStyleSheet.media.length from 0 to 1, which means we're now entering this nice broken code that assumes media queries don't exist:
  5843       let media = currentStyleSheet.media.mediaText.split(", ");
  5844       if (media.indexOf("screen") == -1 &&
  5845           media.indexOf("all") == -1)
  5846         continue;
and determining that the style sheet doesn't apply.


This can be tested by loading page_style_sample.html in scratchpad and running:

var links = document.getElementsByTagName("link");
var mylink = links[17];
alert(mylink.getAttribute("title") + ", " + mylink.sheet.media.length + ", " + mylink.sheet.media.mediaText);

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: make page style menu use window.matchMedia

This improves on bug 498312 by using window.matchMedia (which didn't exist then).  (It changes the test to make the failures that the other patch on this bug caused be the new correct behavior -- and then adds some additional tests.)

Comment 6

[Dão Gottwald [:dao]]

Comment on attachment 567455 [details] [diff] [review]
make page style menu use window.matchMedia

>+    // [...]  Note
>+    // that this assumes that the media queries are static, which isn't
>+    // quite true.  (For example, resizing the window can change which
>+    // style sheets apply.)

Does this actually matter? Manually resizing the window would close the page style menu, reopening it would call stylesheetFillPopup again.

>     if (currentStyleSheet.media.length > 0) {
>-      let media = currentStyleSheet.media.mediaText.split(", ");
>-      if (media.indexOf("screen") == -1 &&
>-          media.indexOf("all") == -1)
>+      var mediaQueryList = currentStyleSheet.media.mediaText;

nit: let mediaQueryList ...

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to Dão Gottwald [:dao] from comment #6)
> Comment on attachment 567455 [details] [diff] [review] [diff] [details] [review]
> make page style menu use window.matchMedia
> 
> >+    // [...]  Note
> >+    // that this assumes that the media queries are static, which isn't
> >+    // quite true.  (For example, resizing the window can change which
> >+    // style sheets apply.)
> 
> Does this actually matter? Manually resizing the window would close the page
> style menu, reopening it would call stylesheetFillPopup again.

Ah, in that case, it doesn't matter.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7ed6db7aab4f
https://hg.mozilla.org/integration/mozilla-inbound/rev/6c9a00ca9d5c

Comment 9

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/7ed6db7aab4f
https://hg.mozilla.org/mozilla-central/rev/6c9a00ca9d5c