Persist deferred session across a browser restart

RESOLVED FIXED in Firefox 8

Status

()

Firefox
Session Restore
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: mossop, Assigned: mossop)

Tracking

unspecified
Firefox 8
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [inbound])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
In bug 476430 we're going to be directing users to restart almost straight after starting their browser. This will currently lose any deferred session, throwing away whatever they were last viewing.
(Assignee)

Comment 1

6 years ago
Created attachment 544643 [details] [diff] [review]
WIP

This is a WIP that basically seems to work. It works by saving any existing deferred session along with the current session. On startup the previous previous session is thrown away unless we're doing a one-time restore in which case it is loaded into _lastSessionState ready for restoring at the users request.

Don't know whether to pursue this or hold off till the app tabs stuff changes
Attachment #544643 - Flags: feedback?(paul)
(Assignee)

Updated

6 years ago
Depends on: 670040
(Assignee)

Comment 2

6 years ago
Created attachment 544654 [details] [diff] [review]
WIP
Attachment #544643 - Attachment is obsolete: true
Attachment #544654 - Flags: feedback?(paul)
Attachment #544643 - Flags: feedback?(paul)
Comment on attachment 544654 [details] [diff] [review]
WIP

So this _works_ for your case, but I'm not 100% sure it's not setting us up for more confusion. It definitely makes this a possibility...

0. user has enabled quit dialog
1. quit normally (don't "save and quit")
2. startup (restore previous session is enabled)
3. decide you don't need that session, just do normal browsing
4. quit, but set resume_session_once (via quit dialog, or maybe an extension)
5. session is restored, but restore previous session is also enabled, which is actually the previous previous session.

I don't want to do this as is, but apart from new topics like we talked about, there's no great way to do this. feedback:ehh?

Poking Dietrich to get another set of eyes on this
Attachment #544654 - Flags: feedback?(dietrich)
(Assignee)

Comment 4

6 years ago
Created attachment 548874 [details] [diff] [review]
patch rev 1

This should work better, clears the deferred session on shutdowns that aren't restarts. This makes the restore-next-time button in the quit dialog behave better.
Attachment #544654 - Attachment is obsolete: true
Attachment #548874 - Flags: review?(paul)
Attachment #544654 - Flags: feedback?(paul)
Attachment #544654 - Flags: feedback?(dietrich)
Comment on attachment 548874 [details] [diff] [review]
patch rev 1

Review of attachment 548874 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. My only nit is that you add _lastSessionState to the SessionStoreService prototype along with a comment.
Attachment #548874 - Flags: review?(paul) → review+
(In reply to comment #5)

> Looks good. My only nit is that you add _lastSessionState to the
> SessionStoreService prototype along with a comment.

D'oh. That's there already. But do add a comment that it's persisted sometimes (primarily) to make the addon restart case better.
(Assignee)

Comment 7

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/086328df641d
Whiteboard: [inbound]
http://hg.mozilla.org/mozilla-central/rev/086328df641d
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 8
You need to log in before you can comment on or make changes to this bug.