110077 - Empty or invalid favicon.ico

Status: RESOLVED WORKSFORME

(Core :: Graphics: ImageLib, defect)

Description

[logan]

Empty or invalid favicon.ico caused no icon to be displayed in the location bar.

I had just grabbed latest nightly and loaded it up. Went to a few sites, and
noticed that CNN had it's own icon. So I just continued working and hit a web
server at work with an empty favicon.ico in /, no idea why it was there, but
regardless...

Then created an empty file on my machine (url above), and again mozilla
displayed nothingness.

I can't seem to reproduce the error anymore. :/ A regular icon appears, but I
thought I'd submit anyway.

Comment 1

[Boris Zbarsky [:bzbarsky]]

hyatt

Comment 2

[John Morrison]

I can't reproduce a missing icon with the given url. There are a few 
cases though where it does drop out. I'll note that one quirk for 
http://beer.dct.com/ is that a request for http://beer.dct.com/favicon.ico
returns a '301 Moved Permanently' with 'Location: http://beer.dct.com', which
results in a '200 OK' with the contents of http://beer.dct.com/

Comment 3

[SineSwiper]

According to the MS specs, it suggests to create a zero-length favicon.ico, if
you want to stop having 404s show up on your web logs.  Is this related, and
should we  change the behavior to display the generic bookmark icon if it's 0 bytes?

Comment 4

[Aleksey Nogin]

This sounds like either a WFM or a dup of bug 109959 or bug 109112. Can anybody
still reproduce this bug? I can't.

A clean reproduction (that is not a dup of 109959) should probably involve
1) Clean all caches
2) Go to some URL and see no favicon

Anobody know a URL that acts like that?

Comment 5

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

Alexsey, no, it's not. This bug is that no icon is displayed if the favicon is
an invalid icon.

Comment 6

[Matthew Miller]

This seems to be a dupe of bug #115357, which has a suggested patch....

Comment 7

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

Matthew, I don't think so. That bug is for a specific kind of invalid icons.
This is for the general case.

Comment 8

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

taking, I have an idea that might work

Comment 9

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

Setting Target Milestone, Platform/OS All, and accepting

Comment 10

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

really accepting this time

Comment 11

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

Attachment: Suggested Patch

This should fix it

Comment 12

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

actually, the two comment lines in nsICODecoder::Flush (// Set Data here...)
should also be removed, they don't really apply here anymore. They data needs to
be set after the initializations.

Comment 13

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

Attachment: Working patch

OK, this patch does not prevent any icon from showing up.

Comment 14

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

putting testcase in url field

Comment 15

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

Comment on attachment 65828 [details] [diff] [review]
Working patch

no, this doesn't work either

Comment 16

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

Back to hyatt. There must be some XUL magic going on that I don't understand...

Comment 17

[kitchin]

In case you're not aware of this, many servers present favicon.ico as
Content-Type: text/plain
instead of
Content-Type: image/x-icon 
Just mentioning this in case it's related.

Also, Phoenix 0.5 (Win98) differs from IE6 in one way:
If you go directly to the URL of a text/plain favicon.ico, you will see the text
instead of the image:
http://www.weather.gov/favicon.ico
http://www.timeanddate.com/favicon.ico
http://www.humanclock.com/favicon.ico 
http://nytimes.com/favicon.ico 

If you're keeping score, these are image/x-icon:
http://www.yahoo.com/favicon.ico
http://www.washingtonpost.com/favicon.ico
http://www.latimes.com/favicon.ico
http://news.google.com/favicon.ico

I don't see a <link ...> override in any of these examples.

Apache has relented and included image/x-icon as default for *.ico in ver 2.*:
http://nagoya.apache.org/bugzilla/show_bug.cgi?id=10993 

Comment 18

[Michael Kohler [:mkohler]]

This is a mass change. Every comment has "assigned-to-new" in it.

I didn't look through the bugs, so I'm sorry if I change a bug which shouldn't be changed. But I guess these bugs are just bugs that were once assigned and people forgot to change the Status back when unassigning.

Comment 19

[Kelsey Gilbert [:jgilbert]]

Nine years on and 6.5 to 9.5 versions, and a new name later, test case fails to reproduce.