updateIcon hides the favicon while the tab is busy, which we don't want anymore.

Comment on attachment 476514 [details] [diff] [review] patch The change to useDefaultIcon is to make sure that the icon gets removed now that we're no longer calling updateIcon() in onStateChange(STATE_STOP & STATE_IS_NETWORK), right? Do you know why we also call updateIcon() from the browser.js WPL? Seems redundant.

(In reply to comment #1) > Comment on attachment 476514 [details] [diff] [review] > patch > > The change to useDefaultIcon is to make sure that the icon gets removed now > that we're no longer calling updateIcon() in onStateChange(STATE_STOP & > STATE_IS_NETWORK), right? Yep. > Do you know why we also call updateIcon() from the browser.js WPL? Seems > redundant. Do you mean setIcon? I don't really see why it does that, but it only does it for !gBrowser.mTabbedMode, which we should probably get rid of. (bug 463384)

(In reply to comment #2) > Do you mean setIcon? Er, no I actually meant useDefaultIcon(). It seems to be called by both the tabbrowser and browser.js, and I don't see why that's necessary.

(In reply to comment #3) > (In reply to comment #2) > > Do you mean setIcon? > > Er, no I actually meant useDefaultIcon(). It seems to be called by both the > tabbrowser and browser.js, and I don't see why that's necessary. This also depends on gBrowser.mTabbedMode.

browser_bug521216.js fix added

http://hg.mozilla.org/mozilla-central/rev/32f406acbb45 Landed for beta 7, as it seems crucial for bug 544818.

This seems to have caused some tabs to have the wrong favicon until they've finished loading. For instance, if I'm on Facebook and someone posts a link to an external site and I middle-click on it, then the new tab will have Facebook's favicon (presumably because all posted links go through a Facebook redirector) but will then keep that favicon right through until the destination page has completely loaded. Actually a more obvious way to see this is to open up any Google search result in a new tab (assuming you don't have scripting disabled). If you choose a slow loading page then it's more obvious.

(In reply to comment #7) Please file a new bug. (The "correct" way would be to clear the icon on location change, but that seems like it would be annoying esp. for fast loading pages. Maybe we can delay it a bit.)

Bug 597665 should cover that.

Dao, so updateIcon means the old throbber?

No... updateIcon didn't care about which throbber image is used.