488238 - Blank about:sessionrestore

Status: VERIFIED FIXED

(Firefox :: Session Restore, defect, P2)

Description

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: Screenshot of a blank about:sessionrestore

I encountered what the screen shot displays.  There must have been two crashes in a row since I am getting about:sessionrestore, however it doesn't look like I actually had a session to restore (I'm on OS X and regularly close out all my tabs by hitting command-w).

So my best guess is that the browser crashed twice due to a reason unrelated to my tab set (I didn't have one), and then I was asked if I wanted to restore my session, even though I didn't have one.

If that is the sequence of events that produced the empty about:sessionrestore, we should probably check to see if there is a session before we ask the user if they want to restore it.

Comment 1

[u88484]

Can't you still have a session with one window with one tab?  I would think Firefox should offer to restore your session even if only one tab was open, epecially if you had input data into a form.

Comment 2

[Simon Bünzli]

Attachment: obvious fix

Right, it doesn't make much sense to display about:sessionrestore when we don't have data for any open windows at all.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Under which situation someone can see this blank page on OS X? I tried but I wasn't successful.

Comment 4

[Dietrich Ayala (:dietrich)]

Comment on attachment 372628 [details] [diff] [review]
obvious fix

r=me

Comment 5

[Dietrich Ayala (:dietrich)]

setting in-testsuite? to flag for eventual testing with windmill or something like that.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

For Mozmill we still use the in-litmus flag. But I need clear STR for that. Will an unit/mochitest test also be possible?

Comment 7

[Simon Bünzli]

StR (only works on OS X):
1. Close all Windows.
2. Wait a few seconds, then kill Firefox.
3. Restart Firefox and repeat these steps once.

Expected result:
Restarting Firefox again should load just a blank page (i.e. nothing to restore).

Actual result:
about:sessionrestore is displayed with an empty list.

(In reply to comment #6)
> Will an unit/mochitest test also be possible?

No. Unit tests don't work for when a crash/restart is required.

Comment 8

[Nochum Sossonko [:Natch]]

It still is All->All as this can be done on Windows/Linux if you open the error console and close all the browser windows.

Comment 9

[Simon Bünzli]

(In reply to comment #8)
> It still is All->All as this can be done on Windows/Linux if you open the error
> console and close all the browser windows.

No, in that case Firefox should (offer to) restore the last closed browser window. Bug 481090 changed this specifically for OS X.

If you do have StR for this on Windows/Linux, this would be a different bug.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #7)
> StR (only works on OS X):
> 1. Close all Windows.
> 2. Wait a few seconds, then kill Firefox.
> 3. Restart Firefox and repeat these steps once.

Those steps work fine. Probably I haven't waited long enough during my last test. Readjusting flags. We will add a test to Litmus.

Comment 11

[Simon Bünzli]

(In reply to comment #10)
> Probably I haven't waited long enough during my last test.

Yeah, with "a few seconds" I really meant "ten (resp. one thousandth of whatever browser.sessionstore.interval was set to at the last regular shutdown)".

Comment 12

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/e6ad6f4cbb70

Comment 13

[Misak Khachatryan]

Sorry for late comment on fixed bug, but while porting this bug to SeaMonkey Neil suggested:

Does this work if rewritten as:
if (!aState.windows || !aState.windows.length)
  return false;

let winData = aState.windows;
if (winData.length == 1 && winData[0].tabs &&

https://bugzilla.mozilla.org/show_bug.cgi?id=488669#c1

Comment 14

[Dão Gottwald [:dao]]

It seems that Neil wanted to get rid of "|| null". But that can be removed without the other change, which just makes the code less convenient.

Comment 15

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/687e516cb5ff

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Dao, see comment 9. This is OS X only.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Verified with:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090420 Minefield/3.6a1pre ID:20090420031158

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b4pre) Gecko/20090420 Shiretoko/3.5b4pre ID:20090420031111

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Litmus testcase added: https://litmus.mozilla.org/show_test.cgi?id=7665

Comment 19

[Alex Faaborg [:faaborg] (Firefox UX)]

This bug's priority relative to the set of other polish bugs is:
P4 - Polish issue that is rarely encountered, and is easily identifiable.