400164 - WebRunner to use favicons if no icon is specified

Status: RESOLVED FIXED

Description

[John Munro]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.6; .NET CLR 3.5.20404) Gecko/20070725 Firefox/2.0.0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a9pre) Gecko/2007092312 Webrunner/0.7

It would be nice if WebRunner could use the web site's favicon if no icon has been specified in the ini file rather than showing the WebRunner planet icon.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 2

[Matthew Gertner]

Created attachment 294074 [details] [diff] [review]
WIP, full patch of all my changes so far including preliminary favicon support

This patch includes the code for bugs 400179 and 408362. I haven't posted the latter yet since I want to test on Windows first. Currently includes support for Windows and Mac, tested only on Mac.

Currently I download the favicon to the profile directory, which is kind of a hack, and then check whether it is there when the user clicks OK to create the app/shortcut. So if they click before the icon is downloaded they will get the default app icon. I'll figure out a way to hold the icon in memory so it doesn't have to be written to disk. The UI should take care of showing the user that the icon is loading and displaying it once it is loaded so they can decide if they want it or some other icon.

Comment 3

[Matthew Gertner]

Created attachment 294075 [details]
Icon for use with attachment 294074 [details] [diff] [review]

Comment 4

[Matthew Gertner]

Created attachment 294251 [details] [diff] [review]
Latest WIP

This is tested and works on Mac and Windows. It's ready for review. Unfortunately it's beyond my abilities to make a proper patch until bug 408362 is committed. In the meantime, here's the latest WIP. As soon as we get consensus on 408362 and get it checked in I'll make a proper patch.

Comment 5

[Matthew Gertner]

Created attachment 295772 [details] [diff] [review]
Consolidate patch to use platform-independent icons and generate from favicon if not available

Comment 6

[Matthew Gertner]

Created attachment 295777 [details] [diff] [review]
New patch, same as the old patch (but without the extraneous files)

Comment 7

[Matthew Gertner]

Created attachment 295789 [details] [diff] [review]
Fixes build issues on win32,, add supports for 256x256 and 512x512 icons

I haven't tested the 512x512 and 256x256 icon support since this is only for Leopard and I have Tiger.

Comment 8

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

Comment on attachment 295789 [details] [diff] [review]
Fixes build issues on win32,, add supports for 256x256 and 512x512 icons

tweaked common/components/src/windows/Makefile.in to use static CRT and add a manifest

Comment 9

[Matthew Gertner]

Created attachment 296114 [details] [diff] [review]
Get rid of the temporary file when generating from favicon