Ability to save session state

RESOLVED FIXED in Camino1.5

Status

Camino Graveyard
General
--
enhancement
RESOLVED FIXED
15 years ago
11 years ago

People

(Reporter: hsivonen, Assigned: Stuart Morgan)

Tracking

({fixed1.8.1.1})

unspecified
Camino1.5
PowerPC
Mac OS X
fixed1.8.1.1

Details

Attachments

(5 attachments, 4 obsolete attachments)

(Reporter)

Description

15 years ago
Galeon and Opera can save the session state when the browser quits so that on
the next launch windows are created at the same positions and the same URLs are
loaded. It would be nice to have this in Chimera as well.

Saving the session is useful if you have to log out and have a lot of windows
open or if you want to update to a newer build. Also, if the session state is
saved each time a new page is loaded, the feature could serve as a crash
recovery method, too.

Comment 1

15 years ago
This seems like a valid RFE for Mozilla in general, and not just Chimera. Anyone
know of a similar RFE already in place for Mozilla?

Comment 2

15 years ago
Perhaps this can be a pref?  Adding [RFE] to Summary line.
Summary: Ability to save session state → [RFE] Ability to save session state
(Reporter)

Comment 3

15 years ago
The Mozilla-wide RFE is bug 36810. But this RFE doesn't need to depend on that one.

Comment 4

15 years ago
May I suggest to add this to Bookmarks rather than a special save as.
So it'll be done by add bookmark as now, just add an additional checkbox
[x] All Windows with all their tabs

In the Bookmarks there is what can be called:
- Saved as One Window
- Saved as One Window with its tabs via checkmark option
Adding 
- Saved as All Windows with All their tabs

I don't think that for All Windows, the checkmark to save their tabs should be
enabled, that would become quite difficult to handle/ask which one should and
which one should not save its tabs.

See also http://bugzilla.mozilla.org/show_bug.cgi?id=156886
to have different icons for each type of bookmark.

Comment 5

15 years ago
I don't think this should be imbued into the bookmark system. Bookmarks don't
(and shouldn't) contain info like browser windows' initial positions and so
forth. Having a lot of bookmark-related options would also cause confusion for
the user.

A session auto-save that gets triggered when Chimera quits would be nicer. You
could just quit the browser (or logout or shutdown)and go away, and upon
returning you could just pick up where you left off with your browsing. That
includes open tabs and open windows.

This is quite similar to what Apple is doing with the latest version of Mail. If
you quit the Mail.app in Jaguar (OS 10.2) while compose windows are open, on the
next launch the windows re-appear and you can just continue working with them.

This sort of behavior is NeXTish, and it's what Apple is aiming for with a
number of changes in the Mac GUI. The Dock, for instance, mixes open apps with
closed apps --the state shouldn't matter, an app should just give you what you
want when you invoke it.
(Reporter)

Comment 6

15 years ago
I agree with Lauri that this shouldn't be handled in the bookmarks system.
(However, Galeon offers to create bookmarks from the previous session when
starting after a crash, which is a nice feature.)

In order to avoid reinventing the wheel, I suggest using Galeon's session
format. Besides, at some point someone is going to share OS X and Linux home
directories over NFS and is going to want to share Galeon and Chimera sessions.

Galeon saves the session in a file called session_saved.xml. The file format is
XML-based and doesn't use namespaces (so it can be parsed with Core Foundation's
XML parser which might be easier to use than expat when the surrounding code is
in Objective-C).

The root element is <session>. It contains <window> elements as children. The
<window> elements are ordered from back to front. The <window> element has four
attributes: x, y, width and height for indicating the location of the top left
corner of the window and the size of the window.

Each <window> element has one or more <embed> elements as children describing
the tabs. The <embed> element has two attributes: title and URL. (Title allow
the page title to be restored before the page has been refetched.)
(Reporter)

Comment 7

15 years ago
I'm trying to implement a simple parser driver for use with the Galeon-style
format and Core Foundation's parser.
(Reporter)

Comment 8

15 years ago
Created attachment 94826 [details]
Standalone code for parsing Galeon-style session files

Problems:
* The code is standalone--not integrated with Chimera
* Apple's XML parser doesn't replace NCR and predefined entities with proper
chars.
Summary: [RFE] Ability to save session state → Ability to save session state

Comment 9

15 years ago
If you only need one session state to be saved, the last one, this can be
implemented in the "File-Open Recent >" menu, see Bug 179673

The sub-menu will be:
Last Session State
-
Last10Closed

It's too bad to just be able to save only one.
Enhancing the bookmark saving by adding ~"Memorize Windows Layout" is simpler to
understand than creating another new system.
Now only Tabs can be saved in bookmarks, people browsing with several windows
opened (and arranged on screen) don't have any equivalent.

Comment 10

15 years ago
Right now, I see this as being implemented in a fashion that wouldn't actually
save the browsers state until the user wants to quit. If a feature such as
session state saving was constantly updated, and a keypress-toggle was added to
the Chimera startup routine, this could help track some of the pesky tabbed
browsing bugs.

My idea is that when Chimera crashes, it could look for a pref to auto-load a
saved state, and if it goes into a crashing loop, a keycombo at Chimera's
startup could force ignore the pref.

I started Bug 181646 in an effort to request such a feature, and I'm not sure if
that's a dupe, or if it's different enough to warrant a separate report.

Ideas?

Comment 11

15 years ago
Created attachment 108649 [details]
Mockup Add Bookmark

Status: the current scroll positions of the page(s)
Tabs: all the tabs in of the window(s)
Windows: all the opened windows
All Tabs and All Windows being disabled according to the situation.

Comment 12

15 years ago
The wording of the options in the mockup is ambiguous and non-descriptive. Bad,
bad, bad. And as has been stated before, the bookmarks system must not be imbued
with magical powers. It has to remain simple and as similar to that of other
browsers as possible.

The most natural way to save the session state is to remember the number of open
windows and tabs and the locations of the sites that were open when the browser
was quit, and reload those sites into their respective tabs and windows upon
launch. In other words, the session should be saved upon quit and restored upon
launch. Nothing more, nothing less. This should be done automatically, without
user input at any point. This is what Mail.app does and is what Chimera should do.

Comment 13

15 years ago
Lauri, I don't want to argue here.
Find better words, I'm from Belgium.
The 'All Windows' is just to be 'fair' with those who don't use tabbed browsing
and would like to be able to open several windows without having to resize,
position everything each time. The Status (scroll) is not so important but would
be great.
Your argument is not correct, bookmarks in Chimera have already introduce an
extended way, tabbed browsing.
What about those who need several arranged windows?

Comment 14

15 years ago
> Your argument is not correct, bookmarks in Chimera have already introduce an
> extended way, tabbed browsing.

No -- Chimera was not the one that introduced tabbed browsing and the
bookmarking solutions that come with it, so that's out of context. Mozilla,
Opera and a few others came before Chimera on this front.

> What about those who need several arranged windows?

I'll take the Mail.app example again. Like Mail, Chimera should  remember window
positions. If you open a bunch of windows in Mail, quit, and reopen, the windows
remember their positions, contents and scroll states. Chimera should work similarly.

Comment 15

15 years ago
Lauri, you are illogic.
>Simple bookmark
Then the other browsers should not had to introduce multi-tabs savings ?
Multi-windows savings is an idea as great as multi-tabs saving.

Chimera will then be the first to introduce multi-windows savings.
The others will follow. If they have not started already, by spying bugzilla.
Or should we wait to see new concepts elsewhere to accept them for Chimera ?
Lead or follow.

>Mail
That's just one session, the last one!
Do you only have one multi-tabs bookmark ?
No, why then should we only have one multi-windows saving ?

Sorry to the devs about my 'arguing' it's my last one.
(Reporter)

Comment 16

15 years ago
This RFE isn't about bookmarking. Bookmarks are things that users specifically
requests to be created and it would be inapproprate to delete bookmarks
automatically.

This RFE is about remembering the session state when the app is quit and
relauched. It would be inappropriate to store this data in the bookmarks file,
because it would either generate a large number of bookmarks that the user
hasn't specifically requested or alternatively the app would have to
automatically remove the old session bookmarks. Neither polluting the bookmarks
with lots of automatically generated bookmarks nor removing bookmarks
automatically is appropriate, because bookmarks are considered to be a
user-managed collection of data.

Ideally, session saving could be extended to work in the background so that it
could be used as a crash recovery mechanism as well (as in Galeon). In general
it is a bad idea to use a single file both for short-term automatic storage and
long-term user-managed storage--especially when losing the long-term storage
would be especially bad for the user. People don't want to lose their bookmarks.
Doing frequent writes in case of a crash increases the likelyhood that sometimes
the crash happens during the write. This could be cured by rotating two files,
but there can be a lot of bookmarks so keeping rewriting all that data to the
disk frequently when the real bookmarks (bookmarks that are considered bookmarks
by the user) don't change doesn't make sense.

FWIW, Galeon allows explicit session saving in addition to the automatic session
saving and the storage and the UI are separate from the bookmark system.

Comment 17

15 years ago
Sorry, I'm new to this system, but I implemented something very much like this.
It auto-saves state to disk every time locationChanged: gets called to a
browserWrapper, and then auto-loads that on startup.

http://www.osxtreme.net/software/Chimera_Custom/

You can try it out, prebuilt Navigator.app is there, as well as the source.
I sent feedback to chimerafeedback@netscape.com, but just got a generic
"thanks for your input" response. *shrug* :)

Comment 18

15 years ago
I was pondering how to avoid a crash-on-auto-load loop for a non-advanced (read:
doesn't want to edit prefs by hand to disable auto-load or edit the autosaved
windows file) user, and came up with some logic that might work:
                -touch file cleanshutdown on succesful/clean browser shutdown
                -if cleanshutdown not present on startup
                        -present the list of windows that were autosaved
                        "Chimera detected an improper shutdown.
                         Remove windows you think will cause a crash,
                         and click OK to proceed or Cancel to abort loading
                         any of them."
                        -allow user to remove (uncheck) some/all via
                                an internal or external plist-like editor
                        -possible to proceed with cancel or ok (up to
                                user pref) after a timer expires?
                -if present, delete cleanshutdown ASAP in case we crash
                -open the remaining windows to autoload

Dunno if the timer is possible or makes sense, and dunno where I'd get a
suitable editor for presenting the Autosaved Window Array (imagining something
that looked like OmniOutliner, for instance)

Comment 19

15 years ago
Henri comment 16 : there is no pollution of auto-saved bookmarks.
Only one is automatically saved, only one is visible in the menu, the "File-Open
Recents > Last  Session". See comment 9.
Internally, the devs can make severals in a cycle and try to go back to the last
valid one.

Comment 20

15 years ago
Created attachment 110642 [details] [diff] [review]
A patch to implement (auto/manual) save/load window state

This patch is to the source.  Binary files, and a copy of this patch, will be
in the next attachement.

Comment 21

15 years ago
Created attachment 110644 [details]
All files needs to install and test my patch

This has:  The same .patch file as above, an additional oldpaths.patch for
anyone who happened to be using my old Custom application (which wrote files to
the wrong place, sorry!), a mainmenu.tgz with three additions to the Window
menu in the .nib file, and, as usual, a .txt file describing the features and
how to install the whole deal.

This implements both a manual save and load window state (which are in the
Window menu and hotkeyed as control-cmd-S control-cmd-O), as well as automatic
saving of state and automatic loading of that state on startup.  auto-save and
auto-load are currently separate options, but could be made a single option if
simplicity is desired over separability.

If you would like a preference pane to test these with, also get my
customprefpane from: 
http://www.osxtreme.net/software/Chimera_Custom/patches/

Comment 22

15 years ago
I've had several good responses on my patch via email.  I'm not sure why a good
crash-recovery feature isn't a priority for a developing browser, but if there's
anything else I can do for this, let me know.
i'm not really a big fan, but i'll keep this around to follow the discussion.
Assignee: saari → pinkerton
Target Milestone: --- → Future
(Reporter)

Comment 24

14 years ago
Pinkerton: Is there something specific about Michael Scheel's code that needs to
be addressed? Why can't it get in?

Comment 25

14 years ago
Why isn't this being pursued? I've spoken to several mac people who have
commented about a feature like this, and now Omniweb has.  But it's been sitting
here for a year gathering dust.  Unfortunately I've been unable to get this
patch applied since there've been so many code changes since.
But this would be a great feature, to leverage against other browsers.

Comment 26

13 years ago
I know this is a bug for Camino but since "Mozilla in general" was mentioned I
thought I'd add that there is a plugin for firefox that does this pretty well:
http://www.extensionsmirror.nl/index.php?showtopic=166

Someone by the alias Pike first wrote it, it was rewriten and enhanced by
someone going by the alias Rue, and then extended (added buttons & shortcuts) by
Rowne-Wuff Mastaile.

Some features:
Remembers all open windows and all tabs
Remembers window-chrome settings (toolbars, etc.)
Remembers window position / size
Remembers every tab's scroll-state
Remembers every tab's history scroll-states (in dev.)
Stores only 1 pref-string per window
Writes prefs to disk for every tab/window open + close
Captures complete session-state on shutdown-request
Writes flag on shutdown-request /
Crash-recovers prior session if flag wasn't written

Comment 27

13 years ago
But of course!
Look at: 63094

(In reply to comment #1)
> This seems like a valid RFE for Mozilla in general, and not just Chimera. Anyone
> know of a similar RFE already in place for Mozilla?

Comment 28

13 years ago
*** Bug 181646 has been marked as a duplicate of this bug. ***

Comment 29

12 years ago
*** Bug 300314 has been marked as a duplicate of this bug. ***

Comment 30

12 years ago
Bump.

Comment 25 seems to have gone as yet unanswered, but this is clearly something
folks want, as evidenced by the dupes it's generating.

I can probably bring Scheel's code up to currency, but I'd like to know I'm not
wasting my time.

cl

Comment 31

12 years ago
It's veru wise to have a chat about this. Last I heard was that the main reason
it never went in or had a serious look at was because it was developed in a time
where such features where not a priority at all.

I agree that from the look of user responce this is something plenty of people
want. But as with all features in Camino it should be as transparant and simple
as possible, please not something like what Omni has.

Comment 32

12 years ago
I don't think we need anything as complex as Omniweb, especially as Camino can save a tab group. 

Restoration of tabs after relaunch (or crash) however would be superb. 

Comment 33

12 years ago
Yeah, I'd be more than happy with just a completely transparent auto-save that
would allow for recovery from crashes. The rest of the stuff is icing on the
cake, but I agree with Jasper that this should be as transparent and as simple
as possible.

If we don't bother implementing the user-activated session state saving, we
should just destroy the file with the saved state in it on a successful quit so
we don't clutter up the profile folder any more than we have to.

cl

Comment 34

12 years ago
The patch as Stephane Moureau isn't bad at all. Basically he created two Window
menu options "Save Window State" and "Open Saved Windows state" 

I think that an invisible autosave when tabs/windows are created and pages are
loaded would be the best way to do a session save/restore. Note that we have the
same kind of approach for downloads right now. We neeed to be carefull to not
impact pageloading, meaning the session save should be an incremental background
process. I'm sure this could be done fast.

Looking at Stephanes patch I think the concept of doing a (one) manual save is
very interesting. Don't get me wrong, the feature as OmniWeb has it is a great
innovation, and of great use in some situations. But I think that Camino's
approach to most features is to make sure that it's functional from the start.
So maybe it's interesting to do the following.

"Auto save session" saves the session inivisbly, and restores after a crash or
whatever.
"Restore session" Restores to the last save state, this could apply to having
closed a window with tabs.
"Save/Restore custom session" I'm not sure we want this, but let's talk it over.

Finally I think this feature should somehow get some integration with Bug
196359. All in the name of making sure people can restore and go back to "lost"
pages.

Comment 35

12 years ago
This discussion reminds me of what John Siracusa wrote at arstechnica a little
while ago in his journal. 

Why not make Camino act more like NetNewsWire in regard to when you quit with
open tabs/windows, on re-launch, that state is preserved.  Maybe that's what is
being looked at, but it's not clear from the discussion.  As a follow-on to
that, if Camino could synchronize of state across computers like NNW does, that
could be a killer feature (like Siracusa points out).  The synching could be
very transparent to the user--an advanced feature, but one that may prove addicting.

Comment 36

12 years ago
Pink, Simon...any comments on whether we'd like to try to get this into 1.1? There seems to be ample demand for it.

cl

Comment 37

12 years ago
Hm, that's a pretty messy patch that needs a lot of cleanup, but the approach seems reasonable.

However, I wonder if we could tack this information into the bookmarks plist (which gets saved pretty frequently).

Comment 38

12 years ago
I guess we could save it in the bookmark.plist but wouldn't that cause all kinds of issues with 3d party apps that import our bookmarks? Especially since not many of them have a clue about what it is?

Or are you thinking of a special category with it own icon in the left column containing folders (session/windows) and bookmark and tabgroups for the saving of the session? But wouldn't we then also have to save window locations and suchs?

Comment 39

12 years ago
You're right; a separate file would be best.

Comment 40

12 years ago
What about using the history to save the information. Simply mark unclosed tabs until they are closed - if camino quits or crashes the history has to be searched for unclosed entries. I don't know whether this could be done efficiently or if there are other problems involved but it sounds like a good idea to me :-).

Comment 41

12 years ago
Using history would be unefficient; you'd have to look through the possibly hundreds of thousands of history entries on launch, looking for the "open" urls.

Comment 42

12 years ago
Having a feature like this would likely encourage more people to download and test unstable builds as they won't have to worry about loosing data.

Comment 43

12 years ago
can you guys piggy back off the firefox sessionsaver plugin? It does this, plus adds syncing across computers. Not really UI dependent, either.
*** Bug 329218 has been marked as a duplicate of this bug. ***

Comment 45

12 years ago
Just like to chime in one my desire for this. It certainly doesn't sound too hard to implement. Just reading the comments it seems a holdup is where to save the session info.
I don't see what the problem is there. I don't expect anything to be added to my bookmarks unless I add it specifically. Just save the session info in Application support as a new/seperate html file.
It's certainly a very, very useful feature. I don't understand why it hasn't been integrated already.

Thanks,
Adam

Comment 46

12 years ago
Taking. I'm gonna see what I can do about bringing the existing patch up to speed. I'd personally really like this for 1.1.

cl
Assignee: mikepinkerton → bugzilla

Comment 47

12 years ago
Targeting for 1.1.

I now have Scheel's code building without errors or warnings, although there's quite a bit of cleanup that needs to happen. I'm dedicating tomorrow to getting this up and running.

cl
Target Milestone: Future → Camino1.1

Comment 48

12 years ago
Status update: I have a functional patch. There are a lot of little gremlins that need to be chased down, though, but nothing that impairs functionality. Lots of performance stuff, that kind of thing.

cl

Comment 49

12 years ago
Created attachment 215530 [details] [diff] [review]
brings Michael Scheel's code up-to-date

This patch brings Scheel's code up to date in the current tree. It's functional, but it's by no means perfect. I've made some notes in the process of bringing it up to speed, presented here roughly in order of importance:

1) We should probably save window sizes and positions. This will likely require the use of an NSDictionary, which I know little about, but I'll look into it if folks agree this should be done.

2) We do not currently know anything about previous crashes that may have happened. We need some means of avoiding a crash loop when autoload is on. Right now, all we do is default autoload to false, which helps a little. ;)

3) We currently save all tabs, regardless of content. We might want to think about not saving blank tabs, other about: URLs, view-source: URLs, etc.

4) The methods that do most of the heavy lifting don't really seem like they belong on PreferenceManager, but I don't have a better suggestion at the moment.

5) We should avoid watching for state changes while we're loading a previously saved session (either autoload or manually).

6) We should consider throwing an error in the following circumstances:

     a) no saved session found when user tries to load (alternatively, and probably better, we should disable the UI for loading a saved session in this case)
     b) trying to auto-load and failing
     c) trying to load (auto or manual) when offline. I've already told MainController not to auto-load when offline.

7) The UI needs some serious thought and discussion, particularly how we'll handle the preference toggling.

8) The |maybeAutosaveWindowStateFile| method has the potential to be a performance issue unless we give it some serious optimisation. Caching/saving the profile path and autosave pref value would be a really good idea here, I think.

I'd like input from Mike, Simon, Stuart, Mark, and the original patch contributor, if possible.

cl
Attachment #110642 - Attachment is obsolete: true
(Assignee)

Comment 50

12 years ago
(In reply to comment #49)
> 7) The UI needs some serious thought and discussion, particularly how we'll
> handle the preference toggling.

There are much more fundamental UI issues. This saves state in terms of URLs, and the model of URL===state is becoming less and less accurate.
- Will users understand that form data won't saved?
- Will users understand that AJAX-y/DHTML pages will likely be completely different on startup?
- Will users understand that sites that have a session identifier might just load up as a "Your session expired" page, or on the front page of the site?

Users have a hard enough time with this as it relates to bookmarks, and I suspect it will be even less apparent that something that is supposed to save their session won't "just work" the way other OS X apps that save and restore state do.

Comment 51

12 years ago
The concerns raised by Stuart should indeed be used to "teach" our users what to expect. That is why I already advised to turn of auto restore, perhaps we should show either a sheet or a window that explaines our users what to expect.


UI wise I will try and make some mockups if needed to see what thing would look like and work, please let me know.

Comment 52

12 years ago
I'm not sure what comments you'd like from me, other than I'm very happy someone is 1) working on this and 2) making use of work I did.  I'm interested in the overall user experience, and in particular, the crash-recovery aspects of this feature.  I understand that it needs a lot of UI help to make it as intuitive as the rest of Camino, and I don't know what to offer, other than I think it is worth it.  Thanks.
One of the things that our friends over at the incendiary mammal are looking at is better use of cache (http://wiki.mozilla.org/Firefox2/Features), so that might help here, though not in the crash variant, unless they fix the crashes-discard-cache issue.  You also might want to chat with autonome next time you catch him on irc to see what else they might have up their sleeves that might benefit us here.

Comment 54

12 years ago
Just throwing in my 2 cents:

Regarding the comment re:saved state = saved URLs... I feel that this still holds true, and most users would understand this on some level. The Firefox session saver works the same, and it's unreasonable to expect we could ever return everything back the way it was, simply due to possible login timeouts etc.
I feel it's really the AJAX/DHTML designers responsability to make sure all the data is kept current. Really this is already the case... If I'm editing my wordpress site and I haven't finished and I try to close the window I get a prompt about losing my data.
If we are restoring a state after quit then this case will have to have been accounted for (user will be prompted on quit) if the state is being restored after a crash, well... it's unreasonable to expect a crash to have no consequences.
(Assignee)

Comment 55

12 years ago
(In reply to comment #54)
> it's unreasonable to expect we could ever return everything back the way
> it was, simply due to possible login timeouts etc.

Users have unreasonable expectations all the time. My point is that the UI needs to be handled carefully to help prevent users from comparing Camino's session saving to the session saving that a variety of other OS X programs do and coming to the conclusion that Camino's is broken.
Comment on attachment 215530 [details] [diff] [review]
brings Michael Scheel's code up-to-date

Two issues with this patch so far: it doesn't actually write out a saved_windows_auto_last.plist file, so it's not really even saving state yet, and when you enable autorestore, Camino creates no window and it is impossible to create a window manually, so the code for handling "failures" in restoring needs to be much more robust (partially 6b).

Comment 57

11 years ago
*** Bug 336631 has been marked as a duplicate of this bug. ***

Comment 58

11 years ago
(In reply to comment #57)
> *** Bug 336631 has been marked as a duplicate of this bug. ***

Sorry about the dupe folks.

Question:  How to implement the patches above?  Save the script or compile it and store it in the Camino folders somewhere?

I myself will not use a browser that does not save my last locations as I need to swap users and reboot to Boot Camp often.  Safari does it with Saft, Firefox does it with an add-on, IE does it with Avant Browser, etc.

Thanks for any help,

Manny
(Assignee)

Comment 59

11 years ago
You patch Camino and rebuild it.  Bugzilla isn't the place to get help with making custom Camino builds though; try irc or the forums.

Comment 60

11 years ago
Now that we have

http://developer.mozilla.org/en/docs/Session_store_API
http://developer.mozilla.org/en/docs/nsISessionStore

in the codebase, do we want to continue working on our own implementation of this, or do we want to use the Mozilla APIs instead? I can see some advantages either way.

Pink, Mento, Simon, Stuart?

cl
(In reply to comment #60)
> Now that we have
> 
> http://developer.mozilla.org/en/docs/Session_store_API
> http://developer.mozilla.org/en/docs/nsISessionStore

"nsISessionStore is defined in browser/components/sessionstore/nsISessionStore.idl"

That doesn't sound like a Mozilla API at all; that sounds like a Firefox-only API.
(Assignee)

Comment 62

11 years ago
Created attachment 243928 [details] [diff] [review]
phase one

Taking.

In order to make reviewing easier and faster for everyone, I'm going to do this in a series of smaller patches.  This is the first step, which creates a class to handle saving and restoring sessions and a (hidden at this point) pref to enable it.  At this point, it will only save on quit, meaning it's not a crash recovery mechanism.

Once this is in, I'll file bugs for the rest, including periodic auto-saving, safe crash recovery, and a GUI pref.
Assignee: bugzilla → stuart.morgan
Attachment #110644 - Attachment is obsolete: true
Attachment #215530 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #243928 - Flags: review?

Comment 63

11 years ago
User's point of view:
As has been pointed out Firefox 2.0 already has some sort of session save. It's only apparent if the application unexpectedly quits though. It doesn't have any options visible to the user.
If it saved state on a standard quit command (Apple Event) it would be perfect.

Comment 64

11 years ago
(In reply to comment #63)
> User's point of view:
> As has been pointed out Firefox 2.0 already has some sort of session save. It's
> only apparent if the application unexpectedly quits though. It doesn't have any
> options visible to the user.
> If it saved state on a standard quit command (Apple Event) it would be perfect.
> 

Yes it does; in the Main section of Firefox's preferences I can choose "Show my windows and tabs from last time" for When Firefox starts:
(Assignee)

Comment 65

11 years ago
Please don't discuss anything outside the scope of what I described in comment 62--the actual mechanism for persisting window state--in this bug.  Anything else, including UI and preferences on save and restore behavior, should be discussed in the follow-up bugs once they are filed.
Created attachment 244043 [details] [diff] [review]
Trunk project patch for phase one
Created attachment 244050 [details] [diff] [review]
Branch project patch for phase one

There's a bit of wonkiness with stack order on the branch on occasion; we think it might be a Gecko focus-stealing bug that was fixed on the trunk.

Updated

11 years ago
Blocks: 358689
QA Contact: winnie → general

Comment 68

11 years ago
This looks like a good start.

Do you only want to restore state when launched directly? What if Camino gets launched via a GetURL event? Do we restore state (in addition to loading the requested URL)? Can the user manually load that last state ever?
(Assignee)

Comment 69

11 years ago
(In reply to comment #68)
> Do you only want to restore state when launched directly? What if Camino gets
> launched via a GetURL event? Do we restore state (in addition to loading the
> requested URL)?

That's the idea, and I've tested using the Open URL service; the state will be restored, then the window opened by the service handler will be pulled to the front.  Similarly with files given on the command line and the one-time-per-release Camino welcome page.

> Can the user manually load that last state ever?

My conception of what we want is something with no user interaction beyond checking a box in the prefs, but if that's not the final goal we can certainly add UI to restore manually in a later bug.

Comment 70

11 years ago
Agreed. When launching Camino by clicking a link in another app, the last session should first be restored and then the clicked link loaded in a new window or tab. Doing anything else could lead to data loss.

Comment 71

11 years ago
Stuart, did you look at Firefox's state saving code? I'm wondering if rolling our own is a good idea, since some aspects of state (e.g. scroll position, form field contents etc) may be hard to save and restore from app-level code).
(Assignee)

Comment 72

11 years ago
I still need to look into that and see if we can make a "what I was looking at" restore instead of a "what pages I had open" restore; that would certainly be ideal. But pretty much everything this patch saves is our chrome information, which Gecko has no knowledge of, so we'll need a way to store it anyway.

Comment 73

11 years ago
Can the code in the (open-source) CaminoSession plugin [http://willmore.eu/plugins/caminosession.html] be used?  It's a good working implementation, IMHO, that would be great to merge into the browser.
(Assignee)

Comment 74

11 years ago
Using other portions of that code has since been proposed by the author, and is now being discussed in a follow-up bug. In terms of what it relevant for discussion here (see comment 65), what I have posted is a working implementation of save/restore; if there are specific portions of the the CaminoSession implementation of the actual save/restore mechanism that you feel are better than what I have posted, feel free to point them out and they can be integrated. There's little value in discarding my implementation out of hand and starting over with the CaminoSession implementation (which, based on having looked briefly at that section of the CaminoSession code posted in the other bug, does not appear to handle some portions of the restoration process as well as this version).
(Assignee)

Comment 75

11 years ago
I've looked into the FF stuff a bit more. I'm still not entirely sure whether we can use it or not, but the way it works is just by dumping the entire window to JSON string and then restoring it from that later. If there is a way for us to call into it at the right granularity, we can just take that string and stuff it into the dictionary, so I'm still pretty confident that this is the right approach, and that adding it later would be no harder than adding it now.

Comment 76

11 years ago
JSON WTF?
(In reply to comment #67)
> There's a bit of wonkiness with stack order on the branch on occasion; we think
> it might be a Gecko focus-stealing bug that was fixed on the trunk.

I was able to reproduce this wonkiness manually this morning when loading my bugzilla tab group and opening a new window to do other stuff while it loaded, so it's somebody's bug but not really related to this at all.  I filed bug 359429 on it.
*** Bug 359627 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

11 years ago
Attachment #243928 - Flags: review? → review?(joshmoz)

Updated

11 years ago
Attachment #243928 - Flags: review?(joshmoz) → review+
(Assignee)

Updated

11 years ago
Attachment #243928 - Flags: superreview?(mikepinkerton)
(Assignee)

Comment 79

11 years ago
So you don't have to say it: I'll add comments for the new class and its methods.
Comment on attachment 243928 [details] [diff] [review]
phase one

+static SessionManager* gSharedSessionManager = nil;

rather than a global, you can make this a static var local to the sharedInstance method.

so this doesn't help at all with crashes, only when you quit. is that another bug or never the intent?

sr=pink
Attachment #243928 - Flags: superreview?(mikepinkerton) → superreview+
(Assignee)

Comment 81

11 years ago
(In reply to comment #80)
> so this doesn't help at all with crashes, only when you quit. is that another
> bug or never the intent?

Another bug.
(Assignee)

Comment 82

11 years ago
Created attachment 245697 [details] [diff] [review]
phase one, v2

As checked in on trunk and MOZILLA_1_8_BRANCH
Attachment #243928 - Attachment is obsolete: true
(Assignee)

Comment 83

11 years ago
Bug 358689 and bug 360839 are the follow-ups
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.1.1
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.