Last Comment Bug 454226 - Media Queries: "all," should work
: Media Queries: "all," should work
: verified1.9.1
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
P2 normal (vote)
: mozilla1.9.1b2
Assigned To: David Baron :dbaron: ⌚️UTC-8
: Jet Villegas (:jet)
Depends on: 461266
Blocks: mediaqueries
  Show dependency treegraph
Reported: 2008-09-08 08:25 PDT by Anne (:annevk)
Modified: 2010-02-07 09:41 PST (History)
15 users (show)
roc: blocking1.9.1+
mozaakash: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (11.09 KB, patch)
2008-09-19 09:54 PDT, David Baron :dbaron: ⌚️UTC-8
bzbarsky: review+
bzbarsky: superreview+
Details | Diff | Splinter Review

Description User image Anne (:annevk) 2008-09-08 08:25:59 PDT
Even though the media query list "all," is invalid, it should still apply. The second (empty) media query should be simply ignored.

Tested with: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b1pre) Gecko/20080908054322 Minefield/3.1b1pre
Comment 1 User image David Baron :dbaron: ⌚️UTC-8 2008-09-09 15:37:56 PDT
It should work for the same reason that "all, [foobar]" should work.  It's probably worth having tests for things like that too.  In particular, all of these should work:


Now I need to see which of these actually work...
Comment 2 User image David Baron :dbaron: ⌚️UTC-8 2008-09-10 13:41:28 PDT
I think the spec could be a little clearer here, though.  It currently says:

  If one media query in a comma-separated list is rejected, the other
  media queries in the list are evaluated as if the rejected media query
  had not been there.

But it doesn't really say what "rejected" means in this context, and it seems reasonable to assume that it's referring only to the possibilities mentioned just before (unknown media types, unknown media features).  If it's actually referring to all syntax errors (which I think is what you're suggesting in this bug), then it probably ought to say so.
Comment 3 User image David Baron :dbaron: ⌚️UTC-8 2008-09-10 13:46:32 PDT
And I seem to remember explicit discussion at some point saying that errors within one query in a list invalidate the whole list, which is what I implemented.

I'm going to wait for feedback before spending the time to fix this; it's not trivial.
Comment 4 User image Anne (:annevk) 2008-09-11 23:23:51 PDT
I thought we reversed that decision and decided that any kind of error would make the selector ignored. (Though still taking the braces and quotes into account. E.g., '(,(width:0),all,)' is false.
Comment 5 User image David Baron :dbaron: ⌚️UTC-8 2008-09-19 08:34:12 PDT
Ah, I see the spec now says:

  User agents are to handle unexpected tokens encountered while parsing a media
  query by reading until the end of the media query, while observing the rules
  for matching pairs of (), [], {}, "", and '', and correctly handling escapes.
  Media queries with unexpected tokens are ignored. [CSS21]
Comment 6 User image David Baron :dbaron: ⌚️UTC-8 2008-09-19 09:54:52 PDT
Created attachment 339475 [details] [diff] [review]

Here's a patch that does what you describe.

It breaks Acid3, test 46, case t.
Comment 7 User image David Baron :dbaron: ⌚️UTC-8 2008-10-08 09:14:48 PDT
Comment on attachment 339475 [details] [diff] [review]

The WG discussed this today; we'll ask Hixie to change Acid3.
Comment 8 User image Boris Zbarsky [:bz] (still a bit busy) 2008-10-08 09:46:15 PDT
Comment on attachment 339475 [details] [diff] [review]

>-  // Callers must clear or throw out aMedia if GatherMedia returns false.

Why is that no longer the case?

Other than that, looks good.
Comment 9 User image David Baron :dbaron: ⌚️UTC-8 2008-10-08 10:17:50 PDT
GatherMedia no longer propagates a "there was a parse error" state, but instead handles it internally; it returns false only for out-of-memory or other low-level scanner error.
Comment 10 User image David Baron :dbaron: ⌚️UTC-8 2008-10-11 17:52:35 PDT
Comment 11 User image Tony Chung [:tchung] 2008-10-31 17:21:12 PDT
cc'ing adam, as he is writing testcases for media query work
Comment 12 User image Aakash Desai [:aakashd] 2009-04-17 11:15:19 PDT
verified FIXED on builds:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090416 Minefield/3.6a1pre ID:20090416030845


Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090417 Shiretoko/3.5b4pre ID:20090417030722

Note You need to log in before you can comment on or make changes to this bug.