Closed Bug 629195 Opened 12 years ago Closed 12 years ago

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


(Firefox Graveyard :: Panorama, defect, P2)



(Not tracked)

Firefox 4.0b12


(Reporter: iangilman, Assigned: raymondlee)




(1 file, 2 obsolete files)


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.
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.
Assignee: nobody → raymond
Attached patch v1 (obsolete) — Splinter Review
Attachment #507910 - Flags: review?(ian)
Comment on attachment 507910 [details] [diff] [review]

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.
Attachment #507910 - Flags: review?(ian) → review+
Attached patch v1 (obsolete) — Splinter Review
(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.
Attachment #507910 - Attachment is obsolete: true
Attachment #508316 - Flags: approval2.0?
Passed Try
Attachment #508316 - Flags: approval2.0? → approval2.0+
Attachment #508316 - Attachment is obsolete: true
Keywords: checkin-needed
Closed: 12 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b12
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.
verified per comment 8
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.