248720 - Tabs do not change properly to reflect a new Tab set

Status: RESOLVED FIXED

(Camino Graveyard :: Tabbed Browsing, defect)

Description

[Andrew Fraser]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7) Gecko/20040623 Camino/0.8
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7) Gecko/20040623 Camino/0.8

When changing from a set of tabs with less tabs than a previous set, for example
from five tabs to three tabs, the new set will still show five five tabs with
the first three tabs reflecting the chosen new tab set but the remaining two
tabs being from the older set. 

Reproducible: Sometimes
Steps to Reproduce:
1.Have a couple of tab sets with differing numbers of tabs in them.
2.Open the tab set with the larger number of tabs
3.Change the tab set to the one with fewer tabs



Expected Results:  
The new tab set should show only those tabs in that set.

Comment 1

[Stuart Morgan]

I'm inclined to agree that our current behavior is strange, so confirming. A
tab-group is fundamentally different than a single bookmark; unless the group is
being opened in *new* tabs by user request, I agree that the expected behavior
is that the tab group will become the new state of the window. The current
behavior is hard to predict by a user, since it requires them to remember all
their tab group page counts. Also, I can't think of case where this would be the
desired behavior.

Nominating for 0.9 so this can be fixed during / as a result of bookmark cleanup.

Comment 2

[Julian Pellico]

taking

Comment 3

[Julian Pellico]

Patch coming up.

There are several places in the code that open a set of tabs, replacing existing
ones, such as drag-and-drop. Is the Tab Group the only place where extra tabs
should be closed? My patch currently alters the behavior only for opening a Tab
Group.

Comment 4

[Julian Pellico]

Attachment: fix for tab groups

Comment 5

[Julian Pellico]

Ack, I probably should've just used ints.

Comment 6

[Julian Pellico]

Jasper and Geoff, do you have any response to comment #3?

Comment 7

[Geoff "temporarily off bugmail" Beier]

(In reply to comment #6)
> Jasper and Geoff, do you have any response to comment #3?

I think a tab group is a special case, and would say that this behavior is only
appropriate when the tab group is opened normally. If it's opened in "new tabs"
this behavior should not apply... please check to be sure this patch doesn't do
that. (I have not tested.)

Comment 8

[Julian Pellico]

Attachment: close extra tabs

I had a really stupid typo in the first patch. Don't know how I missed it.

Anyways, this can be tested.

Comment 9

[Geoff "temporarily off bugmail" Beier]

This patch is against BrowserWindowController.mm 1.148; current is 1.149. It still appears to apply 
cleanly.

Comment 10

[Geoff "temporarily off bugmail" Beier]

Comment on attachment 164069 [details] [diff] [review]
close extra tabs

Looks good, works as described. r=[e-mail]

Comment 11

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 164069 [details] [diff] [review]
close extra tabs

sr=pink

Comment 12

[Josh Aas]

guess I forgot to land this... landed on trunk