Closed Bug 586198 Opened 14 years ago Closed 14 years ago

Incorrect placement of tab after Undo Close Tab in Tab Candy


(Firefox Graveyard :: Panorama, defect)

Not set


(blocking2.0 beta4+)

Firefox 4.0b4
Tracking Status
blocking2.0 --- beta4+


(Reporter: aza, Assigned: ttaubert)



(Keywords: regression, Whiteboard: [in-litmus-bug-week][hardblocker][fx4-fixed-bugday])


(1 file, 2 obsolete files)

In a group with multiple tabs (say three):

(1) Close a tab while inside zoomed into the group
(2) Using the context-menu or the keyboard shortcut to undo the close tab
(3) The tab will appear in a group by itself (i.e., the other tabs will go away) and the tab in the Tab Candy interface will appear directly underneath the former group (so hidden until the group is moved).
Whiteboard: b5
Mozilla/5.0 (Windows NT 5.1; rv:2.0b4pre) Gecko/20100812 Minefield/4.0b4pre ID:20100813015949

I think this is the same bug, except that the behaviour I see on step (3) is the following :
- the restored tab is the only tab
- when going to TabView, it works as it should, the restored tab is zoomed-out to its group.
Mass moving all Tab Candy bugs from Mozilla Labs to Firefox::Tab Candy.  Filter the bugmail spam with "tabcandymassmove".
Product: Mozilla Labs → Firefox
Target Milestone: -- → ---
Version: unspecified → Trunk
Undo close tab is a pretty major feature. Technically not a regession as undo close tab does still reopen the closed tab. I suppose not quite my call if it should be fixed in beta 4.

What's the expected behavior though? Restore the tab into the group it was in before? Restore into old group and switch to that group? Restore it into the current group?
blocking2.0: --- → ?
Keywords: regression
Blocks: 574217
(In reply to comment #4)
>Restore the tab into the group it was in before?

To me, this makes the most sense and also generally covers option 3 ("Restore it into the current group"), when undo closed tab is bounded by group, instead of operating globally.

Here are my thoughts.  Assume I have Group A, Group B, and Group C:

SCENARIO I:  While browsing Group A, I close a tab.

a) while still browsing Group A, I undo closed tab, the tab should be restore to my current group, which is Group A
b) I switch to Group B or C and choose to undo closed tab.  The action should be ignored, because I have no closed tabs in Group B or C.
c) having switched to Group B or C, I switch back to Group A and choose to undo closed tab.  The action should succeed and the tab should be restored, because it lived in Group A.

SCENARIO II: I am in "Group Your Tabs" view and close one or more tabs, in one or more groups, therein.

Note 1: Since tabs can be closed from "Group Your Tabs" view, it would make sense to have undo closed tab buttons included in each tab group window.

If I browse any of the groups, from which I closed tabs while in "Group Your Tabs" view, undo closed tab should still be bound to only tabs closed within the respective group -- should not act beyond the current group's scope to undo tabs in groups not in view.

If "undo closed tab" and "undo closed group (or window)" buttons are added in the global scope of the "Group Your Tabs" view, then I believe that is the only time undo closed tab/window should act globally.

Note 2: Session restore should continue to work per normal and not be bound by these conditions.

Of course, these are just my thoughts, which carry no weight, and I should don my flame retardant suit immediately upon hitting the "Save Changes" button. :-)
Yes, we should restore to the previous group - this ends up looking a lot like dataloss to users, even though it isn't.
blocking2.0: ? → beta5+
This is a regression, for sure.  To users who don't use tab candy (and I suspect there will be many) it's the only part of tab candy that can affect them, and really really feels like dataloss.  (I couldn't figure out how to get those other tabs back when it happened to me.)

If we can jam a fix in, even at the expense of some tab candy functionality, I think we should.
QA Contact: tabcandy → tabcandy
Attached patch wip (obsolete) — Splinter Review
Kinda fixes it but not really. With the fix in bug 586693, the undo-close-tab tab will appear in the tab strip but disappear on switch because it thinks it has no parent.

With this patch, it'll remember what parent it belongs to but it still shows up in the current tabstrip and disappears when switching away.

The patch works by saving the parent before the tabitem sends "close" to its subscribers -- one of them being the groupitem which will then tabitem.setParent(null) which then calls the Item.setParent -> parent implementations and saves no group id.

Raymond, Ian, any suggestions for better fixes?
After thinking about this more, I believe it needs to block beta4. (Sorry; thought I did this earlier, looks like there was a bugzilla timeout)
blocking2.0: beta5+ → beta4+
Attached patch v1 (obsolete) — Splinter Review
Temporary fix to just keep visible tabs visible when undo close tab instead of hiding the rest of the tabs.

I'll file a followup bug to implement the desired ux of undo close tab should be group-specific and only undo close tabs of tabs in that group.
Assignee: nobody → edilee
Attachment #466410 - Flags: review?(dolske)
Attached patch v1Splinter Review
Apparently hg treats "null" as a special tag.. renamed my mq patch to something else to export :p
Attachment #466342 - Attachment is obsolete: true
Attachment #466410 - Attachment is obsolete: true
Attachment #466411 - Flags: review?(dolske)
Attachment #466410 - Flags: review?(dolske)
Attachment #466411 - Flags: review?(dolske)
Attachment #466411 - Flags: review+
Attachment #466411 - Flags: approval2.0+
Explicitly wipe out tab storage data on close so that undo close tab acts like a brand new tab.
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: b5
Target Milestone: --- → Firefox 4.0b4
Blocks: 587874
tab position in current group after restore is wrong.

if the tab was originally closed in another group then it should reopen as the last tab in the current group
(In reply to comment #15)
> tab position in current group after restore is wrong.
Yes. That was known for this temporary fix. The desired full fix is bug       587874 where only tabs of the current group will get restored, so it will end up in the expected position.
if my browser.sessionstore.max_tabs_undo is 10.
and i have group A,B and C each with 8 tabs , how many closed tab will be saved in the closed tab list for each group.

lest say i closed 3 tabs from group A, then open group D from some news site open 15 tabs then close group D.

How many closed tabs are in the closed tabs list and in what order?
if i switch to group C and i like to restore one of the closed tabs can i do it?
if only tabs from current group can restored, how can i restore tabs from closed groups?
Flags: in-litmus?
Added to Litmus:
Flags: in-litmus? → in-litmus+
Whiteboard: [in-litmus-bug-week]
I am able to reproduce this issue using Mozilla/5.0 (Windows NT 5.1; rv:2.0b9pre) Gecko/20110106 Firefox/4.0b9pre.
Blocks: 623907
I also just reproduced this on trunk, on Mac.
Resolution: FIXED → ---
It's likely this is due to my recent sessionstore-related patches:

2011-01-05 12:54 -0800	Ian Gilman - Bug 617511 - bug fix for test breakage by 594644 r=dietrich, a=johnath

2010-11-03 11:14 -0700	Ian Gilman - Bug 605935 - Use private-browsing-transition-complete instead of sessionstore-browser-state-restored r=ehsan a=dietrich

2010-11-22 16:28 -0800	Ian Gilman - Bug 594644 part 2 r=dietrich a=blocking

2010-10-08 15:59 -0700	Ian Gilman - Bug 594644 - Return from Private Browsing Mode, Panorama forgets tab groups, until Panorama is manually re-launched r=dietrich a=blocking

2011-01-05 12:54 -0800	Ian Gilman - Bug 598795 - Clean up reconnect code once we have instant access to sessionstore r=dietrich, a=blocking
Assignee: edilee → tim.taubert
(In reply to comment #24)

It happens after using Panonama once.

In local build:
build from f453924d5fe1 : fails
build from 8bb78383bdb6 : works
build from ccfc9d214703 : works
build from b06a94065ef0 : works
build from 2d849e2d302e : works
build from bddc89a2200c : works

And related to Bug 623792.
Blocks: 617511
mitcho, ian, tim, since this is a new regression, it should really be in a separate bug, otherwise all the flags and dependencies are getting messed up. (For example, it might get lost from the blocking radar because it's set to block an old beta).
regression of this bug is now handled in bug 624265
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
Whiteboard: [in-litmus-bug-week] → [in-litmus-bug-week][hardblocker]
i am seeing this on  Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b9) Gecko/20100101 Firefox/4.0b9
Resolution: FIXED → ---
Nope.  This bug is resolved, and bug 624265 handles the regression with a fix that didn't make it into 4.0b9.
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
verified with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b11) Gecko/20100101 Firefox/4.0b11
Whiteboard: [in-litmus-bug-week][hardblocker] → [in-litmus-bug-week][hardblocker][fx4-fixed-bugday]
Verified fixed on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110204 Firefox/4.0b12pre
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.