Closed Bug 1150534 Opened 9 years ago Closed 9 years ago

Favicons no longer displayed in jump list

Categories

(Core :: Widget: Win32, defect, P3)

37 Branch
x86
Windows 7
defect

Tracking

()

VERIFIED FIXED
mozilla41
Tracking Status
firefox38 + wontfix
firefox39 + wontfix
firefox40 + verified
firefox41 --- verified

People

(Reporter: vertova, Assigned: seth)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(5 files)

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:37.0) Gecko/20100101 Firefox/37.0
Build ID: 20150326190726

Steps to reproduce:

I just upgraded to FF 37 and noticed that the uri entries in Win7 jump list no longer display the favicon but the default Firefox icon


Actual results:

When using the default profile, .ico files in the jumpListCache subdir of the user profile directory are deleted. When starting with a fresh profile, that subdir is not even created.


Expected results:

In previous versions of FF, uri entries in the jump list would display their favicons; favicons would be saved in the jumpListCache subdir
ugh, I can reproduce in Nightly. interesting that nobody noticed.

I don't know if this is intended... Jim?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(jmathies)
(In reply to Marco Bonardo [::mak] from comment #1)
> ugh, I can reproduce in Nightly. interesting that nobody noticed.
> 
> I don't know if this is intended... Jim?

not that I'm aware of. I can reproduce as well.
Flags: needinfo?(jmathies)
Flags: qe-verify+
Flags: firefox-backlog+
Keywords: regression
This is on Aurora 38.0a2 (2015-03-30), which I'm pretty sure is now Beta. The good news it's not on 36, however I wasn't able to test 37.
Tracking for 38+.  It does sound like we'd want to uplift this to beta if possible. 

Florin can you test this for 37 on Win7?
Flags: needinfo?(florin.mezei)
I'm not quite sure what the actual issue is here. I tested a bit today on Windows 7 x64, and got some rather weird results, as favicons seemed to appear and disappear in a random manner.

To illustrate this, see the following cases with attached screenshots:

-= Firefox 37.0.1 =-
- opened with clean profile
- opened some pages, then right-clicked the Firefox icon in the bar
===> the jump list displayed entries with firefox icon
- did some additional navigation in Firefox, then right-clicked the Firefox icon again
===> the jump list now displayed favicons - jump_list_fav_icons_37_1.png
- closed 37.0.1 to do some other checking in other builds (e.g. DevEdition and Nightly), then reopened 37.0.1 (restored previous session) and right-clicked the Firefox icon again
===> the jump list now displayed favicons only for some entries - jump_list_fav_icons_37_2.png

-= latest Firefox 40 Nightly =-
- opened with same profile as above and restored session profile from above
- right-clicked the Nightly icon in the bar
===> the jump list displayed entries with nightly icon - jump_list_nightly_fav_icons_1.png
- did some additional navigation in Firefox, then right-clicked the Firefox icon again
===> the jump list now displayed favicons only for some entries - jump_list_nightly_fav_icons_2.png

Note that I've seen the same behavior in Firefox 32 as well. Older versions like Firefox 29 seem to show a similar behavior, it's just there's no icon at all initially, and then the favicons are displayed.

This would need further investigation, as it's not very clear to me what causes the favicons to appear and disappear. It may be that they appeared after I tried older versions, but then disappeared again when I checked Nightly... not sure if that would make any sense, but it could make it difficult to identify a regression window.

Any thoughts on the above?
(In reply to Florin Mezei, QA (:FlorinMezei) from comment #5)
> I'm not quite sure what the actual issue is here. I tested a bit today on
> Windows 7 x64, and got some rather weird results, as favicons seemed to
> appear and disappear in a random manner.

> Any thoughts on the above?

Bug 1015595 (favicons may need some time to show up) might affect the tests? Also, each user profile keeps its own list of entries: how does this affect the jumplist when switching between different profiles?

For my part (37.0.1 Win7 x86) I consistently get the Firefox icons, with my default and with a fresh profile. And since the jumpListCache is not created (fresh profile) or deleted (default profile) I don't see how favicons could be displayed, unless they have moved to a new location.

Conversely, on two Win7 x86 machines at work, the euballot builds (that for some reason did not update to 37 and are stuck to 36.0.4) consistently display the favicons.
I also noted that the justListCache folder is not being re-created when deleted.

I have trying to chase this down for past two days, and finding the regression range is maddening.  Sometimes they work, next they don't.  

I did discover that it can take up to 120secs before the favicons are generated, thus further slowing the triage process.

For several hours yesterday after I quit trying the icons were all present and being displayed, then over the course of few hours, they all reverted back to 'generic' favicons.

So, question is, where are the cached ?, if at all anymore.
I got the 120 secs information from bug 549472
I was just going through my old test profiles, found one that still had icons in the jumplistcache.

Started up that profile, and within two mins or less, the jumplistcache was emptied, and all were now 'generic' icons on the 'right-click' of the taskbar icon.  

No icons were left in the folder, no new one added/created.

Question, still remains... where are they supposed to be ?
Something in this bug, https://bugzilla.mozilla.org/show_bug.cgi?id=1101195 or its dependencies break the favicons again in the jumplist possibly ?
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #14)
> Something in this bug, https://bugzilla.mozilla.org/show_bug.cgi?id=1101195
> or its dependencies break the favicons again in the jumplist possibly ?

I don't think so. Bug 1101195 was platform-independent and caused favicons to be saved as 0-length files, among other things. It was fixed in 35, which worked as expected. This bug seems Windows-specific and new to 37 (I don't remember ever seeing the Firefox icon in jumplist before; in 33 and 34, b/c of bug 1101195, I got a generic document icon).

Florin tested on Win 7 x64, I can only report about Win 7 x86: does this make a difference?

Something in the caching of favicons on Windows changed in 37, what?
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #11)
> I also noted that the justListCache folder is not being re-created when
> deleted.
> 
> I have trying to chase this down for past two days, and finding the
> regression range is maddening.  Sometimes they work, next they don't.  
> 
> I did discover that it can take up to 120secs before the favicons are
> generated, thus further slowing the triage process.
> 
> For several hours yesterday after I quit trying the icons were all present
> and being displayed, then over the course of few hours, they all reverted
> back to 'generic' favicons.
> 
> So, question is, where are the cached ?, if at all anymore.

The 120 second dealy is set in prefs - see 'browser.taskbar.lists.refreshInSeconds'. We set it out so it doesn't hit startup perf.

The cache is in:

C:\Users\[username]\AppData\Local\Mozilla\Firefox\Profiles\[someprofilefolder].default\jumpListCache

which, interestingly, is empty for me on Nightly.
(In reply to Jim Mathies [:jimm] from comment #16)
> (In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #11)
> > I also noted that the justListCache folder is not being re-created when
> > deleted.
> > 
> > I have trying to chase this down for past two days, and finding the
> > regression range is maddening.  Sometimes they work, next they don't.  
> > 
> > I did discover that it can take up to 120secs before the favicons are
> > generated, thus further slowing the triage process.
> > 
> > For several hours yesterday after I quit trying the icons were all present
> > and being displayed, then over the course of few hours, they all reverted
> > back to 'generic' favicons.
> > 
> > So, question is, where are the cached ?, if at all anymore.
> 
> The 120 second dealy is set in prefs - see
> 'browser.taskbar.lists.refreshInSeconds'. We set it out so it doesn't hit
> startup perf.
> 
> The cache is in:
> 
> C:
> \Users\[username]\AppData\Local\Mozilla\Firefox\Profiles\[someprofilefolder].
> default\jumpListCache
> 
> which, interestingly, is empty for me on Nightly.

Even more interesting is the fact that if you delete that folder, its not re-created.
In fact, I think someone said its not there in a new profile.  I will test that theory in a few mins.

The folder is not created in a new profile either.
More looking for the regressor - 

I changed pref:  browser.taskbar.lists.refreshInSeconds  to 10secs to expedite testing.

I tested several builds from Jan thru current builds.

I did not find a window where it broke, as the icons fill in very early during startup with the 10sec setting.

I then set the pref back to default (120) and I still have favicons... 

Don't know what to make of this.  More testing obviously required, but I have to work today.
Still something funky happening.

I switch to clean profile and it had the same jumplist as my working profile.
I 'removed from list' all those entries, and after two mins I can jumplist icons all 'generic' in the test profile.
I changed to timing to 10 secs as above, restarted browser twice and still have 'generic' Icons.  :(

Swithced back to my working profile, and it has the jumplist from the test profile.

Something really strange here...gotta go for now.
The two euballot builds at work finally updated to 37.0.1 and both showed the (by now) expected behaviour: the jumpListCache emptied and, after a couple of minutes, all favicons replaced by the generic icon.

As to the weird or random behaviour reported by others, I cannot observe it. I think it may be related to the switching of user profiles, because, AFAIK:

- the list of entries in the jump list is stored in %APPDATA%\Microsoft\Windows\Recent\CustomDestinations\[0123456789abcdef].customDestinations-ms
that file belongs to the Windows account and is shared by all Firefox user profiles for that account

- the cache of favicons associated to entries in the jump list is different for each Firefox profile and is in %LOCALAPPDATA%\Mozilla\Firefox\Profiles\[user.profile]\jumpListCache

I don't know how Firefox associates the entries and the favicons; however, since the contents of the CustDest file are likely inherited by a fresh profile, this might cause strange things. For instance, after some switching profiles for this bug, all my jump list entries disappeared, including "new private window" etc. I could recover them by restoring the CustDest file.
Finally have a regression range:
 m-c tinderbox builds win32:
20150118085158 6446c26b45f9 good
20150119071103 f8e4fdb89a05 bad not updating

Range: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=6446c26b45f9&tochange=f8e4fdb89a05

Testing is very complex to see this.  Once you have a good set of icons they will carry through up to present builds.  New sites it seems that come into the 'recents' will be generic icons in the bad build.

STR:  Load up good cset
Note that the right-click on Nightly taskbar icon will display favicons is 'recents list'
Remove the bottom two or three from list and wait, you will see sites added to list at first with generic icon, then will 'update' to the favicon.

Install the bad cset as above and not all favicons are still displayed
Remove again bottom two or three and wait for list to repopulate.
Check with a right-click often and you will see sites added to list (only 7 is displayed) and note they are generic icons and do NOT update, even if you re-visit the site. 

In my testing the path noted in above comment does not update or show the favicons.  My folder has not updated since 5/2014, so I don't think that has anything to do with issue.
C:\Users\Default\AppData\Roaming\Microsoft\Windows\Recent

The 'jumpListCache folder' does not exist in New Profiles and is not recreated if you have one and you delete it.  So, not being a coder or how to use MXR I still don't know where the 'recents' favicons are stored if at all.
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #21)
> Range:
> https://hg.mozilla.org/integration/mozilla-inbound/
> pushloghtml?fromchange=6446c26b45f9&tochange=f8e4fdb89a05

Bug 1119774 seems like a good guess.
Seth, any chance https://bugzilla.mozilla.org/show_bug.cgi?id=1119774 caused this ?
Flags: needinfo?(seth)
Another unexpected behaviour that may be related to this bug:

- visit a page
- drag and drop the url on the address bar to the desktop; it is saved as a shortcut with a generic icon

The saved shortcut's contents look like this:

[InternetShortcut]
URL=https://bugzilla.mozilla.org/show_bug.cgi?id=1150534
IDList=
HotKey=0
IconFile=%LOCALAPPDATA%\Mozilla\Firefox\Profiles\[user.profile]\shortcutCache\[filename].ico
IconIndex=0

but the shortcutCache dir is not created (if already existing, I guess it would be left unchanged).

Another guess is, if somebody has a url shortcut saved somewhere, after upgrading to 37 the icon may change to a generic one.
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #23)
> Seth, any chance https://bugzilla.mozilla.org/show_bug.cgi?id=1119774 caused
> this ?

It's possible, but it's not immediately obvious how. Downscale-during-decode does not currently apply to ICO, PNG, or BMP images, and it's not currently enabled by default in any case, so I don't think this is related to DDD. What's more likely is that something somehow got broken in the process of adding the DDD codepath.

One thing that surprises me is that this would break without breaking rendering of the same favicons everywhere. These same favicons do render correctly in e.g. the awesomebar, right?

Jim, I'm having trouble finding where in the code we might be deciding not to cache these icons. (That's what's happening, right?) Can you suggest where to look?
Flags: needinfo?(seth) → needinfo?(jmathies)
(In reply to Seth Fowler [:seth] from comment #25)
> (In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #23)
> > Seth, any chance https://bugzilla.mozilla.org/show_bug.cgi?id=1119774 caused
> > this ? 

>These same favicons do render correctly in e.g. the awesomebar, right?
> 
> Jim, I'm having trouble finding where in the code we might be deciding not
> to cache these icons. (That's what's happening, right?) Can you suggest
> where to look?

Yes, the icons appear to be ok in the drop-list from the Awesomebar.  They are also, of course, Generic icons in the fly-out list from 'Recent's' when using the 'Start Button' on Windows and highlighting the entry for Nightly.

Another oddity I've just noticed is that if one hovers over the entry in 'jumplist' the Text shows title of the entry twice.  

I beginning to wonder if 'Recents' is being updated properly, but I have no idea how to test that at this time, will have to think a bit.
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #27)
> Another oddity I've just noticed is that if one hovers over the entry in
> 'jumplist' the Text shows title of the entry twice.  

The title is only repeated for some entries. The only pattern I see is:

if the title takes 1 line, it is shown 1 time
if it takes 2 lines, it is shown 2 times

Didn't notice before, so I can't tell if this is new to 37.
Seth, did the comment #26 help?
Flags: needinfo?(seth)
(In reply to Sylvestre Ledru [:sylvestre] from comment #29)
> Seth, did the comment #26 help?

Honestly, no. =\ I still have no idea what would trigger this regression, especially given that the favicons display correctly in all other contexts. It seems like the bug is something that only affects the jumplist code - possibly it breaks some assumption the jumplist code is making. I was hoping for something like "we're probably failing to cache these icons because we're encountering an image-related error condition in this function".
Flags: needinfo?(seth)
I'm not a coder, but reading those links above in the 'mxr' about cache, etc the question still remains, where are they (Favicons) supposed to be cached.  I was unable to make an educated guess as where/what the name of the cache location is.

I know for certain that jumpListCache is NOT be created and if it exists, it gets emptied.

So if the jumpList has a special cache, I'm not finding it.
Flags: needinfo?(florin.mezei)
Seth, this sounds frustrating. Is there anything we can do to help diagnose this? Can you or jimm (or Jim) describe how people can capture useful information when they see the problem?  Or, is there something we can set up builds to run in a different state, like a debug build, that will give us more useful info?

Also sounds like this may have been broken for a while, and we're not sure how long it's been broken, or how widespread the problem is.
Flags: needinfo?(seth)
I honestly know nothing about this code, and it runs only on Windows, which is a platform that I'm relatively inexperienced with. It would be helpful if someone familiar with the jumplist code could do some initial debugging here. It's really not obvious what's going on, especially since the exact same ImageLib code is used to draw favicons on tabs, in the awesome bar, in the bookmarks window, etc., and all of those work fine.

Jim, looking at the blame it seems like you've touched this code a lot. Could you please do a triage here and try to figure out what's going wrong at the level of the jumplist code? For example, maybe the timing of a notification has changed, and the jumplist code depended on the previous timing? It's not obvious to me how changes in ImageLib map to an issue like "the jumpListCache emptied and, after a couple of minutes, all favicons replaced by the generic icon." This is far-removed from the kind of thing that ImageLib is responsible for, and I'm really not sure how to bridge the gap.
Flags: needinfo?(seth) → needinfo?(jmathies)
sure, I'll take a look this weekend. Sorry, I would have jumped on this normally but e10s is sucking all my time.
Thanks, I really appreciate that! (And totally understood on the e10s time-suck!)
Too late for 38. We will be happy to take a fix for 39.
jimm, any chance to look at this? I'm not sure what impact this has on users, depending on how many people use the jump list or how often it breaks.
(In reply to Liz Henry (:lizzard) from comment #37)
> jimm, any chance to look at this? I'm not sure what impact this has on
> users, depending on how many people use the jump list or how often it breaks.

I do not have time for this currently. My backlog on e10s is a mile high.
Flags: needinfo?(jmathies)
I know you have a lot to handle with e10s, but this is a regression on release that many Windows users are seeing.  Can someone who's a peer on Core:Widget take this on?
Flags: needinfo?(jmathies)
I don't have time for this until maybe around the end of the month. Tim do you have any free time for this?
Flags: needinfo?(jmathies) → needinfo?(tabraldes)
(In reply to Jim Mathies [:jimm] from comment #40)
> I don't have time for this until maybe around the end of the month. Tim do
> you have any free time for this?

I don't, sorry :-/

Didn't :bbondy do a bunch of work with jump lists?

Brian - are you working on Fx bugs these days?
Flags: needinfo?(tabraldes) → needinfo?(netzen)
On some selective things and very part time contributions, so someone else should take if avail.
Flags: needinfo?(netzen)
Florin, can you check one more thing - does this affect Windows 10? 
No rush but I don't want to forget. 
If it does, can you connect this to https://bugzilla.mozilla.org/show_bug.cgi?id=windows-10 ?
I know it puts the jump list in the start menu. So, it seems likely.
Flags: needinfo?(florin.mezei)
Blocks: windows-10
Flags: needinfo?(florin.mezei)
Seth, I poked at this with MSVS for a few minutes, looks like it fails when doing:

  RefPtr<SourceSurface> surface =
    container->GetFrame(imgIContainer::FRAME_FIRST, 0);
  NS_ENSURE_TRUE(surface, NS_ERROR_FAILURE);

inside:

NS_IMETHODIMP
AsyncFaviconDataReady::OnComplete(nsIURI *aFaviconURI,
                                  uint32_t aDataLen,
                                  const uint8_t *aData, 
                                  const nsACString &aMimeType)

here:

http://hg.mozilla.org/mozilla-central/annotate/0093691d3715/widget/windows/WinUtils.cpp#l1122

I don't know why because I don't know anything about imglib/SourceSurface/whatever. But you do! Can you help figure out the rest? :-)
Flags: needinfo?(seth)
Gijs, thank you very much for investigating this! That really helps a lot.

I am digging out from under the pile of stuff that accumulating while I was on PTO over the past week, but I'll investigate this in more detail either today or Monday based on what you've discovered, and hopefully we'll have a fix soon.
Just for some more context, the easy way to repro this is to toggle the about:config pref ( browser.taskbar.lists.enabled ) which forces the lists to be regenerated.
(this is probably P3 for the win10 prioritization list, but it's a regression for everything else, so a quick fix would obviously still be very helpful...)
Priority: -- → P3
(In reply to :Gijs Kruitbosch from comment #46)
> Just for some more context, the easy way to repro this is to toggle the
> about:config pref ( browser.taskbar.lists.enabled ) which forces the lists
> to be regenerated.

If the issue at comment 24 is related to this bug, then the steps described there may be another quick way to reproduce.
Ugh, sorry it took me so long to get to this. Still unburying myself.

The actual fix is pretty simple, though.
Attachment #8625007 - Flags: review?(tnikkel)
Assignee: nobody → seth
Status: NEW → ASSIGNED
Flags: needinfo?(seth)
Attachment #8625007 - Flags: review?(tnikkel) → review+
(In reply to Seth Fowler [:seth] from comment #51)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/22c57d6465dd

Testing this build, and the issue stated in this bug is fixed.  Jumplist Favicons are once again displayed, and the jumplist cache folder is re-populated, re-created if its missing.

thanks Seth.
https://hg.mozilla.org/mozilla-central/rev/22c57d6465dd
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Fabulous!  We may want to at least uplift this to 40.
A little bit too late for 39. We should uplift this to 40 though.
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #52)
> Testing this build, and the issue stated in this bug is fixed.

That's great. Thanks for verifying the fix!
Comment on attachment 8625007 [details] [diff] [review]
Synchronously decode favicons before calling GetFrame

Approval Request Comment
[Feature/regressing bug #]: Bug 1119774
[User impact if declined]: Favicons won't display in the jumplist on Windows.
[Describe test coverage new/current, TreeHerder]: I have absolutely no idea how to write a test for this. On m-c for 4 days.
[Risks and why]: Very, very low risk.
[String/UUID change made/needed]: None.
Attachment #8625007 - Flags: approval-mozilla-aurora?
Comment on attachment 8625007 [details] [diff] [review]
Synchronously decode favicons before calling GetFrame

Let's take in aurora! We would have time to fix potential regressions.
Attachment #8625007 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Sylvestre Ledru [:sylvestre] from comment #58)
> Comment on attachment 8625007 [details] [diff] [review]
> Synchronously decode favicons before calling GetFrame
> 
> Let's take in aurora! We would have time to fix potential regressions.

uplifted to aurora in https://hg.mozilla.org/releases/mozilla-aurora/rev/fb9337b67ab2
I've verified that I can still reproduce the original issue with Firefox 37.0.2 on Windows 7 x64. I then tested with Firefox 40 Beta 6 on the same Windows 7 x64 machine, as well as on Windows 10 x64 & x86. In all cases the jump list was populated with new entries which originally had the default firefox icon, but after a few minutes the correct favicon would show for each entry. On Windows 10 x64, the items in the start menu list also displayed the favicons as expected.
I was able to reproduce this issue on Firefox 37.0.2 under Windows 7 64-bit. 
Verified fixed on Firefox 41 beta 3 (20150820142145) under Windows 7 64-bit and Windows 10 32-bit. The correct favicons are successfully displayed after a few minutes.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.