Media Queries: "all," should work

VERIFIED FIXED in mozilla1.9.1b2

Status

()

Core
CSS Parsing and Computation
P2
normal
VERIFIED FIXED
9 years ago
8 years ago

People

(Reporter: annevk, Assigned: dbaron)

Tracking

({verified1.9.1})

Trunk
mozilla1.9.1b2
verified1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

9 years ago
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
Flags: blocking1.9.1?
(Assignee)

Comment 1

9 years ago
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:

,all
all,
all,foobar
foobar,all
(foobar),all
all,(foobar)
[foobar],all
all,[foobar]

Now I need to see which of these actually work...
(Assignee)

Comment 2

9 years ago
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.
(Assignee)

Comment 3

9 years ago
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.
(Assignee)

Updated

9 years ago
Blocks: 156716
(Reporter)

Comment 4

9 years ago
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.
(Assignee)

Comment 5

9 years ago
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]
(Assignee)

Comment 6

9 years ago
Created attachment 339475 [details] [diff] [review]
patch

Here's a patch that does what you describe.

It breaks Acid3, test 46, case t.
(Assignee)

Updated

9 years ago
OS: Linux → All
Hardware: PC → All
(Assignee)

Updated

9 years ago
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
(Assignee)

Comment 7

9 years ago
Comment on attachment 339475 [details] [diff] [review]
patch

The WG discussed this today; we'll ask Hixie to change Acid3.
Attachment #339475 - Flags: superreview?(bzbarsky)
Attachment #339475 - Flags: review?(bzbarsky)
Attachment #339475 - Flags: superreview?(bzbarsky)
Attachment #339475 - Flags: superreview+
Attachment #339475 - Flags: review?(bzbarsky)
Attachment #339475 - Flags: review+
Comment on attachment 339475 [details] [diff] [review]
patch

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

Why is that no longer the case?

Other than that, looks good.
(Assignee)

Comment 9

9 years ago
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.
(Assignee)

Comment 10

9 years ago
Fixed: http://hg.mozilla.org/mozilla-central/rev/53740a23fe93
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b2
Depends on: 461266
cc'ing adam, as he is writing testcases for media query work
Keywords: fixed1.9.1
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

and

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090417 Shiretoko/3.5b4pre ID:20090417030722
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
Keywords: fixed1.9.1 → verified1.9.1

Updated

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