Last Comment Bug 705621 - No tab item is selected after removing last tab in a group outside Panorama
: No tab item is selected after removing last tab in a group outside Panorama
Status: RESOLVED FIXED
:
Product: Firefox Graveyard
Classification: Graveyard
Component: Panorama (show other bugs)
: Trunk
: All All
: -- normal
: Firefox 11
Assigned To: Raymond Lee [:raymondlee]
:
Mentors:
Depends on: 706736
Blocks: 637840
  Show dependency treegraph
 
Reported: 2011-11-28 01:27 PST by Raymond Lee [:raymondlee]
Modified: 2016-04-12 14:00 PDT (History)
1 user (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
v1 (7.80 KB, patch)
2011-11-28 02:12 PST, Raymond Lee [:raymondlee]
ttaubert: review+
Details | Diff | Splinter Review
Patch for checkin (7.71 KB, patch)
2011-11-28 08:19 PST, Raymond Lee [:raymondlee]
no flags Details | Diff | Splinter Review

Description Raymond Lee [:raymondlee] 2011-11-28 01:27:49 PST
Steps to reproduce:
1) Open two groups in Panorama. Each group with one tab.
2) Click on one tabitem
3) In the normal browser mode, close the last tab
4) It goes into Panorama automatically.  One group with tab item and one empty group
5) No tab item is selected.

Expected Result:
5) A tab item is selected.
Comment 1 Raymond Lee [:raymondlee] 2011-11-28 02:12:29 PST
Created attachment 577209 [details] [diff] [review]
v1
Comment 2 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2011-11-28 04:40:09 PST
Comment on attachment 577209 [details] [diff] [review]
v1

Review of attachment 577209 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, nice work!

::: browser/components/tabview/groupitems.js
@@ +1143,5 @@
>  
>        let closed = options.dontClose ? false : this.closeIfEmpty();
> +      if (closed ||
> +          (this._children.length == 0 &&
> +           (!gBrowser.selectedTab.pinned && !item.isDragging))) {

Nit: maybe we could have an additional variable here that makes this condition a bit more nice and readable.

::: browser/components/tabview/test/browser_tabview_bug705621.js
@@ +34,5 @@
> +    hideTabView(function() {
> +      //win.gBrowser.removeTab(win.gBrowser.selectedTab);
> +
> +    });
> +*/

Nit: please remove that commented out code.
Comment 3 Raymond Lee [:raymondlee] 2011-11-28 08:19:16 PST
Created attachment 577278 [details] [diff] [review]
Patch for checkin

(In reply to Tim Taubert [:ttaubert] from comment #2)
> ::: browser/components/tabview/groupitems.js
> @@ +1143,5 @@
> >  
> >        let closed = options.dontClose ? false : this.closeIfEmpty();
> > +      if (closed ||
> > +          (this._children.length == 0 &&
> > +           (!gBrowser.selectedTab.pinned && !item.isDragging))) {
> 
> Nit: maybe we could have an additional variable here that makes this
> condition a bit more nice and readable.

I just keep it as it is because if "closed" is true, the second condition wouldn't be executed.  If we add an additional variable before the "if" statement, that condition would be executed even "closed" is true.

> 
> ::: browser/components/tabview/test/browser_tabview_bug705621.js
> @@ +34,5 @@
> > +    hideTabView(function() {
> > +      //win.gBrowser.removeTab(win.gBrowser.selectedTab);
> > +
> > +    });
> > +*/
> 
> Nit: please remove that commented out code.

Removed
Comment 4 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2011-11-28 10:02:53 PST
https://hg.mozilla.org/integration/fx-team/rev/1d63a85fea37
Comment 5 Tim Taubert [:ttaubert] (on PTO, back Aug 29th) 2011-11-29 21:52:04 PST
https://hg.mozilla.org/mozilla-central/rev/1d63a85fea37
Comment 6 Stefan 2011-11-30 18:39:14 PST
If you quit/restart the Fx before deleting the tab, no tab will be selected.

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