701432 - Add support for fave icons on jump list uri entries.

Status: RESOLVED FIXED

(SeaMonkey :: OS Integration, defect)

Description

[Philip Chee]

Bug 549472 added support for fave icons on jump list uri entries to Firefox.

Comment 1

[Philip Chee]

Attachment: Patch v0.1 Part 1: Whitespace cleanup and remove DOS line endings. [Check-in comment 8]

Convert line endings to unix \x0A, and remove trailing whitespace.

Comment 2

[Philip Chee]

Attachment: Patch v1.0 Part 2: Support Favicons.

Actual changes.

Comment 3

[[e-mail]]

Comment on attachment 573561 [details] [diff] [review]
Patch v0.1 Part 1: Whitespace cleanup and remove DOS line endings. [Check-in comment 8]

r+ based on -w diff.

Comment 4

[[e-mail]]

Comment on attachment 573564 [details] [diff] [review]
Patch v1.0 Part 2: Support Favicons.

Seems reasonable, given that I don't have a W7 system to try this on, so you should try to find someone else who does and get f+ from them.

Comment 5

[Justin Wood (:Callek)]

Comment on attachment 573564 [details] [diff] [review]
Patch v1.0 Part 2: Support Favicons.

If you find someone else to steal this feedback request from me, great. but since neil seemed to require it as part of his review, I'm stuffing it at myself, since I do have w7

Comment 6

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 573561 [details] [diff] [review]
Patch v0.1 Part 1: Whitespace cleanup and remove DOS line endings. [Check-in comment 8]

Warning: "hg qimport bz://701432", if used on Windows, screws up the line ends (filed bug 701667 for that). The resulting patch cannot be applied. You need to download this patch manually.

Comment 7

[Philip Chee]

Comment on attachment 573564 [details] [diff] [review]
Patch v1.0 Part 2: Support Favicons.

Frank, Neil doesn't have a Win7 box to test this patch. If you have one, could you see if this works for you?

Comment 8

[Philip Chee]

Comment on attachment 573561 [details] [diff] [review]
Patch v0.1 Part 1: Whitespace cleanup and remove DOS line endings. [Check-in comment 8]

While waiting for feedback on Part 2, I'm checking in Part 1.
http://hg.mozilla.org/comm-central/rev/24c90a00c902

Comment 9

[Jens Hatlak (:InvisibleSmiley)]

I haven't tried this yet, but one possible way to get a working test Win 7 legally is to install MS Virtual PC and get an image from here:

http://www.microsoft.com/download/en/details.aspx?id=11575

Unlike the Win XP images, the Win 7 ones do not expire (as is explained on the page) but can only be prolonged twice for a total of 90 days, so keep the original images if you want to test again later and don't want to download again (needs setting up again from scratch then, but that's intended behavior).

Comment 10

[Philip Chee]

Attachment: Patch v1.0a Part 2: Support Favicons (un-bitrotted). r=Neil

In order to see the changed behaviour of this patch you must clear the Windows 7 icon cache:

Rebuild the Icon Cache, Method 1.

http://www.winhelponline.com/blog/how-to-rebuild-the-icon-cache-in-windows-vista/

How to Rebuild the Icon Cache in Windows Vista and Windows 7

Note: %userprofile% represents the path to user profile folder.

1. Close all folder windows that are currently open.
2. Launch Task Manager using the CTRL+SHIFT+ESC key sequence, or by running taskmgr.exe.
3. In the Process tab, right-click on the Explorer.exe process and select End Process.
4. Click the End process button when asked for confirmation.
5. From the File menu of Task Manager, select New Task (Run…)
6. Type CMD.EXE, and click OK
7. In the Command Prompt window, type the commands one by one and press ENTER after each command:

    CD /d %userprofile%\AppData\Local
    DEL IconCache.db /a
    EXIT

8. In Task Manager, click File, select New Task (Run…)
9. Type EXPLORER.EXE, and click OK.

Rebuild the Icon Cache, Method 2.

http://social.technet.microsoft.com/Forums/en-US/w7itproui/thread/bea47202-d869-4155-8c8f-2a5b8bd7be1d/#d88bbf39-4e06-4b6b-bd5e-179a615f2bbb

Another way to rebuild the icon cache in Windows Vista and Windows 7 w/o a restart is to change momentarily the screen color depth to 16 bits, for example, and, when Windows asks you whether you want to keep the changes or not, click "No" to restore the original settings. This will invalidate the icon cache and Windows will recreate it instantly.

Comment 11

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 617457 [details] [diff] [review]
Patch v1.0a Part 2: Support Favicons (un-bitrotted). r=Neil

As I already told you on IRC I had given the IE App Compat VPC approach a try but didn't succeed, then found and fixed the bug 743692 regression but still had no luck in getting any taskbar lists like Frequent above the Tasks list - neither with a current FF trunk nightly, SM trunk nightly, or custom (self-built) SM trunk nightly with this patch.

On my host however (Win 7 x64), the Frequent list was there (in fact has been for a long time already). I found that it contained the contents of the Most Visited pseudo-folder (located under the Bookmarks Toolbar pseudo-folder), limited to 7 (which equals the browser.taskbar.lists.maxListItemCount pref default value).

Now, since you fixed bug 747774, the mail icons in front of the list entries have been replaced by HTML file icons (nice!). This I could reproduce both with a current trunk nightly and my custom build.

With this patch however, I finally got favicons for some of the entries. I cannot tell for sure what the logic behind additions or updates to the list is (tried your Clear Icon Cache method 2, loading pages, reloading pages, and removing entries from Most Visited using the BM), and as I said I still cannot reproduce any of this using my test environment VM. But it's certainly enough for me to give you f+ for this patch since it does what it's supposed to do. If the initial taskbar list addition is broken, that's certainly another bug.

Good job! :-)

Comment 12

[Philip Chee]

Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/c6359e5dddab

Comment 13

[Frank Wein [:mcsmurf]]

Comment on attachment 617457 [details] [diff] [review]
Patch v1.0a Part 2: Support Favicons (un-bitrotted). r=Neil

Favicons seem to work fine on Windows 7. Maybe we should check if the installer can force a rebuild of the icon cache as otherwise the user likely will not notice this change and instead still see the old mail icon there.

Comment 14

[Jens Hatlak (:InvisibleSmiley)]

(In reply to Frank Wein [:mcsmurf] from comment #13)
> Favicons seem to work fine on Windows 7. Maybe we should check if the
> installer can force a rebuild of the icon cache as otherwise the user likely
> will not notice this change and instead still see the old mail icon there.

Sure, if you can do it (I can't). Otherwise we might want to just add to Known Issues (relnote).

I haven't actually tried any of these, but the following blog post suggests the issue might fix itself automatically after 120 seconds (browser.taskbar.lists.refreshInSeconds I guess) or restarting explorer.exe:

http://msujaws.wordpress.com/2012/05/01/task-specific-icons-for-windows-7-jumplists/

[We haven't ported the task icons change that is discussed there, but I think the measures suggested there apply to our case, too.]