541434 - incorrect parsing of invalid url() containing { or [ before what makes it invalid

Status: RESOLVED FIXED

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

Description

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

The comment I added in http://hg.mozilla.org/mozilla-central/rev/a3cda2dce88e to nsCSSParser::GetURLInParens that pushing aURL back into the buffer is unnecessary actually turns out to be wrong; yesterday I wrote a testcase in which it does make a difference:
  background: url({"")})
This is not a valid URI token according to the spec, and therefore brace-matching applies.  (brace-matching doesn't apply in a URI token, since {} and [] are allowed inside it.)

Comment 1

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

Attachment: patch

A bit ugly, but substantially less so than adding the ability to push buffers back into the tokenizer just for this, I think.

Comment 2

[Zack Weinberg (:zwol)]

You have the patch the wrong way around for the test case.

I should really get off my butt and revamp the invalid-url-token proposal as fantasai asked last month.

Comment 3

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

(In reply to comment #2)
> You have the patch the wrong way around for the test case.

No, it's removing an extra copy of the test (which is added in a different patch) that has the one failing test commented out so that the rest of the tests are still tested.

Comment 4

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

http://hg.mozilla.org/mozilla-central/rev/0ff87d9d2d73

Comment 5

[Justin Wood (:Callek)]

(In reply to comment #4)
> http://hg.mozilla.org/mozilla-central/rev/0ff87d9d2d73

Is this worth a backport to 1.9.2, and/or would it be too much trouble to do so?

I just personally helped someone who I think was bitten by this...(http://callek.pastebin.mozilla.org/699525)

Had the whole stylesheet (after this rule) missed. the Error console (at EOF) complained about invalid |background| definition, followed by unable to find a |}| before EOF.

User claimed it worked fine in 3.5.x (he even tested seconds before fixing his CSS in question)

Comment 6

[Justin Wood (:Callek)]

(In reply to comment #5)
Erm actually; I should read the bug, patch and summary before commenting; or rather just not comment when I'm overtired; please ignore my last here.

Comment 7

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

I reverted this change in bug 569646 based on the working group's decision at yesterday's teleconference.  See that bug for more details.