Last Comment Bug 643392 - A "ghost tab" appears inside a Tab Group
: A "ghost tab" appears inside a Tab Group
Status: VERIFIED FIXED
:
Product: Firefox Graveyard
Classification: Graveyard
Component: Panorama (show other bugs)
: Trunk
: All All
: P4 normal
: Firefox 6
Assigned To: Tim Taubert [:ttaubert] (on PTO, back Aug 29th)
:
Mentors:
Depends on:
Blocks: 653099 654295
  Show dependency treegraph
 
Reported: 2011-03-21 04:20 PDT by Ajith Nair
Modified: 2016-04-12 14:00 PDT (History)
10 users (show)
mounir: in‑testsuite+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch v1 (4.77 KB, patch)
2011-05-16 03:33 PDT, Tim Taubert [:ttaubert] (on PTO, back Aug 29th)
raymond: feedback+
Details | Diff | Splinter Review
patch v2 (5.24 KB, patch)
2011-05-16 06:05 PDT, Tim Taubert [:ttaubert] (on PTO, back Aug 29th)
dao+bmo: review+
Details | Diff | Splinter Review
patch v3 (5.23 KB, patch)
2011-05-16 15:19 PDT, Tim Taubert [:ttaubert] (on PTO, back Aug 29th)
ehsan: review+
Details | Diff | Splinter Review
patch for checkin (4.47 KB, patch)
2011-05-17 14:33 PDT, Tim Taubert [:ttaubert] (on PTO, back Aug 29th)
no flags Details | Diff | Splinter Review

Description Ajith Nair 2011-03-21 04:20:50 PDT
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 2011-03-25 13:37:28 PDT
That's really unfortunate. Sounds like you've got some bogus data in your profile. 

Has anyone else seen this?
Comment 2 alex 2011-03-26 06:30:27 PDT
This happens to me pretty often. If i restart Firefox it disappears but after a while it appears again
Comment 3 Kevin Hanes 2011-03-31 11:10:27 PDT
Will investigate.
Comment 4 Boris Smelov 2011-05-03 10:46:44 PDT
I can confirm that this has happened to me a couple times.
Comment 5 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2011-05-12 11:07:14 PDT
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] 2011-05-12 11:28:48 PDT
(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] (on PTO, back Aug 29th) 2011-05-16 03:33:03 PDT
(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] (on PTO, back Aug 29th) 2011-05-16 03:33:52 PDT
Created attachment 532597 [details] [diff] [review]
patch v1
Comment 9 Raymond Lee [:raymondlee] 2011-05-16 05:10:41 PDT
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] (on PTO, back Aug 29th) 2011-05-16 06:05:28 PDT
Created attachment 532616 [details] [diff] [review]
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 :Ehsan Akhgari 2011-05-16 08:49:55 PDT
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] (on PTO, back Aug 29th) 2011-05-16 08:54:05 PDT
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] (on PTO, back Aug 29th) 2011-05-16 10:55:33 PDT
Comment on attachment 532616 [details] [diff] [review]
patch v2

Passed try:

http://tbpl.mozilla.org/?tree=Try&pusher=tim.taubert@gmx.de&rev=72fee5cfc4fa
Comment 14 Dão Gottwald [:dao] 2011-05-16 11:35:09 PDT
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] (on PTO, back Aug 29th) 2011-05-16 15:19:05 PDT
Created attachment 532760 [details] [diff] [review]
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 :Ehsan Akhgari 2011-05-17 08:21:38 PDT
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] (on PTO, back Aug 29th) 2011-05-17 08:27:33 PDT
(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 :Ehsan Akhgari 2011-05-17 14:21:47 PDT
Comment on attachment 532760 [details] [diff] [review]
patch v3

Oh, I see!
Comment 19 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2011-05-17 14:33:36 PDT
Created attachment 533073 [details] [diff] [review]
patch for checkin
Comment 20 Mounir Lamouri (:mounir) 2011-05-19 06:13:30 PDT
Pushed:
http://hg.mozilla.org/mozilla-central/rev/e707886bdf16
Comment 21 Simona B [:simonab ] -PTO- back Sept 5th 2011-05-26 06:49:52 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.