464756 - Opening new tab after closing another recycles previous favicon in new tab

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Al Billings [:abillings - ex-MoCo]]

If a user has two or more tabs open in Fennec and closes one, if they open a new tab, it will show up with the favicon from the site on the previously closed on.

Steps to Reproduce
1. Start Fennec
2. Open new tab and load http://www.cnn.com (or any site with a favicon) in it
3. After page loads, close tab
4. Open new tab

Result: New tab will be prepopulated with the cnn.com favicon.

This was found in Mozilla/5.0 (X11; U; Linux armv6I; en-US; rv:1.9.1b2pre) Gecko/20081113 Fennec/1.0a2pre. (The 11/13 automatic build...)

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: patch

This patch does two things to fix the bug:
* sets BrowserUI._faviconLink in BrowserUI._tabSelect so the code in BrowserUI.update doesn't override the favicon
* sets BrowserUI._faviconLink to null in BrowserUI.update after we use it to set the favicon

The patch also:
* disables the zooming in ScrollwheelModule because it was broken
* removes unused code in BrowserUI.update
* adds the javascript.options.showInConsole = false, so I can at least set it to true using about:config

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 351243 [details] [diff] [review]
patch

>diff --git a/chrome/content/browser-ui.js b/chrome/content/browser-ui.js

>   _tabSelect : function(aEvent) {
>     var browser = Browser.currentBrowser;
>     this._titleChanged(browser.contentDocument);
>     this._favicon.src = browser.mIconURL || kDefaultFavIconURL;
>+
>+    // for new tabs, _tabSelect & update(TOOLBARSTATE_LOADED) are called when
>+    // about:blank is loaded. set _faviconLink here so it is not overriden in update
>+    this._faviconLink = this._favicon.src;

I think this might screw up background tab loads, but I guess we don't really support those at the moment, eh?

Comment 3

[Stuart Parmenter]

we should be careful to make sure we don't have issues with backgroudn loads -- i made sure all the tab and event listener stuff pays attention to if it is the current browser or not.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

As gavin points out, BrowserUI is not set up to handle background loads, even though Browser might be ok.

Filed bug 468294

http://hg.mozilla.org/mobile-browser/rev/3e7f1001ad3d

Comment 5

[Al Billings [:abillings - ex-MoCo]]

Verified with Mozilla/5.0 (X11; U; Linux a mv6l; en-US; rv:1.9.2a1pre) Gecko/20081223 Fennec/1.0a2.