Call groupItem.closeIfEmpty in onTabSelect

RESOLVED WONTFIX

Status

Firefox Graveyard
Panorama
--
minor
RESOLVED WONTFIX
7 years ago
2 years ago

People

(Reporter: ithinc, Unassigned)

Tracking

Dependency tree / graph

Details

(Reporter)

Description

7 years ago
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
(Reporter)

Updated

7 years ago
Blocks: 603789
Severity: normal → minor
No longer blocks: 603789
Blocks: 653099
(Reporter)

Updated

7 years ago
Depends on: 654311
(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?
(Reporter)

Comment 2

7 years ago
Isn't there a branch to handle group switching in onTabSelect? And I suggest that you take bug 654311 into consideration.
bugspam
No longer blocks: 653099
bugspam
Blocks: 660175
Empty groups won't be removed automatically anymore (bug 663421).
Status: UNCONFIRMED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → WONTFIX
(Assignee)

Updated

2 years ago
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.