Closed Bug 914950 Opened 11 years ago Closed 10 years ago

Support ICNS favicon format

Categories

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

All
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: ckitching, Unassigned)

References

Details

Attachments

(1 file)

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 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
Attachment #802837 - Attachment description: Add support for ICN → Add support for ICNS Favicons
Attachment #802837 - Flags: review?(wjohnston)
I'm not sure we want this. Do you have any sites you know of using icns?
(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.
(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.
Attachment #802837 - Flags: review?(wjohnston)
Status: NEW → RESOLVED
Closed: 10 years ago
Component: Theme and Visual Design → Favicon Handling
Hardware: ARM → All
Resolution: --- → WONTFIX
Summary: ICNS Favicons are not supported. → Support ICNS favicon format
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.