914950 - Support ICNS favicon format

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: Favicon Handling, defect)

Description

[Chris Kitching [:ckitching]]

While rarely used, Apple's ICNS format is valid for use as a Favicon. We don't support this. We should.
Seems like a solution very like that of Bug 748100 can be used here, as it, too, is a container format.
The format is documented here:

http://en.wikipedia.org/wiki/Apple_Icon_Image_format

It may prove useful to consider this bug the task of decoding icp4, icp5, ic07, ic08, ic09, ic10, ic11, ic12, ic13, ic14 payloads, and the remainder a followup (Since the above are secretly PNG/JPG images we can just hand off to Android's BitmapFactory, similar to how the ICODecoder from Bug 748100 does with PNG payloads in Microsoft ICO files.

Comment 1

[Chris Kitching [:ckitching]]

Attachment: Add support for ICNS Favicons

Comment 2

[Chris Kitching [:ckitching]]

Comment on attachment 802837 [details] [diff] [review]
Add support for ICNS Favicons

Adding partial support for ICNS favicons (Supports JPG/PNG payload types - no attempt is made to decode the variety of (fairly) rarely used custom payload types.).

In addition, refactors the ICO decoder somewhat - groups such decoders into their own package.

Even more horrifying code to decode proprietary formats for you to review, Wes. Enjoy. :P

Comment 3

[Wesley Johnston (:wesj)]

I'm not sure we want this. Do you have any sites you know of using icns?

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Wesley Johnston (:wesj) from comment #3)
> I'm not sure we want this. Do you have any sites you know of using icns?

Correct. There is a cost, in testing, maintenance and bugs, for any code we land. We need to balance the need for a feature over that cost. Given the limited use of ICNS as favicons, I would suggest we mothball this patch.

Comment 5

[Chris Kitching [:ckitching]]

(In reply to Mark Finkle (:mfinkle) from comment #4)
> Correct. There is a cost, in testing, maintenance and bugs, for any code we
> land. We need to balance the need for a feature over that cost. Given the
> limited use of ICNS as favicons, I would suggest we mothball this patch.

After looking into it, it appears that not even Safari actually supports this, and it's an Apple proprietary format.

So. Yes. Got a wee bit carried away there. Probably worth shelving - can always revive it if ICNS favicons ever actually become a thing.