Last Comment Bug 701432 - Add support for fave icons on jump list uri entries.
: Add support for fave icons on jump list uri entries.
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: OS Integration (show other bugs)
: Trunk
: x86 Windows 7
: -- normal (vote)
: seamonkey2.12
Assigned To: Philip Chee
:
Mentors:
Depends on:
Blocks: 566138
  Show dependency treegraph
 
Reported: 2011-11-10 10:18 PST by Philip Chee
Modified: 2012-05-06 10:50 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch v0.1 Part 1: Whitespace cleanup and remove DOS line endings. [Check-in comment 8] (43.73 KB, patch)
2011-11-10 10:25 PST, Philip Chee
neil: review+
Details | Diff | Splinter Review
Patch v1.0 Part 2: Support Favicons. (5.10 KB, patch)
2011-11-10 10:29 PST, Philip Chee
neil: review+
Details | Diff | Splinter Review
Patch v1.0a Part 2: Support Favicons (un-bitrotted). r=Neil (5.10 KB, patch)
2012-04-23 06:28 PDT, Philip Chee
bugzilla: feedback+
jh: feedback+
Details | Diff | Splinter Review

Description Philip Chee 2011-11-10 10:18:06 PST
Bug 549472 added support for fave icons on jump list uri entries to Firefox.
Comment 1 Philip Chee 2011-11-10 10:25:33 PST
Created attachment 573561 [details] [diff] [review]
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 2011-11-10 10:29:43 PST
Created attachment 573564 [details] [diff] [review]
Patch v1.0 Part 2: Support Favicons.

Actual changes.
Comment 3 neil@parkwaycc.co.uk 2011-11-10 15:58:47 PST
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 neil@parkwaycc.co.uk 2011-11-10 16:09:29 PST
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) 2011-11-10 22:49:10 PST
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) 2011-11-11 04:31:34 PST
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 2011-12-01 07:56:24 PST
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 2012-01-17 01:09:47 PST
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) 2012-04-22 11:37:50 PDT
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 2012-04-23 06:28:39 PDT
Created attachment 617457 [details] [diff] [review]
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) 2012-04-25 14:48:01 PDT
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 2012-04-26 00:39:22 PDT
Pushed to comm-central
http://hg.mozilla.org/comm-central/rev/c6359e5dddab
Comment 13 Frank Wein [:mcsmurf] 2012-04-27 15:59:08 PDT
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) 2012-05-01 10:32:57 PDT
(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.]

Note You need to log in before you can comment on or make changes to this bug.