@namespace 'a b' results in serialization weirdness

RESOLVED FIXED in mozilla1.9.2a1

Status

()

Core
CSS Parsing and Computation
P4
minor
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: Jesse Ruderman, Assigned: dbaron)

Tracking

(Blocks: 1 bug, {testcase})

Trunk
mozilla1.9.2a1
testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

9 years ago
Created attachment 361933 [details]
testcase -- outputs PASS or FAIL
(Assignee)

Comment 1

9 years ago
Yeah, we probably need to put quotes around the contents of the url().
(Assignee)

Updated

9 years ago
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Comment 2

9 years ago
I have patches for this in my queue (two of them, though there's really more that needs to be done, although that's probably covered by other bugs), but I need to write tests for them at some later point.

(My queue is at http://hg.mozilla.org/users/dbaron_mozilla.com/patches/ , and the patches are escape-string-should-append and quote-and-escape-url .)
(Assignee)

Comment 3

9 years ago
Created attachment 365614 [details] [diff] [review]
patch 1: refactor nsStyleUtil::EscapeCSSString into nsStyleUtil::AppendEscapedCSSString

This simplifies a lot of callers of this function, including future ones (I'm going to add more, and not only in this bug).
Attachment #365614 - Flags: superreview?(bzbarsky)
Attachment #365614 - Flags: review?(bzbarsky)
(Assignee)

Comment 4

9 years ago
Created attachment 365615 [details] [diff] [review]
patch 2: serialize all url() in quoted form, and escape the strings properly

This fixes this bug.
Attachment #365615 - Flags: superreview?(bzbarsky)
Attachment #365615 - Flags: review?(bzbarsky)
Attachment #365614 - Flags: superreview?(bzbarsky)
Attachment #365614 - Flags: superreview+
Attachment #365614 - Flags: review?(bzbarsky)
Attachment #365614 - Flags: review+
Attachment #365615 - Flags: superreview?(bzbarsky)
Attachment #365615 - Flags: superreview+
Attachment #365615 - Flags: review?(bzbarsky)
Attachment #365615 - Flags: review+
Comment on attachment 365615 [details] [diff] [review]
patch 2: serialize all url() in quoted form, and escape the strings properly

> nsROCSSPrimitiveValue::GetEscapedURI(nsIURI *aURI, PRUnichar **aReturn)
>+  // XXX Should this reuse nsStyleUtil::AppendEscapedCSSString?

Please.  And get rid of this function altogether.

r+sr=bzbarsky
(Assignee)

Comment 6

9 years ago
http://hg.mozilla.org/mozilla-central/rev/5a21d2b181e9
http://hg.mozilla.org/mozilla-central/rev/1ff736fd5e41
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Priority: -- → P4
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Blocks: 535072

Comment 7

8 years ago
I just spent part of a day tracking down a problem in an older extension (I did not write the original code).  It turns out that the code assumed that getComputedStyle() would NOT return quotes within url() values but because of this bug fix that is not a valid assumption in Firefox 3.6.

I added a note to the end of this section:
  https://developer.mozilla.org/en/Firefox_3.6_for_developers#DOM

Is more documentation needed somewhere?  I suspect this change will affect other pages and extensions too....

Comment 8

8 years ago
(In reply to comment #7)
> I suspect this change will affect other pages and extensions too....

Like the N900 mobile version of Google Images. See bug 537218.

Updated

8 years ago
Blocks: 537218
No longer blocks: 537218
Depends on: 537218
You need to log in before you can comment on or make changes to this bug.