169477 - favicon color palette reversed or too red

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect, P1)

Description

[John "AntiTuX" Dee]

favicons on most sites are getting their colors reversed by netscape, and
showing up incorrectly on this morning's builds.

I've so far seen this behaviour on the following sites:
http://home.netscape.com
http://my.netscape.com
http://www.yahoo.com
http://www.cnn.com
http://antitux.net

Comment 1

[Malcolm Rowe]

*** Bug 169478 has been marked as a duplicate of this bug. ***

Comment 2

[Malcolm Rowe]

Comments from dupe:

Lot of color problems with favicons with today build.
http://www.cnn.com/ (all blue)
http://www.cbc.ca/  (all blue)
http://www.google.com/
favicon are looking weird...

http://bugzilla.mozilla.org/ is OK.

Perhaps related to bug 116808

Mozilla 1.2b
Mozilla/5.0 (Macintosh; U; PPC; en-US; rv:1.2b) Gecko/20020918

Comment 3

[Götz Waschk]

I can verify this on Solaris Sparc 2.7 with the 2002091822 nightly build.
http://www.heise.de/favicon.ico should be blue but it's red.

This could be big-endian issue.

Comment 4

[John "AntiTuX" Dee]

I'm seeing the exact opposite on windows, linux, macosx, and macos9.
everything's blue.

Comment 5

[Frankie]

More messed up favicons in nightly 20020918. I'm using OS X 10.2.

http://slashdot.org has turned an ugly green-brown, and my 5 color apple icon at
http://uyf.tripod.com has gone sour.

Comment 6

[hub-san]

On Linux, BuildID:2002091721 is OK.  But, BuildID:2002091804 is weird.  I think
that the difference among several hours caused this problem.  Regression of bug
168048?(See comment #17)

Comment 7

[hub-san]

Attachment: Screenshot(comparing buildID:2002091721 and 2002091804)

Comment 8

[Jeremy M. Dolan]

I see both effects described in comment 5, but yahoo and cnn don't load favicons
for me, with today or yesterdays build. Linux x86 (so it's not endian, it seems).

Comment 9

[Benjamin Mucci]

If favicons don't load add user_pref("browser.chrome.favicons", true); in your
prefs...

Comment 10

[Olivier Cahagne]

*** Bug 169855 has been marked as a duplicate of this bug. ***

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Maybe a regression of bug 168839.

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

grrrrr
it's a regression of bug 168048 in combination with bug 168839
alecf was moving a define to the .cpp file, where it was used before 168839's patch
after that patch, it was used in the .h file...

will attach a patch in a second

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Comment 14

[Christian :Biesinger (don't email me, ping me on IRC)]

alecf, could you r= (or sr=) this patch, as your patch caused this regression?
thanks

Comment 15

[Alec Flett]

the reason I moved it out of the header file was because GFXFORMAT was being
used by other header files (I think I moved one out of jpeg as well) and they
were conflicting

please either call this BMP_GFXFORMAT or move the function out of the header.

Comment 16

[Christian :Biesinger (don't email me, ping me on IRC)]

it was used in XBM as well but there it was moved to the C++ file.
I can't move this out of the header. the functions are |inline| functions. (hm,
that reminds me, gotta check if the inlining is worth it)

but sure, I can rename that

Comment 17

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: alternative

let me clarify my earlier comment. they are not only inline, but used in two
different .cpp files. so they must stay in the header or be made non-inline.

Comment 18

[Alec Flett]

Comment on attachment 100384 [details] [diff] [review]
alternative

sr=alecf

Comment 19

[ArronM (:paper)]

Comment on attachment 100384 [details] [diff] [review]
alternative

r=paper

Comment 20

[Christian :Biesinger (don't email me, ping me on IRC)]

checked in.

Comment 21

[Benjamin Mucci]

Fixed in

Mozilla 1.2b
Mozilla/5.0 (Macintosh; U; PPC; en-US; rv:1.2b) Gecko/2002092505

Mac8.6.