629195 - Restore previous session does bad things if you have an "undo close group" and an app tab

Status: VERIFIED FIXED

(Firefox Graveyard :: Panorama, defect, P2)

Description

[Ian Gilman [:iangilman]]

STR:

1. Make sure automatic sessionrestore is off
2. In Firefox, set up a single window with one app tab and two normal tabs in a single group
3. Enter Panorama
4. Restart your browser (note that this only works on Mac; you can't exit Fx with Panorama up on Windows or Linux for the time being)
5. Enter Panorama (there should be one group with one tab)
6. Make a new empty group
7. Close the original group 
8. Before the "undo close group" goes away, select "restore previous session" from the history menu

Expected result:

The current window should be replaced with a Panorama view of step 2. 

Actual result: 

The group and tabs are restored, but they're in a weird half-way state.

I realize that's a long STR, but each of the elements (being in Panorama when you start, the app tab, the "undo close group", etc) seem to be necessary.

Comment 1

[Raymond Lee [:raymondlee]]

If you undo the close  group after step 8, you would see all tabs are in that group.  I believe the group restored by the session restore has the same group id as the group with the "undo close group" button.

Comment 2

[Raymond Lee [:raymondlee]]

Attachment: v1

Comment 3

[Ian Gilman [:iangilman]]

Comment on attachment 507910 [details] [diff] [review]
v1

Nice fix!

In the test, we need to switch to a public domain license block (see bug 629514); please change this (I know it's news to you).

Also, we're collecting a good set of utility routines in head.js; you could simplify the test a little by using them. Don't bother with this test, though... just mentioning for the future.

Can you verify that the test fails without the patch (seems like it should, but good to make sure). R+ with that, and the license block fixed.

Comment 4

[Raymond Lee [:raymondlee]]

Attachment: v1

(In reply to comment #3)
> Can you verify that the test fails without the patch (seems like it should, but
> good to make sure). R+ with that, and the license block fixed.

Yes, the test fails without this patch.  Updated the license block.

Comment 5

[Raymond Lee [:raymondlee]]

Passed Try

Comment 6

[Raymond Lee [:raymondlee]]

Attachment: Patch for check-in

Comment 7

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/602d24b981e5

Comment 8

[George Carstoiu]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b12pre) Gecko/20110215 Firefox/4.0b12pre

Verified issue and everything seems normal after going through all the steps presented in Comment 1.

Comment 9

[Tracy Walker [:tracy]]

verified per comment 8