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

VERIFIED FIXED in fennec1.0a2

Status

Firefox for Android Graveyard
General
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: abillings, Assigned: mfinkle)

Tracking

Trunk
fennec1.0a2
x86
Linux

Details

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
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...)

Updated

10 years ago
Assignee: nobody → mark.finkle
Target Milestone: --- → Fennec A3
(Assignee)

Comment 1

10 years ago
Created attachment 351243 [details] [diff] [review]
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
Attachment #351243 - Flags: review?(gavin.sharp)
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?
Attachment #351243 - Flags: review?(gavin.sharp) → review+

Comment 3

10 years ago
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.
(Assignee)

Comment 4

10 years ago
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
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Assignee)

Updated

10 years ago
Target Milestone: Fennec A3 → Fennec A2
(Reporter)

Comment 5

10 years ago
Verified with Mozilla/5.0 (X11; U; Linux a mv6l; en-US; rv:1.9.2a1pre) Gecko/20081223 Fennec/1.0a2.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.