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

VERIFIED FIXED in Firefox 4.0b12

Status

defect
P2
normal
VERIFIED FIXED
8 years ago
3 years ago

People

(Reporter: iangilman, Assigned: raymondlee)

Tracking

unspecified
Firefox 4.0b12
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

8 years ago
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.
(Assignee)

Comment 1

8 years ago
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)

Updated

8 years ago
Assignee: nobody → raymond
(Assignee)

Comment 2

8 years ago
Posted patch v1 (obsolete) — Splinter Review
Attachment #507910 - Flags: review?(ian)
(Reporter)

Comment 3

8 years ago
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.
Attachment #507910 - Flags: review?(ian) → review+
(Assignee)

Comment 4

8 years ago
Posted 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?
(Assignee)

Comment 5

8 years ago
Passed Try
(Assignee)

Updated

8 years ago
Status: NEW → ASSIGNED
Attachment #508316 - Flags: approval2.0? → approval2.0+
(Assignee)

Comment 6

8 years ago
Attachment #508316 - Attachment is obsolete: true
(Assignee)

Updated

8 years ago
Keywords: checkin-needed

Comment 7

8 years ago
http://hg.mozilla.org/mozilla-central/rev/602d24b981e5
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b12

Comment 8

8 years ago
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
Status: RESOLVED → VERIFIED
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.