643392 - A "ghost tab" appears inside a Tab Group

Status: VERIFIED FIXED

(Firefox Graveyard :: Panorama, defect, P4)

Description

[FlameWolf]

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0) Gecko/20100101 Firefox/4.0
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0) Gecko/20100101 Firefox/4.0

I had two tab groups open when I updated to Fx4 RC2 and restarted. After that, an empty tab appeared in one of my tab groups. Clicking on it will just return me to the current tab -- regardless of which tab group is currently active. I can even click and drag the "ghost" tab around inside or across tab groups. But trying to close it will have no effect; a border appears around the close button, and it stays that way. The tab cannot be closed. Here's a screenshot of what it looks like: http://i121.photobucket.com/albums/o204/ajifocus/Public/FxGhostTab.jpg

Reproducible: Didn't try

Comment 1

[Kevin Hanes]

That's really unfortunate. Sounds like you've got some bogus data in your profile. 

Has anyone else seen this?

Comment 2

[alex]

This happens to me pretty often. If i restart Firefox it disappears but after a while it appears again

Comment 3

[Kevin Hanes]

Will investigate.

Comment 4

[Boris Smelov]

I can confirm that this has happened to me a couple times.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

I manage to reproduce a ghost tab with this:

1) Create multiple groups
2) Restart firefox
3) close all tabs of the current group
4) Open Panorama

The actually closed group is still there and contains a ghost tab.

Comment 6

[Raymond Lee [:raymondlee]]

(In reply to comment #5)
> I manage to reproduce a ghost tab with this:
> 
> 1) Create multiple groups
> 2) Restart firefox
> 3) close all tabs of the current group
> 4) Open Panorama
> 
> The actually closed group is still there and contains a ghost tab.

Please have a look at the patch for bug 654295 which probably fixes this as well.

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

(In reply to comment #6)
> Please have a look at the patch for bug 654295 which probably fixes this as
> well.

This is indeed fixed by bug 654295. The problem is that it only fixes the symptom (removing the ghost tab afterwards) but not the source (not linking tabs that are being removed). I'd rather not combine these patches so let's handle the fix here in this bug and let bug 654295 depend on it.

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v1

Comment 9

[Raymond Lee [:raymondlee]]

Comment on attachment 532597 [details] [diff] [review]
patch v1

Looks good!

+function newWindowWithState(state, callback) {

+function whenWindowLoaded(win, callback) {

+function whenWindowStateReady(win, callback) {

May b we can move those functions to head.js

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v2

(In reply to comment #9)
> +function newWindowWithState(state, callback) {
> +function whenWindowLoaded(win, callback) {
> +function whenWindowStateReady(win, callback) {
> 
> May b we can move those functions to head.js

Good idea, done.

Comment 11

[(no longer active)]

I'm not sure if it's OK to access the tabbrowser guts like this.  You should ask Dao whether he thinks this is OK or not.

Comment 12

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 532616 [details] [diff] [review]
patch v2

(In reply to comment #11)
> I'm not sure if it's OK to access the tabbrowser guts like this.  You should
> ask Dao whether he thinks this is OK or not.

Ok, I know that we access _removingTabs in another place but let's just ask him :)

Comment 13

[Tim Taubert [:ttaubert] (inactive)]

Comment on attachment 532616 [details] [diff] [review]
patch v2

Passed try:

http://tbpl.mozilla.org/?tree=Try&pusher=[e-mail]&rev=72fee5cfc4fa

Comment 14

[Dão Gottwald [:dao]]

Comment on attachment 532616 [details] [diff] [review]
patch v2

>   get tabs() {
>     // Get tabs from each browser window and flatten them into one array
>     return Array.concat.apply(null, browserWindows.map(function(browserWindow) {
>-      return Array.slice(browserWindow.gBrowser.tabs);
>+      let removingTabs = browserWindow.gBrowser._removingTabs;
>+      return Array.slice(browserWindow.gBrowser.tabs).filter(function (tab) {
>+        return removingTabs.indexOf(tab) == -1;
>+      });

Replace 'Array.slice(browserWindow.gBrowser.tabs).filter(' with 'Array.filter(browserWindow.gBrowser.tabs, '.

I haven't reviewed the test.

Comment 15

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch v3

(In reply to comment #14)
> Replace 'Array.slice(browserWindow.gBrowser.tabs).filter(' with
> 'Array.filter(browserWindow.gBrowser.tabs, '.

Fixed.

> I haven't reviewed the test.

Asking Ehsan for review again :)

Comment 16

[(no longer active)]

Comment on attachment 532760 [details] [diff] [review]
patch v3

>+    let checkTabCount = function () {
>+      if (win.gBrowser.tabs.length > 1) {
>+        executeSoon(checkTabCount);
>+        return;
>+      }

Why is this necessary?  Is it because you're closing the tab with animation?  If yes, please take that out and just check this once after calling removeTab.

Comment 17

[Tim Taubert [:ttaubert] (inactive)]

(In reply to comment #16)
> Why is this necessary?  Is it because you're closing the tab with animation?
> If yes, please take that out and just check this once after calling
> removeTab.

Yeah this is because I remove the tab with animation, which is actually part of the problem because the problem doesn't exist when I remove the tab directly. Panorama is initialized while the tab is still being removed and therefore a ghost tab is linked.

Comment 18

[(no longer active)]

Comment on attachment 532760 [details] [diff] [review]
patch v3

Oh, I see!

Comment 19

[Tim Taubert [:ttaubert] (inactive)]

Attachment: patch for checkin

Comment 20

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/e707886bdf16

Comment 21

[Simona Badau, Desktop QA]

Mozilla/5.0 (Windows NT 5.1; rv:6.0a2) Gecko/20110525 Firefox/6.0a2

Verified issue using the STR from Comment 5 on Win XP, Win 7, Ubuntu, Mac OS X. The "ghost tab" is no longer present.