203256 - [FIXr]incorrect color for background, shows white instead of black.

Status: RESOLVED FIXED

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

Description

[Dustin Bruce]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4a) Gecko/20030401
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4a) Gecko/20030401

The background color at http://www.chatarea.com/tgmod is black, but Mozilla is
displaying it as white, it's even showing white if you view the source. The page
in question is a forum of mine, and i know the background color is black.

Reproducible: Always

Steps to Reproduce:
1. goto the page.
2.
3.

Actual Results:  
White background instead of black.

Expected Results:  
Displayed black background instead of white(incorrect) color.

Bug occurs with any theme i've used... including the default. Mozilla is a ton
better than IE i think, but i don't like the preferences for displaying
different fonts 'n such. I've tried using the preferences to use the webpages
attributes, but the fonts and the background for at least this one site are changed.

Comment 1

[Boris Zbarsky [:bzbarsky]]

The page has the following CSS:

  background: #000000 url();

We treat 'url()' as an invalid token and drop the whole declaration.

Looking at the CSS2 grammar, this seems to be a valid URL token, though....

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: testcase

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch in case we want to change this behavior

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 121602 [details] [diff] [review]
Patch in case we want to change this behavior

David, are we confused, or the CSS2 grammar?

Comment 5

[Hixie (not reading bugmail)]

url() is (or, if that is being debated, should be IMHO) valid, and represents
the URL "". I _think_ that is relative URL that points to the stylesheet. Which
in this case is an invalid image (it's text/css and we don't support that (yet)
as a background image format). It doesn't have to be though, it is quite
possible that a server would serve up an image/png file if the stylesheet's
referrer is a stylesheet. (Do we do that? Or do we set the referrer to the
document?)

Hmm, this gives me evil ideas... ;-)

Comment 6

[Boris Zbarsky [:bzbarsky]]

We set the referrer to the document at the moment.  But that will likely change,
hopefully in the 1.5-1.6 timeframe; then the referrer will be the stylesheet.

Another interesting question, Ian, somewhat offtopic.  What happens if we take
something like:

url("http://foo%$^%$^%$^54654654654");

(as in, the string is something that Necko will absolutely not convert into a
URI object and will report a malformed URI; note that this will only happen for
protocol schemes that Necko 'knows', like http).  Should we treat that as an
invalid decl or no?  In particular, note the check we do at
http://lxr.mozilla.org/seamonkey/source/content/html/style/src/nsCSSScanner.cpp#638
-- it seems odd to do that check (which will cause the decl to be dropped) but
not other URI validity checks...  Specifically, we will drop the following at
the moment:

url(aaa"bbb)
url(aaa(bbb)
url(aaa bbb)

(note the lack of quotes around the URI value; if quotes are present, all is good).

Comment 7

[Hixie (not reading bugmail)]

If it matches the grammar, it should be accepted at the parser level, and then
fail in the computed value stage.

  INPUT                                  PARSE AS
  -------------------------------------  -------------------------------------
  url("http://foo%$^%$^%$^54654654654")  url() token
  url(aaa"bbb)                           identifier, unterminated parenthesis-
                                         delimitted section containing an
                                         identifier and an unterminated string
                                         token.
  url(aaa(bbb)                           identifier, unterminated parenthesis-
                                         delimitted section containing an
                                         identifier followed by a parenthesis-
                                         delimitted section containing an
                                         identifier.
  url(aaa bbb)                           identifier followed by a parenthesis-
                                         delimitted section containing two
                                         identifiers.

Thus the first is valid, and the last three cause the declaration to be dropped.

Comment 8

[Boris Zbarsky [:bzbarsky]]

OK.  So the computed value in that first case should be

  url("http://foo%$^%$^%$^54654654654")

and we should just show no image.  Right?

Comment 9

[Hixie (not reading bugmail)]

Yes.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Taking, I guess... 

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 121602 [details] [diff] [review]
Patch in case we want to change this behavior

Could this please be approved for 1.4?	This is a very low-risk change that
just makes 'url()' (with nothing between the parentheses) a valid URL token in
CSS; this will not affect parsing of pages that do not use that construct and
on pages that do use it the patch will make us compatible with both IE/Win32
and the CSS spec.

Comment 12

[Brendan Eich [:brendan]]

Comment on attachment 121602 [details] [diff] [review]
Patch in case we want to change this behavior

Sure, why not?	a=brendan for 1.4final.

Why is Pushback(ch) called first thing in the then and else clauses, instead of
before the if (ch ==')') condition?  Never mind.

/be

Comment 13

[Boris Zbarsky [:bzbarsky]]

Checked in for 1.4. I'm not sure whether it's more readable to have the
Pushback() hoisted outside the if(), but we may well want to do that at some
point...