651440 - Call groupItem.closeIfEmpty in onTabSelect

Status: RESOLVED WONTFIX

(Firefox Graveyard :: Panorama, defect)

Description

[ithinc]

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:6.0a1) Gecko/20110419 Firefox/6.0a1
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:6.0a1) Gecko/20110419 Firefox/6.0a1

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-tabview.js#285

285         self._initFrame(function() {
286           let groupItems = self._window.GroupItems;
287           let tabItem = groupItems.getNextGroupItemTab(event.shiftKey);
288           if (!tabItem)
289             return;
290 
291           // Switch to the new tab, and close the old group if it's now empty.
292           let oldGroupItem = groupItems.getActiveGroupItem();
293           window.gBrowser.selectedTab = tabItem.tab;
294           oldGroupItem.closeIfEmpty();
295         });

It seems better to call closeIfEmpty in onTabSelect, to simplify the group switching code.

Reproducible: Always

Comment 1

[Raymond Lee [:raymondlee]]

(In reply to comment #0)
> User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:6.0a1) Gecko/20110419
> Firefox/6.0a1
> Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:6.0a1) Gecko/20110419
> Firefox/6.0a1
> 
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser-
> tabview.js#285
> 
> 285         self._initFrame(function() {
> 286           let groupItems = self._window.GroupItems;
> 287           let tabItem = groupItems.getNextGroupItemTab(event.shiftKey);
> 288           if (!tabItem)
> 289             return;
> 290 
> 291           // Switch to the new tab, and close the old group if it's now
> empty.
> 292           let oldGroupItem = groupItems.getActiveGroupItem();
> 293           window.gBrowser.selectedTab = tabItem.tab;
> 294           oldGroupItem.closeIfEmpty();
> 295         });
> 
> It seems better to call closeIfEmpty in onTabSelect, to simplify the group
> switching code.
> 
> Reproducible: Always

The above code is triggered when switching to another group using the keyboard combination.  Calling closeIfEmpty() in onTabSelect seems to be overkilled as user would switch to another tab in the same group.

@tim: any comments?

Comment 2

[ithinc]

Isn't there a branch to handle group switching in onTabSelect? And I suggest that you take bug 654311 into consideration.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

bugspam

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

bugspam

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

Empty groups won't be removed automatically anymore (bug 663421).