User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 SeaMonkey/2.26 (Beta/Release) Build ID: 20140415200419 Steps to reproduce: I visited CNN and then logged into and then out of eBay. When I viewed the Aero peek window of the logged out eBay session, I still saw the CNN site icon in the top left corner of the Aero peek window (see attached image). Actual results: The site icon of the previously viewed web site (CNN) showed as being the site icon of the currently viewed web site (eBay). Expected results: The eBay site icon should have appeared in the Aero peek window instead of the CNN site icon.

Work done in bug 839891 appears to be causing this issue. Setting browser.taskbar.previews.enable=false fixes the issue.

Ah, I bet the problem here is that eBay isn't showing a link rel=icon and so we should be clearing the previously loaded favicon.

Conveniently WindowsPreviewPerTab.jsm already handles null icons, resetting to the default icon, so I thought the easiest way to handle this was to make setIcon null-safe (like it is in Firefox) and also nsBrowserStatusHandler. Unfortunately the code for setIcon is more complicated than it could have been because of the string compatibility requirement.

Comment on attachment 8415838 [details] [diff] [review] Proposed patch The patch does seem to work, but I wonder about another thing: Shouldn't the icon in aero UI always be the same as the icon in the url bar and the icon on the tab itself? This seems to be different when telling SeaMonkey to always load favicon.ico (pref browser.chrome.favicons set to true). For example when I go to http://edition.cnn.com/ I see the CNN logo in url bar and tab, but not in the aero preview of that tab.

So with rel=icon it looks like it runs this code http://hg.mozilla.org/comm-central/annotate/0927618bbc2e/suite/browser/tabbrowser.xml#l1085 which calls " 1106 this.setIcon(this.tabs[index], iconUri);" which is why the icon gets updated. Maybe the loadFavIcon method at http://hg.mozilla.org/comm-central/annotate/0927618bbc2e/suite/browser/tabbrowser.xml#l779 should call setIcon to set the icon (which will call onLinkIconAvailable). But we can deal with this in a new bug if needed.

(In reply to Frank Wein [:mcsmurf] from comment #4) > Comment on attachment 8415838 [details] [diff] [review] > Proposed patch > > The patch does seem to work, but I wonder about another thing: Shouldn't the > icon in aero UI always be the same as the icon in the url bar and the icon > on the tab itself? This seems to be different when telling SeaMonkey to > always load favicon.ico (pref browser.chrome.favicons set to true). For > example when I go to http://edition.cnn.com/ I see the CNN logo in url bar > and tab, but not in the aero preview of that tab. If the site has an icon, I've always seen it displayed in the Aero peek.

The current navigator code wants to process favicon.ico itself so we'd have to rewrite those parts too (I guess we should start using the favicon service too).

I wonder if a patch like this does the correct thing? (Actually it does not as in this line browser.mIconURL = aURI.spec; browser is undefined. But somehow it sets the icon anyway in the aero and browser GUI).

(In reply to Frank Wein from comment #8) > (Actually it does not as in this line > browser.mIconURL = aURI.spec; > browser is undefined. But somehow it sets the icon anyway in the aero and > browser GUI). That's because it's called from navigator.js as well, and that's when you're getting the undefined browser.

Oh, and there's a difference in behaviour: tabs prefer the loading icon, then the link icon, then the image thumbnail, then favicon.ico, while the URL bar prefers favicon.ico until a link icon overrides it. (Obviously the URL bar has no loading icon, and it also doesn't try to thumbnail images either.)

Comment on attachment 8415838 [details] [diff] [review] Proposed patch Ok, r+ then, can deal with the other issue in a new bug.

Pushed comm-central changeset b533f0b61aef.

Comment on attachment 8415838 [details] [diff] [review] Proposed patch [Approval Request Comment] Regression caused by (bug #): 839891 User impact if declined: Bogus icons appear in Aero Peek Testing completed (on m-c, etc.): Landed on c-c Risk to taking this patch (and alternatives if risky): Low, onLinkIconAvailble now passes null to indicate no icon, copying existing Firefox behaviour. String changes made by this patch: None

Comment on attachment 8415838 [details] [diff] [review] Proposed patch a=me comm-aurora and comm-beta

Landed on comm-release for SeaMonkey 2.26.1 $ hg tip changeset: 20186:aa67f2715fd3 branch: SEA_2_26_1_RELBRANCH tag: tip user: Neil Rashbrook <neil@parkwaycc.co.uk> date: Mon Jun 02 00:10:24 2014 +0100 summary: Bug 998171 Clear the Aero Peek favicon when the document changes r=mcsmurf a=Ratty

Any ETA on when the 2.26.1 build will be up to test this?

testing ready should be tomorrow/this weekend, we're hoping for official release early next week.

Just tested with 2.26.1 and it look good. Thanks for fixing.