<quote src="news://news.mozilla.org/3900EFDB.B3117412@gwu.edu"> Subject: Failure Tolerance of Netscape Date: Fri, 21 Apr 2000 20:18:35 -0400 From: Ben Ruppel <slate@gwu.edu> Newsgroups: netscape.public.mozilla.seamonkey [...] I am trying to think of ways to make the imminent Mozilla crash less frustrating, because when Netscape 4.x crashes and I loose 15 some pages and the message I've been typing, it's quite enraging. [...] I think Mozilla should keep a persistant record of open windows. It should generate an html file that is updated every time the open URL's change. This should be accessable from the browser so we can get the heck back to what we were doing without searching the history or hunting for the links again. </quote> (Also see follow-ups) Such a feature could save me a lot of time (for reconstructing 10 windows 3 times a day or so). I don't even think, it's that hard to implement. Maybe, you just have to hook into some URL loading function (anywhere high-level, i.e. in the browser?), update the state and save it. Maybe I'm naive, but why can't you just write file B and then copy B over A? The copy operation is pretty atomic, isn't it? But I'd capsulate the save operation, so there can be platform-specific implementations, which don't go to a harddisk.

Oh boy, I just lost 4 paragraphs on this because mozilla forgot my text field when I hit the back button when I tried to send the message but didn't have the password.. here goes again... Okay, it should keep an html log. Security nuts won't like it so it'll be optional. The log could have a closing tag that mozilla puts in when properly shut down. If no closing tag is found, mozilla would know not to overwrite the existing tags and to open it up upon launch, allowing for link recovery. But then things can get sticky, as what happens if it crashes halfway through restoring those links? Would session number 2's history overwrite the first one's? What would be recovered on the third restart? Obviously, multiple logs should be kept. User definable limits, like save my last 10 or 100 logs. There could be a menubar button called "sessions" or something, and the dropdown box would contain the last X logs, clicking on one would open the pages full of the urls that were open. A little complex, make it an option for power users only and nobody gets hurt, and mozilla will have an incredible incredible incredible (did I say incredible?) advantage over internet explorer. The code wouldn't be that hard, I curse my choice of mechanical engineering over comp sci!

Okay, I was thinking of creating the concept of "sessions" in mozilla. Basically, besides keeping these log files of html pages open at a time, a user could at any time create a "session", which could be added to a menu that works much like bookmarks work. In the "sessions" menu or button or whatever, there would be a section of user save sessions and sessions from recent crashes. There would have to be a distinction between loading and viewing a session, as loading would open up the links, each in a new browser window, while viewing it would open up an index page full of the links. I really think each of these options would be neccessary in order to make sessions the most usefull. Furthermore, with the loading sessions concept, people could open up the exact pages they want automatically upon opening mozilla. I've heard calls for this forever. Most people first start up their browser to check a preset set of pages, and this pefectly enhances that process. Configuration would be done in a command line, such as " mozilla -session compnews". So if I wanted to read computer news, I could make a shortcut for it and there you go. People could have a bunch of mozilla shortcuts, each having certain groupings of what they want to see. The many ways this could be used astounds me. What does everyone think?

Alphanumerica is working on it (crash recovery). Maybe, David can keep slate's session management ideas in mind and we file a new dependant bug for them?

adding helpwanted keyword and assigning to nobody@mozilla.org where a bunch of the helpwanted bugs live.

Sorry for the spam. New QA Contact for Browser General. Thanks for your help Joseph (good luck with the new job) and welcome aboard Doron Rosenberg

REASSIGNing to Pete Collins, who's "Total Recall" fixes this bug. Pete, could you take care of the checkin (i.e. get a review, approval by brendan or waterson and find somebody to check it in), please?

Ben, I'll talk to Brendan and Waterson about this bug and checking in the fix. I have this sitting in my tree on my unix build but i need to update it to the latest version, as well as find out where to put it in the mozilla menus. I'll gladly accept this bug. ;-) pete

> i need to [...] find out where to put it in the mozilla menus. Preferable something where I can make it go away during the session after the crash.

accepting bug. I won't have time to do this until after PR2. pete

accepting bug

Does it still work with M18? can we get this in, now? I don't have the time to catch every build, but the number of crashes make the browser still unusable for me.

em, patch rather. pete, if it still works, you just need to find somebody to review it and check it in. Not much work for you.

No Ben, I talked to Waterson and Brendan. What i need to do is move the code over to extensions and add the necessary makefiles so this can be a build flag. I is not going to be be part of the default build. --pete

> over to extensions and add the necessary makefiles ic. > I is not going to be be part of the default build. Why not? IMO, this is one of the coolest non-Netscape contributions Mozilla has seen.

Well, i think everyone wants to test it out and make sure it is solid before any merging is done. They are past feature freeze right now i beleive. pete

Ok, i have this ready to go . . . I mailed Brendan for approval. pete

any development on this?

ping?

I don't think we are going to see any checkins for a while. I can't get anything reviewed. Everyone i think is pushing hard to get a browser out the door. --pete

Don't matter. Mozilla live must go on. Who is it who doesn't answer?

It is up to Brendan or Waterson i beleive . . . right? --pete

Pete, see <http://www.mozilla.org/hacking/reviewers.html>.

Gimme a patch to review. Or give a better reviewer from the list, by area. /be

I am not going to be able to do this anytime soon . . For those interested, there is a full WORKING IMPLEMENTAION of this in Aphrodite 0.05. I did some updating today so it is working again. --pete

What is the current status of this bug? What would I have to to to add the code to my source tree (on Linux)?

OK, i'll put this together as an extensions package as soon as i can. Then perhaps people can poke and tweak. Thanks --pete

is this dead?! i would love this (lost 23 pages just a few minutes )8 mozilla is alot more stable, but now when i lose it, it take down too many hours of surfing, too many pages... and would be a great thing to "pause" the surfing (changing computers, unstable PC, boss coming, etc), just close, run later and all the pages back what is needed for this enter in mozilla tree?! i could try to help, but i dont know C and i dont know what to do to help, but i learn fast 8)

Basically what this implementation did is on each page load it queried the open windows through nsIWindowMediator and saved things like the url, title, width, height and name to a crash.rdf file in the users moz home dir. It is really not hard to do at all. You don't need to implement anything in C, it can all be done in js. I'll attach the old js file used in aphrodite. --pete

I started a project here but haven't worked on it in a while. http://recall.mozdev.org/ This was going to be an xpi package. --pete

*** Bug 84914 has been marked as a duplicate of this bug. ***

This bug better not be dead -- mine was changed to being a duplicate of this one so I'm watching for new details. My proposal(s): 1) have a manual "save session (as...)" option that allows the user to define sessions of browser windows. In the save dialog, allow the user to request the windows' positions and state to be saved or just the URIs. 2) have an automatic session maintenance function that uses the same back-end function as #1 above, but runs whenever the user changes URIs. This need not be intensive -- we are already dealing with history information. There is also no need for this to be stored seperately from history information to some degree -- marking pages in the history with a session identifier should be sufficient, and then not removing session-related pages from the history. Sessions should be visible from within the Bookmarks view and loading a previous session should offer the option of either replacing all current browser windows or being in addition to those already open. Eventually adding a back-end "make this page available offline" function like the one in Internet Explorer&tm; would help as well, since the session caching routine could then mark the respective URIs for local (semi-permanent) caching (if not just by markers stating to not expire the data, until reloaded of course). For those of us who spend much of our time with the same sites open in session-like activities, bookmarks are frustrating and almost useless compared to being able to resume a full arbitrary session. Since the browser still crashes from time to time (depending on what you do with it), automatic sessions would also be wonderful for the testing community (--resume-session=last or some such command-line option).

Michael, if a bug is open then by definition it is not `dead'. But the speed at which a bug gets fixed depends entirely on the number of people wanting to fix it (*not* on the number of people wanting it fixed). At the moment no-one is willing and able to implement this RFE, which is why it is marked `helpwanted'. --> XP Apps

the <a href="http://recall.mozdev.org/">recall</a> isnt working anymore with the new mozilla, i think that the tab support broke it

marking future

*** Bug 113494 has been marked as a duplicate of this bug. ***

*** Bug 115790 has been marked as a duplicate of this bug. ***

-> default assignee

->future

Maybe use X session management on Unix/X11? <http://developer.gnome.org/doc/GGAD/sec-sessionmanagement.html>

If I shut down right after I finish loading http://www.webstore.com/purchase.cgi?item=fedora&one_click_shop=1, will I buy another hat when I start up the next time?

That store is broken. You should never buy using GET. What happens, if you bookmark that page?

I'm sure the store is broken, and that I should never buy with GET. My point, perhaps lost in the fact that I chose an example, which can be deconstructed and disputed in its meaningless component parts rather than taken as a representative of a class of concern, is this: not all URLs are idempotent. Various server-administration URLs for Netscape web servers, back in the day when they existed, were GET- and query-string-driven, probably precisely so that you _could_ bookmark "restart server" or "reload app" for ease of use during development and debugging. (Bookmarking the page is a much more explicit act than shutting down when I forget that I have a window open on another desktop there.) Maybe we don't care, and caveat user who turns this feature on (in which case, perhaps we should have a caveat-window pop up the first time it's enabled for a profile), but I'd like to at least see someone say that. There had been no mention of this in the course of this bug, that I could see, and I just wanted to make sure that we took it into account. I want this feature too, and I think the Unix session-management angle is a good one for platforms that support it (if not, none of your other apps have implicit cross-session state either). Of course, I don't think it's going to happen for 1.0, unless someone comes up with a miracle patch in the very near future -- minimal, non-intrusive and easy to subject to large-scale testing. (And I sure wouldn't hold Mozilla 1.0 if this were the last bug left.)

shaver, I indeed misunderstood you, thinking that you argued for wontfix. Yes, you are right, this is something to look out for. Maybe a confirmation dialog (when the browser is reopened) like Mozilla does for POST-Reloads now (because it can't cache them). Maybe that's overkill, however, considering that most queries (GETs) are probably harmless and useful, while dialog boxes are annoying, esp. with session managment (you just logged into your machine and Mozilla asks 10 questions).

If it is a true "come back to where I left off" then I don't think, ideally, Mozilla should re-fetch pages from the network on session resume. Just like we don't re-fetch the page when the user switches tabs or windows.

*** Bug 128423 has been marked as a duplicate of this bug. ***

*** Bug 127637 has been marked as a duplicate of this bug. ***

Once this feature request is implemented, we should be able to resolve bug 19454, crash recovery, and save session information, including URI's automatically in the event of a crash. We should also be able to resolve bug 63094, ability to manually save session information.

*** Bug 146170 has been marked as a duplicate of this bug. ***

Would the desired functionality on the saving of open window data not be the easiest part? Writing a file each time the browser changes pages / opens new windows could be done in the history-handling routines. The reloading and positioning of those windows may be more difficult, but it can be left out while the former is tested; once the information is being tracked, using it is a lot easier.

I think saving URL of open windows and tabs is now very easy, because now you can add all open tabs as group in bookmark (menu "Bookmarks"->"File Bookmark..." and chechbox "File as group"). We need simple program, that uses this automatically, when user opens new window or new tab.

Cool! I'm not even sure this bug needs fixing anymore :-)

We still need "autosave" to work. That functionality can probably be built on top of bookmark groups.

I agree with Andrew Hagen

*** Bug 149002 has been marked as a duplicate of this bug. ***

*** Bug 155319 has been marked as a duplicate of this bug. ***

Re comment 51: this is about autosaving for purposes of crash recovery, not saving lots of user-preferred pages in the bookmarks file easily. Total Recall still works ok for me on 1.0, as long as I don't worry about loing all but one tab from each window :|

Crash recovery is the killer feature that keeps me using Galeon. Why isn't there support in the mainline Mozilla code for good crash recovery like Galeon has? This shouldn't be an add-on, but core code...

deven, we accept patches.

Well, I'd love to offer a patch, but I don't have one. Moreover, I wouldn't know where to begin in terms of creating one. I'm an experienced developer, but there is a barrier to entry here. I realize it's not an intentional barrier, but it's there nonetheless. Basically, Mozilla is so large and complex that it's hard to get started at working on the code, especially for people who have little time to spare. I've wanted to work on various things in Mozilla for years, but I haven't had the time to familiarize myself with this million-line codebase or the many APIs and standards Mozilla uses or establishes. Maybe I'm just missing it, but at the moment it's not very clear how to effectively get involved without a large time commitment...

I think that, even if it's off topic part of Comment #60, Deven is right. I have similar problem. I'm working with many big projects in many different languages, but Mozilla is really huge. Even if i read a part about downloading a source, and building (part for Windows is not true). But i think there should be some more or less simple tutorial "how to change one simple thing and view results". Back to bug - wouldn't reslove problem simple "dirtyhack" with using some hidden "Group of Bookmarks"? And after crash this group should be placed as Start Location.

This would be a killer feature.

Nav triage team: nsbeta1-

*** Bug 187175 has been marked as a duplicate of this bug. ***

*** Bug 193302 has been marked as a duplicate of this bug. ***

For the non-crash case, see attachment 94617 [details] (of bug 39057) for an example of how the UI could be implemented (the attachment is a collage of screenshots of Opera). For the crash case (and, actually, perhaps for non-crash too), please note that if we are to preserve all state (and I would love that), it's not enough to do what manual bookmarking would do: the browsing _history_ of the windows/tabs should also be preserved, so that the back/forward buttons work as before the crash. Also, cookies that are set to expire at end of browsing session should be preserved. And data in forms, too. And any downloads should be resumed, but there's a separate bug for that capability.

Those would all be nice, but I don't think they're essential or a necesary part of this bug.

Here is one possible solution. First, we add an additional flag to each history.dat entry. Its state will be either 0 or 1. When a user browses to a page, the history.dat entry that is generated has its state set to 0. Once the user closes the tab or window, or browses to a new site in the existing tab or window, it is set to 1. When you need a list of all pages that were open before Mozilla crashed, query history.dat for all entries with flags set to 0.

Good idea.

*** Bug 200723 has been marked as a duplicate of this bug. ***

This bug is already fixed - just install Tabbrowser Extensions: http://white.sakura.ne.jp/~piro/xul/_tabextensions.html.en these extensions add lot of useful functions to mozilla: - Auto Reload (Reloads tabs with favorites interval) - You can open last visited tabs instead of last visited page when Navigator starts up (also browsing history is saved !!!) - Also this can restore tabs at the next startup of crash - Tabbrowser Extensions autosaves URL of open web pages !!! (browsing history is saved too !!!) - this closes bug #36810 (Multi-tabbed windows should confirm close) - You can undo "Close Tab" until fifty times ago - You can re-open the closed tab by Ctrl+Shift+Z or Alt+Z - Tabs are moved and re-ordered with drag and drop. - zillion of other improvements. Mozilla developers, please include this wonderfull utility into Mozilla.

Sorry but even if Egle's suggested extension solves the problem, the solution must become a core part of Mozilla. Is this extension part of the build testing? If so, when could this capability be merged into the trunk? From reading comments, it sounds as though there have been several patches for this problem. How many hours have been lost by this basic missing capability in the three years since this bug was reported? Personally, I think the severity of this should be 'major', not 'enhancement' since basic instability and lack of robustness prompts people to look for alternatives that don't cause them to lose work.

I agree. When I am working on something very important, I choose Opera (6.05; not sure if the newer is as fault tolerant). If I need to be able to save whole web pages with images in one file, I use IE. Very inconvenient...

Is Pete Collins' recall.mozdev.org a dead project? does it still work with current builds? Prog.

It's dead in the respect that I can't make the time to work on it. It is a great starting point if anyone want's to adopt the project.

I do think that this is one of the most important features when it comes to making people move from Opera/IE to Mozilla. I for example (and a lot of my friends, and I mean A LOT), still mainly use Opera for browsing as I simply had entering my 5 favorite sites addresses everytime I start my browser. Though I never think that this feature will make it while the Mozilla project still exists (because of the new 1.4+ branch with the Phoenix replacement), I still hope that it will be implemented soon.

*** Bug 207082 has been marked as a duplicate of this bug. ***

Still no progress on this non-capability? Yet work continues on other non-essentials. Sorry, but Mozilla's instability and lack of robustness is becoming unbearable. It's been more than three years now. Is it really that hard to maintain a file that contains the URLs of all open windows? Even a partial solution would be better than the nothing that exists now! I've been a loyal user since Netscape 0.9 but losing work constantly is getting really old... The severity of this bug must be elevated to 'critical'. By not fixing this problem, crashes are causing a major loss of data.

Pete Collins updated the recall to work again in recent builds and with tabs... check http://bugzilla.mozilla.org/show_bug.cgi?id=19454 and send him some beer... also check recall.mozdev.org isnt perfect, specially for tabs, but works... now people can hack it to add more things, fix bugs, etc

Does recall work for Firebird?

I was wondering about the relation between this bug and bug that you pointed out now: http://bugzilla.mozilla.org/show_bug.cgi?id=19454 Above on this page, it shows 36810 block 19454, but they seem more like duplicates to me. If 19454 is solved, this should also be assumed to be solved, and vice versa. So shouldnt they be marked duplicates? And yes, I think both of these should be marked "critical", loss of browsing information is actually critical, imho. (I havent check recall yet)

Let's keep in mind that projects like Recall are not solutions to this bug, and all of us saying how much we want this feature won't make it materialize. I just wanted to mention that the TabBrowser extension (for Firebird) can save your tabs and reload them the next time you start. It doesn't always work when Mozilla actually crashes, but it's better than nothing. What I'd like to see is saved history in each tab, but that's another dream I'm not helping materialize.

Recall <http://recall.mozdev.org/> installed a Jar file but it doesn't seem to do anything. Hell, it took five minutes to even find mention of it on UI. Don't seem to be able to turn it on and crashing the browser doesn't invoke it automatically. I'm also not sure I'm following what exactly it does since there didn't appear to be any documentation either. Was there some configuration required? Since various sites across the internet will cause Mozilla to crash on my machine (e.g. http://ericjohnson.com), restoring all open windows will leave me right where I started. What's the name of the file containing the open window history? For the record, even assuming this extension can be made to work, I don't find this a general purpose solution unless it's integrated into the product. For that matter, does it need to be reinstalled every time I upgrade this browser?

install the recall XPI, it will install the javascript lib that he uses also... restart the browser now in the tools menu you have a recall item check the on/off to enable the recall funcionality browse the web, open the tabs, new windows, etc see that you have one item in the recall sub-menu for each windows... for tabs there is no diference, its stays the same "crash" the browser (usually is some pluging crashing, not really mozilla) reopen the browser go to the recall menu, click on the itens with the name of the windows you want to recover, they will popup with only the first tab loaded... it appers that is all over, but wait... wait a few seconds/minutes (on a slow computer 8) , the more tabs you had on that windows, the more it takes loading all the tabs will show up at the same time, all loading do the same for the others windows you want to recover if you start browsing the web without recovering, i think you will lose the saved windows and tabs

Well, upgrading from Shockwave Flash NP-PPC 6.0 r29 to. Shockwave Flash NP-PPC 6.0 r79 seems to have fixed the crashing problem with http://ericjohnson.com/. >now in the tools menu you have a recall item >check the on/off to enable the recall functionality Now this is where I ran into problems. The menuitem label is literally "Off/On" (as opposed to just "Off"). But selecting the menuitem never produces a checkmark (nor, more logically, just changes the menuitem label to "On"). The other menuitem (labeled ".Restore All") is always desensitized. Any ideas? Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.4) Gecko/20030624

Any possibility my inability to get Recall to function is related to this? <http://www.mozilla.org/projects/plugins/plugin_scripting_ABI_technote.html>

No, it's talking about a different kind of plugin. You cannot use such an old add-on with new Mozillas, way too much changed. Don't even try.

*** Bug 218985 has been marked as a duplicate of this bug. ***

This could be considered a privacy issue and we'd need to make sure people realize that this information would not be used in any way by Mozilla.org

*** Bug 220754 has been marked as a duplicate of this bug. ***

*** Bug 224523 has been marked as a duplicate of this bug. ***

Recall is back in action. I updated it last week to work w/ Mozilla 1.5 +. http://recall.mozdev.org/ Any bugs, RFE's etc, file them on mozdev. Regards

Voting there, but I liked the bug #220754 phrasing better (or Camino #152147 bug description). I am much more concerned about keeping as much of the UI layout (at least open tabs and windows with respective URLs, possibly pages history also) when quitting. Crash recovery is not much of an issue in my experience (I found Mozilla pretty stable for years now). But as a end user, I often restrain myself from quitting mozilla for fear of losing my navigation state. Most important is a 'transparent' solution to the bug (plain quitting and restarting, no menu to invoke, no bookmarking tab groups for every window).

That would be session management, probably.

I found bug #93789 regarding X11 session management, and it indeeds points here. So I guess those things are related. However the proposed solution should be cross-platform, not X11 only.

Manually saving session information is bug 63094. See meta bug 19454.

blocking? you're kidding. there's no way this should block a release.

The browser pages need to be saved not only in the event of crash, but also for future browsring this group of pages, when creating a group does not justify it. Just like having the feature of last page open, restored if the browser is exited, we need to have a group of pages restored in the event the browser is exited.

We're not likely to hold 1.7 alpha for this enhancement.

*** Bug 232994 has been marked as a duplicate of this bug. ***

(In reply to comment #99) > We're not likely to hold 1.7 alpha for this enhancement. Didn't figure you (all) would since it didn't hold any other release prior (since 2000) to this either. Since no work is apparently ever going to be done on this, perhaps it's just time to give up hope that setting flags in bugzilla might get this ~four year old NEW bug fixed. For any Mac OS X users out there, OmniWeb 5 is available and contains saved state as one of its major features. Read about workspaces in the review; even given quibbles concerning current implementation, this far exceeds anything currently available (or possibly ever will be) in Mozilla. Imagine not having to remember which 25 pages you had open when it crashed, nor having to hope your kids couldn't hear when the expletives spewed... <http://www.arstechnica.com/reviews/004/software/mac/omniweb/ow5-4.html>

I wouldn't jump to the conclusion that no one will ever fix this. Right now Mozilla is going through a major foundation and we need to get Firebird so it replaces Seamonkey, stabilize code, and start thinking about freezing APIs for 2.0 Following all that, you'll probably see another massive influx of experimentation and changes just like after 1.0 I wouldn't have such a gloomy outlook.

Is there more to this bug at this point than getting Total Recall bundled, probably with its own Preferences section? Then some decision as to whether it's on or off by default?

*** Bug 236054 has been marked as a duplicate of this bug. ***

There is also a Firefox extension. http://members.lycos.nl/mozillafirefox/forums/index.php?showtopic=166&hl=saver We don't want to just bundle an extension. We would like, however, to integrate their code into Mozilla so it fits naturally.

In the event of "switch profile" the URI's should be saved as well in order to restore the workspace.

*** Bug 245512 has been marked as a duplicate of this bug. ***

*** Bug 250853 has been marked as a duplicate of this bug. ***

*** Bug 272036 has been marked as a duplicate of this bug. ***

*** Bug 283706 has been marked as a duplicate of this bug. ***

(In reply to comment #110) > *** Bug 283706 has been marked as a duplicate of this bug. *** You would think after the dozens and dozens of duplicate requests to fix this somebody would actually get around to doing it. I looked through most of the posts and noticed a good chunk of them are just there saying "Bug X is a duplicate of this bug." Why does nobody want to fix this after 5 years? It seems like a huge feature that a lot of people want in Mozilla's core that competition already offers. Is the code for something like this just that hard to add in? Nick

no. why don't you do it?

(In reply to comment #112) > no. why don't you do it? Because I am the end user of the product, not the designer or a programmer for the project. I am merely expressing my opinions as to what would make the product, Firefox/Mozilla core, better. It seems as though a large number of people have reported this request and that means there are hundreds, maybe thousands, more who haven't reported the request but desire the feature. The definition of an end user is as follows: "The ultimate consumer of a product, especially the one for whom the product has been designed." Open source project or not, saying "fix/make it yourself" to someone who doesn't work on the project is quite silly in my opinion. If I were able/willing to fix it myself, why would I report it here as a request to the project members? Nick

He was merely saying this, because you appeared to "blame" the developers for not putting in the fix imho. And in open source, free as in speech and in beer, the developers are not "responsible" for fixing it to the users, since what they are getting from it is not substantial. I have always been confused about "open source" in general -- and the concept doesnt really make sense to me. In fact, the supporters of "open source" seem to be nothing more than asking "Give It To Me For Free!!!", something like "demanding a Good Sale/Discount Deal" on the software. (Sorry for ranting on a bug....if you need to reply to me, plz email me personally.)

sorry about the spam, but i want to clarify something: most people here already have some kind of soluctions, what they want its to make mozilla and firefox better, not nag the dev team just for fun... timeless: the main problem is that there is already support for this, recall (dont know if still works) and session saver (that i use in firefox and works fine, better than recall, and works in mozilla also) what is need is to include this as default!! if people dont want to be able to save sessions, at least crash recover... i know no one that after a browser crash didnt want to recover the open pages... a simples question restarting the browser asking if they want they last state restore would allow a exit on saved "crash all the time" page a simple warn "some data may have been lost"; would make invalid any bug about not restoring form state, in page locations, whatever... its also small, the full session saver is 53Kb xpi, the lite was about 20Kb IIRC mozilla and firefox install the quality feedback, so why not include session saver lite (the one that only restores crashs)? those that dont want it could disable in the same way as the quality feedback i guess that most of the users would prefer crash recover than the quality feedback i already use session saver, but most of the mozilla and firefox users dont know about it, that is why this should be on the default install if mozilla team want firefox with at least 10% of the web browser share, this would help for sure, if not for anything, at least it would stop people going back to IE to lunch one (new) process for each page to stop losing work its not easy for the marketing people to lose new users that were so hard to get (sometimes with great personal/social/professional expense) just beause some plugin crash and took away all browser windows finally the "Assigned To:" should go back to something more userfull, as nick will not work on it

Note that there's a firefox extension to do this called Session Saver: https://addons.mozilla.org/extensions/moreinfo.php?id=436

If there's a working implementation it should be integrated in the default installation, this is not a feature that only a restricted group of people needs.

I also support the idea that this feature should be integrated into the core rather than left to extensions like SessionSaver (Firefox) and Total Recall (Mozilla Suite). See my comments at bug 159357 comment 64 regarding why having this in a separate extension like SessionSaver is not the right solution.

SessionSaver supports both Firefox and SeaMonkey / Mozilla Suite. More information about this extension see the mozillazine knowledgebase: http://kb.mozillazine.org/SessionSaver To download the latest version and to discuss bugs see the following thread: http://forums.mozillazine.org/viewtopic.php?t=47184 Prog.

Recent versions of Session Saver work pretty well in Firefox, but do NOT work properly in Mozilla Suite. I reported some of the bugs I've been seeing about a month ago: http://forums.mozillazine.org/viewtopic.php?t=47184&postdays=0&postorder=asc&postsperpage=15&start=1800#1965481 but was never acknowledged. As for Total Recall, it has not been updated in years and does not work with current versions of Mozilla. Therefore, there is no working workaround, and yes, this really needs to get implemented in core browser code. (Someone with the power to do so, please remove the misleading "Workaround in comment #119" from Status Whiteboard.)

I use Session Saver 2 in Firefox 1.5, it's great. I'm not just fanboying here, I am throwing weight behind the idea of supporting SS2 in Fx2, folding it into the default UI even. /be

You may not like it, but the fact is that I wrote the Tab Session Manager for MultiZilla long before others even had tabs, and this is how it looks these days: http://multizilla.mozdev.org/screenshots/features/session-manager/v2/tsm-main-window.png http://multizilla.mozdev.org/screenshots/features/session-manager/v2/tsm-add-window.png http://multizilla.mozdev.org/screenshots/features/session-manager/v2/tsm-merge-window.png http://multizilla.mozdev.org/screenshots/features/session-manager/v2/tsm-rename-window.png http://multizilla.mozdev.org/screenshots/features/session-manager/v2/tsm-window-properties.png http://multizilla.mozdev.org/screenshots/features/session-manager/v2/tsm-tab-properties.png http://multizilla.mozdev.org/screenshots/features/session-manager/v2/tsm-sessionTab-SaveBehavior.png http://multizilla.mozdev.org/screenshots/features/session-manager/v2/tsm-sessionTab-RestoreBehavior.png http://multizilla.mozdev.org/screenshots/features/session-manager/v2/tsm-sessionTab-Confirmations.png http://multizilla.mozdev.org/screenshots/features/session-manager/v2/tsm-close-confirmation.png http://multizilla.mozdev.org/screenshots/features/session-manager/v2/tsm-sessionTab-Miscellaneous.png Now a question: "Why should extension authors put efforts into extensions if they will taken over by Mozilla eventually?". Wouldn't that effectively eliminate the need of extensions? Tell me, how much money do you want to loose? /HJ

(In reply to comment #122) I even forgot one: http://multizilla.mozdev.org/screenshots/features/session-manager/v2/tsm-session-saver.png

> I wrote the Tab Session Manager for MultiZilla long before others even had > tabs.. Do u know the date or year? Please. :-) MultiZilla's session-manager is fine, but somewhen is little buggy for me, because I'm not a normal user .-) When I use SeaMonkey without MultiZilla I save sessions to Bookmarks (group of tabs)..little tricky, but very useful. The saving session is important and other developers (K-Meleon, Opera..) know this. (Thank HJ!) SessionSaver has nice feature..after crash it saves data from forms!

While starting on crash recovery, the ability to define and save multiple sessions is extremely useful and desirable. See also 63094 which seems to be incorporated in the expanded 'plans' for this bug fix/enhancement. I am out of the loop it seems and have just became aware of SessionSaver last week after a reference to it was posted on 63094. It is now a 'can't live without' feature extension for me. I have since become aware of another extension called Tab Mix Plus which I think also has related features, but am not so familiar with it. From my short experience, the feature set of SessionSaver would seem very desirable in guiding any implementation of the needed crash-recovery and session saving. Privacy can be an issue. Saving the detailed session includes elements of history, bookmarks. cookies, typed form information, etc.. and all of these have purge functions to erase tracks already implemented. The concept of saving session infos would seem to conflict with the desire for privacy. But need there be such a conflict? What about encrypting the saved session info, so that the files containing the data are not 'in plain language', readable in notepad. After a crash, perhaps a password would be required to re-open the last known session. Without the password, it opens to about:blank.

So many related, but this number seems to be the main tracker Omnibus SessionSaver solution: https://bugzilla.mozilla.org/show_bug.cgi?id=36810 Crash Recovery: https://bugzilla.mozilla.org/show_bug.cgi?id=19454 Manual save session info: https://bugzilla.mozilla.org/show_bug.cgi?id=63094 X11 session management: https://bugzilla.mozilla.org/show_bug.cgi?id=93789 Download manager - resume https://bugzilla.mozilla.org/show_bug.cgi?id=131871 HTTP form POST data autosave: https://bugzilla.mozilla.org/show_bug.cgi?id=158258 Add "Last tabs visited" to Navigator startup preferences: https://bugzilla.mozilla.org/show_bug.cgi?id=159357 Crash Protection: https://bugzilla.mozilla.org/show_bug.cgi?id=237647 http://wiki.mozilla.org/Session_Restore http://wiki.mozilla.org/Firefox2/StatusMeetings/2006-02-07#Session_Saver http://kb.mozillazine.org/SessionSaver =========================== Some 'Current' related Extensions: SessionSaver: https://addons.mozilla.org/extensions/moreinfo.php?application=mozilla&id=436 http://forums.mozillazine.org/viewtopic.php?t=47184 http://aasted.org/adblock/viewforum.php?f=4 Pike's Rewrite of SS http://www.pikey.me.uk/mozilla/?addon=ss Tab Mix Plus: https://addons.mozilla.org/extensions/moreinfo.php?id=1122&application=firefox Crash Recovery: https://addons.mozilla.org/extensions/moreinfo.php?id=1955&application=firefox MultiZilla: http://multizilla.mozdev.org/features/index.html Total Recall: http://recall.mozdev.org/

(In reply to comment #122) > You may not like it, but the fact is that I wrote the Tab Session > Manager for MultiZilla long before others even had tabs, ... Sounds great! Can you then help in getting this functionality into some software component supported by Mozilla.Org for general use in Mozilla-based applications? > Now a question: "Why should extension authors put efforts into > extensions if they will taken over by Mozilla eventually?". > Wouldn't that effectively eliminate the need of extensions? Extensions are a testing ground for ideas and also provide flexibility for special needs. If there are one or more extensions that provide a particular feature that would be a significant advantage to 90% or more of the users, then it is time to incorporate into the main product line the ideas that have been tested in the extension. I think in many cases the ultimate ambition of an extension author can be for the extension to become a supported component of the mainstream. If this is done well, it will produce a reusable component that can be used in Firefox, Seamonkey, Thunderbird, etc. The ability to restore application state after a shutdown (whether due to a crash or other reason) is of general use to nearly everyone. If it provides robustness in the face of other bugs (e.g., Firefox crashes but less than a minute later you're back browsing exactly as before), then this only makes it more important.

Hmm sounds like a dream :) So why Firefox has not e.g. Mouse Gesture? Firefox is like BeOS (and clons)..long way. That's all.

Note that this is being considered for Firefox currently (blocking&#8209;firefox2 has been granted). See bug 342701.

Oops -- I confused this bug with another. This is about crash recovery. Sorry.

isn't this now done in current Trunk (using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a6pre) Gecko/20070623 ID:2007062304) have a look at §History/Restore from last session" and the already for Firefox 2 introduced Session Management even in a case of crashing ?

Yes, isn't this feature in place now on the trunk? And there is also now the option to save all your open tabs on shut down as well.

Reopening. The feature is not in SeaMonkey yet. Note this bug is filed against the Suite not Firefox.

This one is much older and has more info. REOPENing.

This is my first patch, don't blame soo much. It almost working, so can be reviewed. Some notable glitches - extra about:blank on restored session (should be something related with browser startup preference), and no preference for session restore.

Comment on attachment 319287 [details] [diff] [review] ports sessionstore from firefox, saves and restores session >Index: suite/browser/navigator.js . . . > >@@ -1268,32 +1294,23 @@ function BrowserOpenTab() > default: > uriToLoad = "about:blank"; > break; > case 1: > uriToLoad = pref.getComplexValue("browser.startup.homepage", > Components.interfaces.nsIPrefLocalizedString).data; > break; > case 2: >- uriToLoad = gBrowser ? getWebNavigation().currentURI.spec >- : Components.classes["@mozilla.org/browser/global-history;2"] >- .getService(Components.interfaces.nsIBrowserHistory) >- .lastPageVisited; >+ uriToLoad = getWebNavigation().currentURI.spec; > break; > } > } catch(e) { > uriToLoad = "about:blank"; > } > >- // Open a new window if someone requests a new tab when no browser window is open >- if (!gBrowser) { >- openDialog(getBrowserURL(), "_blank", "chrome,all,dialog=no", uriToLoad); >- return; >- } >- > gBrowser.selectedTab = gBrowser.addTab(uriToLoad); > var navBar = document.getElementById("nav-bar"); > if (uriToLoad == "about:blank" && !navBar.hidden && window.locationbar.visible) Not a big deal right now, but you might want to do a new diff later on so you don't accidentally backout bug 429926 :-)

http://oreilly.wirelessdevnet.com/pub/a/mozilla/2000/09/29/keys.html This URL does not currently exist so removing.

Comment on attachment 319367 [details] [diff] [review] better patch, eliminates extra abot:blank , but fires error on console Hmmm, I do see you calling nsISessionStore here, but I don't see you porting the actual component from mozilla/browser/components/sessionstore/ Am I just missing something?

Sorry, i'm soo newbie in mozilla and cvs yet, correct diff is this one.

I'm currently maintaining the Session Manager extension which uses SessionStore in Firefox. I got a heads up on this from Philip Chee so I'm CCing myself. Assuming that the nsSessionStore component is ported over it will be simple to get Session Manager working with whichever browser you're adding it to (SeaMonkey?).

I'm using sessionstore enabled Seamonkey for a while without a problem and tried to install "Session Manager" using Philip Chee suggestions on the forum. It installs without errors, but i didn't get any UI, only preferences from addon manager are accessible.

(In reply to comment #145) > I'm using sessionstore enabled Seamonkey for a while without a problem and > tried to install "Session Manager" using Philip Chee suggestions on the forum. > It installs without errors, but i didn't get any UI, only preferences from > addon manager are accessible. > That's the standard location (provided by the addons manager) for an add-on's preferences; anything else (such as a menuitem under Tools) is extra (and provided by the extension). If the Preferences button isn't greyed-out when you select the add-ons in the add-ons manager, you can't say there isn't "any UI"; there just isn't any duplicate access to the UI. Or did you mean the only not-greyed-out buttons there were "Disable" and "Uninstall"?

Tony: <http://sessionmanager.mozdev.org/screenshots.html> Bascially SeaMonkey's "Tools" menu (i.e.menupopup) has a different id so two things need to be done: 1. Overlay navigator.xul instead of browser.xul; 2. change "menu_ToolsPopup" -> "taskPopup" in the session manager overlay;

(In reply to comment #147) > Bascially SeaMonkey's "Tools" menu (i.e.menupopup) has a different id so two > things need to be done: > > 1. Overlay navigator.xul instead of browser.xul; > 2. change "menu_ToolsPopup" -> "taskPopup" in the session manager overlay; > Session Manager used to support SeaMonkey (using the Crash Recovery extension) back in version 4. The latest version of Session Manager kept all the old SeaMonkey code (though it wasn't updated) so Session Manager should work in SeaMonkey without modification (assuming it can find the nsSessionStore component). I did find that the latest version of Session Manager there are typos in the locale's content.rdf files which prevented installation in SeaMonkey 1.1.9, but after I corrected that I could see all the Session Manager menus (though nothing worked obviously). To get Session Manager to display in SeaMonkey 2.0a I needed to add a line to the chrome.manifest. Of course without the nsSessionStore component, nothings works. If I can manage to get SeaMonkey compiled I can test out and see if things work. In the meantime if anyone wants to play around with what I have so far (it may or may not work), you can grab a version that installs at: http://downloads.mozdev.org/sessionmanager/session_manager-0.6.1.13-seamonkey.xpi

I am using SeaMonkey 1.1.8 and got an download error.

First you should upgrade to 1.1.9 (for security purposes). Second, try again. I just tried and it worked now, so i might not have finished replicating.

And third "Of course without the nsSessionStore component, nothings works." This is a WIP patch that hasn't been reviewed or checked in to the CVS. And even then it will not be backported to SeaMonkey 1.1.x (unless an extension author does it).

I've got it installed on my Seamonkey with patch applied and it get menu in Tools, but nothing works. When i click on on "Save Session..." for example, nothing happens. Preferences also displaying correctly. It seems something also should be tweaked.

OK, I figured it out. You need @mozilla.org/suite/sessionstore instead of @mozilla.org/browser/sessionstore in sessionmanager.js line 3. Now i have working SessionManager.

One correction, closed tabs list is not working now, because suite uses it's own undoclosetab implementation, and it not yet integrated with sessionstore. That's why i'm getting Error: [Exception... "'TypeError: No JSON representation for this object!' when calling method: [nsISessionStore::getClosedTabData]" nsresult: "0x8057001c (NS_ERROR_XPC_JS_THREW_JS_OBJECT)" location: "JS frame :: chrome://sessionmanager/content/sessionmanager.js :: anonymous :: line 761" data: no] Source File: chrome://sessionmanager/content/sessionmanager.js Line: 761 This part needs some rework to integrate undoclosetab in sessionstore.

I updated the Session Manager link above to work correctly based on comment #153. It will install in both Firefox and SeaMonkey, but will uninstall itself if it can't find the SessionStore component. As for the undoclosetab stuff, the patch looks like it includes the getClosedTabData method from SessionStore so it chould work, but it looks like the closed tab data is not being initiated correctly nor is it populated when tabs are closed. I guess the question is, will this be fixed to make it work with the suite's current implementation or is it going to be tied off?

> I guess the question is, will this be fixed to make it work > with the suite's current implementation or is it going to be tied off? It's up to Misak, but I guess initially we could do NS_ERROR_NOT_IMPLEMENTED or just return a null object until we figure out what to do.

in reply to comment #155 You have extra space in sessinmanager.js, which prevents installing on sessionstore enabled Seamonkey. After correction it install normally. Also I've contacted with seamonkey undoclosetab implementer, i think he can show the way to provide full sessionstore API functionality.

I'm trying to get working sessionstore API, but it seems i've missed something in this patch - all API functions that should return JSON encoded strings are throwing exception "TypeError: No JSON representation for this object!' when calling method: ". Same code works in Firefox. So if anybody know details of JSON implementation differences in Seamonkey and Firefox, or extension developer, which uses sessionstore, or just a guy with better knowledge than my very basic one in mozilla development - please help and provide as much detail as you can.

JSON is documented at http://www.json.org/ There is also a javascript specific example and parser on that site. That parser is actually included in the trunk and is required for sessionstore to work. The following two files are required. I don't know if they are built into SeaMonkey or not. http://lxr.mozilla.org/seamonkey/source/js/src/xpconnect/loader/JSON.jsm http://lxr.mozilla.org/seamonkey/source/js/src/xpconnect/loader/XPCOMUtils.jsm You can test JSON in SeaMonkey by running the tests located at: http://lxr.mozilla.org/seamonkey/source/dom/src/json/test/unit/

If SeaMonkey is not building or not packaging the JSON module, please file a separate bug on that, possibly blocking this one. We really want to have the JSON module included.

> http://lxr.mozilla.org/seamonkey/source/js/src/xpconnect/loader/JSON.jsm > http://lxr.mozilla.org/seamonkey/source/js/src/xpconnect/loader/XPCOMUtils.jsm Both of these are packaged in SeaMonkey 2.0a - at least on windows.

JSON, is included for sure, just because it throws exception. Otherwise i'll get other errors - not defined or something like that. The problem is that JSON don't like sessionstore component objects. Again, same code in Firefox works.

Comment on attachment 319402 [details] [diff] [review] version 0.6, includes all files except tests >Index: suite/browser/browser-prefs.js >+pref("browser.startup.page", 3); "browser.startup.page = 3" means always restore. I think we want to leave the default at 1 (which is already set earlier in this file). Also, because the pref UI isn't updated, it throws NS_ASSERTIONS and shows "blank page" as the page shown at browser startup. >Index: suite/browser/navigator.js >+const Cc = Components.classes; >+const Ci = Components.interfaces; >+const Cr = Components.results; >+const Cu = Components.utils; I doubt you'll get these past Neil. :) You never even use Cr or Cu. Just use the full Components.foo in the different places. >+ // this part of code should be settled when shell service will be ported, >+ // leaving it there for reference. got from firefox browser.js What does this mean for now? What session store functionality is broken? >Index: suite/browser/nsBrowserContentHandler.js > switch (prefs.getIntPref("browser.startup.page")) { >+ case 3: >+ // this fires error on console but works, don't know how to deal with it yet >+ return null; I'm not seeing any errors in the console, but this does cause a few problems. This is called 1) at startup, either with no flags or with the -browser flag 2) when someone invokes seamonkey when one instance is already running, with no flags, with the -browser flag or with -remote "xfedocommand(openbrowser)" For case #1, returning null results in a window with no tabs, and then session store restores the tabs For case #2, returning null still results in a window with no tabs, but there's no session store data, so you just get a blank tab. With sessionstore, it seems navigator.js needs to handle cases #1 and #2 differently >Index: suite/browser/tabbrowser.xml >+ // Session store >+ if (gPrefService.getBoolPref("browser.sessionstore.enabled")) { >+ this.mUndoCloseTabMenuItem.hidden = As discussed on IRC, undo closed tabs functionality shouldn't change (it shouldn't switch to using to using sessionstore), so this whole change to tabbrowser.xml should not be needed. This would also mean that the "browser.sessionstore.max_tabs_undo" pref would be unneeded. >Index: suite/common/src/nsSessionStartup.js >Index: suite/common/src/nsSessionStore.js >+ onTabClose: function sss_onTabClose(aWindow, aTab) { >+ // store closed-tab data for undo >+ if (tabState.entries.length > 1 || tabState.entries[0].url != "about:blank") { >+ this._windows[aWindow.__SSi]._closedTabs.unshift({ >+ state: tabState, >+ title: aTab.getAttribute("label"), >+ image: aTab.getAttribute("image"), >+ pos: aTab._tPos This is the reason for the JSON failure. JSON does not handle undefined and aTab._tPos is undefined. Our tabs do not have a _tPos property. When it tries to serialize closeTabs, it fails. Practically speaking, since you don't need to be keeping the closedTabs around, this problem will take care of itself. If you want to still fully implement the sessionstore API, you'll need to get a position, but we don't remember the closed tab's position (we always restore it at the end). So you could just pretend that all the closed tabs were the last one. >Index: suite/common/src/nsSuiteGlue.js >+ _onQuitRequest: function(aCancelQuit, aQuitType) This method never gets called, so I never get the dialog to ask if I want to save, but I still get the close tabs warning dialog. Sessionstore still works because the pref is set to 3.

Ok, this patch make API really work. It has some problems, but it almost working. The undoclosetab API function call suite own restoretab method(it's better that ff's one). To sessionmanager developer: undoclosetab function didn't work. It fires "Error: undoCloseTab is not defined Source File: chrome://sessionmanager/content/sessionmanager.js Line: 139". I don't why it doing this, will try to figure out later. But sessinmanager now correctly displays closed tabs in menu. All thing seems to work. The patch is safe enough to use it on test environment, so all interrested people can apply it and test their extensions and main functionality and report problems if they have time.

I'm going to need to make changes to Session Manager to get undoCloseTab to work correctly. Currently Session Manager calls the undoCloseTab function in browser.js (not the one in the SessionStore component) because it provides some existing functionality. Since SeaMonkey doesn't have an undoCloseTab function in browser.js, an error occurs. I'm going to need to make a number of changes on my end to handle other discrepancies from using the built in restoretab method (namely different preferences, variables, etc) so I'll fix this on my end. There is still a few issues though. If you want this to be "correct", you'll need to modify any function that references this._windows[aWindow.__SSi]._closedTabs or aWindow.__SS_dyingCache._closedTabs to call browser.getUndoList() instead since the _closedTab list isn't maintained in SeaMonkey. In addition, you should probably modify the onTabClose function to never actually store the close tabs since that's just a waste of memory as the _closedTabs array isn't used. Currently my extension call getCloseTabCount() which will currently never be correct in SeaMonkey. I can implement my own call to browser.getUndoList() to count the closed tabs in SeaMonkey, but it would be nice if the sessionstore component in SeaMonkey could do it since I'm trying to keep the required browser specific code to a minimum.

> In addition, you should probably modify the onTabClose function to never Is there any reason to have onTabClose at all? > Currently my extension call getCloseTabCount() which will currently never be > correct in SeaMonkey. wouldn't it be correct to just return getUndoList().length ?

(In reply to comment #165) > There is still a few issues though. If you want this to be "correct", you'll > need to modify any function that references > this._windows[aWindow.__SSi]._closedTabs or aWindow.__SS_dyingCache._closedTabs > to call browser.getUndoList() instead since the _closedTab list isn't > maintained in SeaMonkey. > > In addition, you should probably modify the onTabClose function to never > actually store the close tabs since that's just a waste of memory as the > _closedTabs array isn't used. As i understand the code, sessionstore maintains _closedTab list. As discussed in IRC with Andrew Schultz, CTho and KaiRo suite will maintain both _closedTab and savedBrowser lists in parallel. The memory footprint shouldn't be noticeable. Of course, it's not an elegant solution, in future I'll modify code to get rid of _closedTabs, when will have enough knowledge in mozilla code. Anyway i think you should use "legal" API calls and expect thet they should work as expected. > Currently my extension call getCloseTabCount() which will currently never be > correct in SeaMonkey. Why ?

(In reply to comment #167) > > Currently my extension call getCloseTabCount() which will currently never be > > correct in SeaMonkey. > > Why ? > Because while the code to add closed tabs to the _closedTabs array exists, the code to remove them is no longer there. There's two problems: #1. You removed the line from undoCloseTab which removes the restored tab from the _closedTab array so currently that array won't match the savedBrowser list after a tab is restored. The line that did that was: var closedTab = closedTabs.splice(aIndex, 1).shift(); You can change this to simply: closedTabs.splice(aIndex, 1); #2. The undoCloseTab function in sessionstore is not called by SeaMonkey when restoring a tab so even if you fix problem #1 above, it still wouldn't work. In Firefox whenever a tab is restored, the undoCloseTab function in browser.js executes, which then calls the SessionStore.undoCloseTab function. That's how the restored tabs get removed from the _closedTabs array (and how Firefox restores the tab). You could call the Sessionstore undoCloseTab function from browser.removeTab(), but the way undoCloseTab is currently written, that would create an infinite loop.

I was thinking that onTabOpen and onTabClose event listeners should catch and update _closedTabs on these events. Do they ?

onTabClose does update _closedTabs when a tab closes. The onTabOpen event triggers any time a tab is open (not just when one is restored). It currently calls the onTabAdd procedure which doesn't open _closedTabs. The only time _closedTabs every had a tab removed was on a call to the undoCloseTabs. This made sense since that was the only way to restore a closed tab.

OK, i understand. This patch adds extra updateClosedTabList() call to API, which is not recommended to use by developers - it should be called only by restoreTab. Now getCloseTabCount() should work as expected.

> As discussed in IRC with Andrew Schultz, CTho and KaiRo suite will maintain both > _closedTab and savedBrowser lists in parallel. Not me... saving parallel lists can only cause pain. The relevant data you need should be in savedBrowsers.

Ok, this patch uses savedBrowsers, API should work, patch should work in general. Michael, can you test it ? My sessionmanager seems to work with it fairly well. This patch has very bad and dirty code, but i'll be very grateful if ajschult review it, just to know that it's going to right direction. Notable glitches: Closed tabs list does not go to right place when reading session state from file. There should be more things to clean.

Comment on attachment 321968 [details] [diff] [review] removed updateClosedTabList(), removed _closedTabs, using savedBrowsers as suggested With the patch applied, the behavior I see if I crash is that sessionstore kicks in -- the window is restored with the tabs I had, but it also opens the home page in an additional tab (which we don't really want). If I have more than one window open, an additional window is restored, but the first tab of that window is replaced with my home page. >Index: suite/browser/nsBrowserContentHandler.js > switch (prefs.getIntPref("browser.startup.page")) { >+ >+ case 3: >+ return null; Hmm. Is this what we want? I suspect that this results in behavior that we want for case 3, but it doesn't seem like the right fix. Doesn't firefox return the same for 1 and 3? I suspect that restore-after-crash might behave better if we had a better fix for this. >Index: suite/browser/tabbrowser.xml > if (aIndex >= this.savedBrowsers.length || aIndex < 0) > return; >- >+ oops. This line was happy as it was. >Index: suite/common/src/nsSessionStartup.js >+# ***** BEGIN LICENSE BLOCK ***** You'll want to format the license block like we do for other .js files in suite/. (no "#") >+ this._lastSessionCrashed = >+ initialState.session && initialState.session.state && >+ initialState.session.state == STATE_RUNNING_STR; _lastSessionCrashed doesn't need to be a property of |this|. It's only used in this function. >+ // invalid .INI file - nothing can be restored >+ } >+ catch (ex) { debug("The session file is invalid: " + ex); } The comment above is totally in the wrong place... it should be in the catch block. This should also set _iniString to null to prevent the rest of the code from having any effect (and so that the state getter below returns null). Also, it looks like the evalInSandbox is the only part that would throw an exception. I don't see why the rest of it needs to be in a try block (you'd have to declare initialState before the try block so you could use it after the try block). >+ // prompt and check prefs >+ if (this._iniString) { This block can go up somewhere in the above if blocks, since you already know that this._iniString is not null up there. >+ if (this._prefBranch.getBoolPref("sessionstore.resume_session_once")) { >+ this._prefBranch.setBoolPref("sessionstore.resume_session_once", false); >+ } This is the only thing that still needs to happen if you hit the catch block above for an invalid file. If you move this part up to the beginning, you could return from the catch block, which would simplify the code flow a lot. That'd break _doResumeSession, so you'd want to perhaps have a doResumeSession variable that's equal to what _doResumeSession returns. That'd save you from calling into the pref service repeatedly as well. Moving this code to the top would also unset the pref when sessionstore is disabled, which also seems like a good idea. >+ * Since we're garanteed to be at least the second caller of defaultArgs >+ * (nsBrowserContentHandler calls it to determine which arguments to pass Our nsBrowserContentHandler doesn't have defaultArgs. I suspect this is why restore-after-crash isn't happy and why case 3 above needed to return null to get the desired behavior. >+ var cvstream = Cc["@mozilla.org/intl/converter-input-stream;1"]. cvstream = "converter stream"? I guess name this cvStream. >+ catch (ex) { } // inexisting file? "nonexistent". Practically speaking, this method should only be called if the file exists, so I'd guess that a more likely cause would be a permission problem or perhaps a sufficiently corrupted file.

this patch will work as it should. The sessionstore.js file will be the most hard part, i think i should have many errors here. Will be happy if someone else have a look to my code.

Comment on attachment 322252 [details] [diff] [review] addresses all ajschult comments, now restores multiple windows correctly Sorry, this patch also didn't do its job well, new one is coming.

Here are the new patch. I've added new read only attribute to API, it should be useful. The boolean restoreInProgress is true if sessionstore session restoration in progress. I'm planning to backport it to ff later, if reviewers agree that it's a good addition.

Comment on attachment 322626 [details] [diff] [review] Correctly restores multiple windows. Behavior I see with this patch and browser.startup.page=3 is that the last tab in the window is replaced with my homepage. If I crash, it restores all the tabs, but also loads my homepage in an extra tab. >Index: suite/browser/navigator.js >+ // initialize the session-restore service (in case it's not already running) >+ if (document.documentElement.getAttribute("windowtype") == "navigator:browser") { Startup only gets called for windows of type navigator:browser, so you don't need this. >+ try { >+ var ss = Components.classes["@mozilla.org/suite/sessionstore;1"]. >+ getService(Components.interfaces.nsISessionStore); >+ ss.init(window); >+ var isRestoring = ss.restoreInProgress; you need to declare (and initialize) isRestoring before the try block. Also, restoreInProgress is false for me even when session store is going to restore the window. This is probably causing the problems I was seeing. >+ dump("restoring "+ isRestoring+ "\n"); dump here is OK for debugging, but remember to remove for the final patch. >Index: suite/common/src/nsSessionStartup.js >+ doResumeSession = this._doResumeSession(); Right. And you're only using this here, so you can just inline the function. >+ // invalid .INI file - nothing can be restored >+ catch (ex) { >+ this._iniString = null; Right. Move the comment into the catch block (below the catch line) >+ // prompt and check prefs >+ if (lastSessionCrashed && this._doRecoverSession()) nit: indentation of the comment >+ // Note: Seamonkey doesn't have defaultArgs. do we need this ? actually, no. Our deafult arguments are nothing. See http://mxr.mozilla.org/seamonkey/source/suite/common/tasksOverlay.js#151 The code in navigator.js (with some tweaking) should be able to handle this properly.

This one should correctly restore windows in any case, but IMHO there will be prettier way to do this. For unknown reason the first window handles differently, so I'm forced to add check for thins in nsBrowserContentHandler.js. Also restoreInProgress code is ugly yet and have lag (it has delay when it's still true, but all windows are restored). I need suggestions there.

Comment on attachment 322770 [details] [diff] [review] Addressed all comments above. >+// if (isRestoring) { >+// window.arguments = []; >+// } you can just remove that >- } else { >+ } else if (!isRestoring) { this works, but only because navigator.js throws an exception later: JavaScript error: chrome://navigator/content/navigator.js, line 674: uriArray is undefined I think you want to turn a rather large chunk of code into an !isRestoring if block: > if (!isPageCycling) { > var uriToLoad = ""; > > if (!isRestoring) { down to the chunk of code where it's calling loadURI. >Index: suite/browser/nsBrowserContentHandler.js >+ case 3: >+ if (!Components.classes["@mozilla.org/appshell/window-mediator;1"] >+ .getService(Components.interfaces.nsIWindowMediator) >+ .getMostRecentWindow("navigator:browser")) { >+ return null; >+ } >+ return getHomePageGroup(prefs); hmm, shouldn't need to do this... just add case 3 up above: switch case1: case3: return getHomePage... >Index: suite/browser/tabbrowser.xml >- this.savedBrowsers.unshift({browser: oldBrowser, history: oldSH}); >+ // Sessionstore already unshifted savedBrowsers and inserted tabData >+ // doing the rest eek. >Index: suite/common/src/nsSessionStartup.js >+ if ((resumeFromCrash || this._doResumeSession) && this._sessionFile.exists()) { this._doResumeSession is a function and so it's always true. I think you want var doResumeSession = this._prefBranch.getIntPref("startup.page") == 3 || this._prefBranch.getBoolPref("sessionstore.resume_session_once"); and then remove the function entirely. >+ /** >+ * Removes the default arguments from the first browser window >+ * (and removes the "domwindowopened" observer afterwards). >+ */ >+ _onWindowOpened: function sss_onWindowOpened(aWindow) { this doesn't actually do anything other than remove itself as an observer (it used to do what's described in the comment). So you can remove this function and the bit above that's calling into here (and then remove the thing that's causing that to be called, etc).

>>Index: suite/browser/tabbrowser.xml >>- this.savedBrowsers.unshift({browser: oldBrowser, history: oldSH}); >>+ // Sessionstore already unshifted savedBrowsers and inserted tabData >>+ // doing the rest > >eek. > Index: suite/common/src/nsSessionStore.js > + onTabClose: function sss_onTabClose(aWindow, aTab) { [...] > + aWindow.getBrowser().savedBrowsers.unshift({browser: {}, history: {}, tabData: tabsData}); How about leaving tabsData on the |aTab| and then letting the removeTab method in tabbrowser.xml pick up the tabsData e.g. Index: suite/common/src/nsSessionStore.js onTabClose: function sss_onTabClose(aWindow, aTab) { [...] aTab.tabsData = tabsData; [...] Index: suite/browser/tabbrowser.xml this.savedBrowsers.unshift({browser: oldBrowser, history: oldSH, tabData: aTab.tabsData}); > + var postdata; > + if (aEntry.postdata_b64) { // Firefox 3 > + postdata = atob(aEntry.postdata_b64); > + } else if (aEntry.postdata) { // Firefox 2 > + postdata = aEntry.postdata; > + } Um, we aren't going to backport this to branch are we?

(In reply to comment #181) [...] > > + var postdata; > > + if (aEntry.postdata_b64) { // Firefox 3 > > + postdata = atob(aEntry.postdata_b64); > > + } else if (aEntry.postdata) { // Firefox 2 > > + postdata = aEntry.postdata; > > + } > > Um, we aren't going to backport this to branch are we? > and in addition, shouldn't these comments rather mention SeaMonkey 2 and SeaMonkey 1 respectively? This is a Suite bug, and Firefox 2 and higher already has a built-in session saver.

Actually, SeaMonkey 2 now using Firefox 2 postdata type. It's safer not to touch these until SeaMonkey developers decide what to pick.

I made all changes suggested, but now have extra blank tab on first window. Should be something related with content handler, but I can't catch it. Anyway asking review, just because there should be more glitches than this one.

This is the same patch, I've just replaced Cc Cr Ci and cleaned code a bit. Still extra tab on first window, i'm totally lost in navigator and content handler code. sessiosstore.js needs review, sessionstartup.js should be close to final version.

Ok, i tweaked navigator.js code a bit, now it correctly restores windows without notable errors. Ajschult should like this, i hope.

>Index: suite/browser/tabbrowser.xml >- this.savedBrowsers.unshift({browser: oldBrowser, history: oldSH}); >+ this.savedBrowsers.unshift({browser: oldBrowser, history: oldSH, tabData: aTab.tabsData}); Hmm. I just realized that if the sessionstore component doesn't get initialized then |aTab.tabsData| will be undefined. I notice that the initialization code in navigator.js is wrapped in a try/catch block so I think that this may be a possibility.

Something like this ? try { this.savedBrowsers.unshift({browser: oldBrowser, history: oldSH, tabData: aTab.tabsData}); } catch { this.savedBrowsers.unshift({browser: oldBrowser, history: oldSH}); } or just make tabData null? Or replace try/catch block with if ? BTW i disabled sessionstore and ran seamonkey and undoclosetab capability of tabbrowser.xml is OK. Extensions may have problems if sessionstore didn't get initialized, but they always should check that case. Sorry for questions like this one, i'm just on first step of learning.

This patch incorporates workaround for comment 187, suggested by Philip Chee and ceorrect handling of restoreinprogress(should be no delay).

Comment on attachment 325952 [details] [diff] [review] correct handling of restoreInProgress, also workaroung for Philip Chee's case sorry for the slow review. I'm still seeing the homepage loaded after a crash if I select restore. So I get the tabs restored plus the homepage. It seems to work OK with browser.startup.page=3. >Index: suite/common/src/nsSessionStore.js >+ else { >+ // Nothing to restore, notify observers things are complete. >+ awindow.setTimeout(function() { oops. "aWindow" Also needs extra indenting. I'll try to get to a more complete review this weekend.

I tweaked logic a bit, now it works in any case of browser.startup.page.

Comment on attachment 326091 [details] [diff] [review] Fixes case above, better logic in content handler. This works for me now recovering from crashes (yay). I still need to look through nsSessionStore, but the rest is looking good. Just a few fixups: >Index: suite/browser/browser-prefs.js > // only load url passed in when we're not page cycling > if (!isPageCycling) { > var uriToLoad = ""; >- > // Check window.arguments[0]. If not null then use it for uriArray oops. Just leave these extra lines. >+ } else uriArray = ["about:blank"]; style nit: put the uriArray assignment on its own line. >Index: suite/browser/nsBrowserContentHandler.js >+ var ss = Components.classes["@mozilla.org/suite/sessionstartup;1"] >+ .getService(Components.interfaces.nsISessionStartup); >+ haveUpdateSession = ss.doRestore(); you can just do: if (ss.doRestore()) return null; since you only use haveUpdateSession to return below. >Index: suite/browser/tabbrowser.xml >- this.savedBrowsers.unshift({browser: oldBrowser, history: oldSH}); >+ this.savedBrowsers.unshift({browser: oldBrowser, history: oldSH, tabData: aTab.tabsData ? aTab.tabsData : {} }); ya, I like this better (thanks, Philip) >Index: suite/common/src/nsSessionStartup.js >+ this._doResumeSession = this._prefBranch.getIntPref("startup.page") == 3 || >+ this._prefBranch.getBoolPref("sessionstore.resume_session_once"); right! But now you can make this into a local variable, since it's only used in this function (and remove it as a member listed above).

This is definitely wanted for SeaMonkey 2, but we won't block a release for a feature.

I've updated patch with ajschult fixups, commented out savedBrowsers restoration code, it will go to separate bug, as disscussed on IRC.

I've included fix of bug 343352 in a patch.

You might also consider porting bug 357419 which just got fixed.

It's already ported, I'm just waiting reviewer comments for previous patch to upload new one.

Comment on attachment 332522 [details] [diff] [review] same as above, plus port of bug 343352 to suite I had comments before... not sure where they went. Hopefully, I remembered all of them. >+++ b/suite/common/src/nsSessionStore.js Wed Aug 06 17:46:51 2008 +0500 >+ // During the initial restore tracks the number of windows yet to be restored Grammar isn't right here. Perhaps During the initial restore, the number of windows yet to be restored >+ // closed tabs list >+ _tabData: {}, Can you just grab this info from tabbrowser as needed (in _getCurrentState)? You do this in _collectWindowData. With that, you could drop _tabData entirely. >+ // Restore state in progress >+ _restoreInProgress : false, >+ >+ _setRestoreInprogress : false, What does this variable represent? >+ if (iniString) { >+ try { >+ // parse the session state into JS objects >+ this._initialState = this._safeEval(iniString); as in nsSessionStartup, this is the only part of the try block that would throw, except for the delete at the end (which shouldn't be try/caught). >+ delete this._initialState.windows[0].hidden; this can be if (this._initialState.windwos[0]) delete this._initialState.windows[0].hidden; >+ // if last session crashed, backup the session >+ if (lastSessionCrashed) { this needs to be in the same scope as lastSessionCrashed is declared.

Ok, _tabData is really unnecessary, i've deleted it. This patch also ports fixes for Bug 450595, Bug 409115, Bug 367052, Bug 448741, Bug 450879. No tests yet, i'll include all tests, when get review+ for patch. > + _setRestoreInprogress : false, >What does this variable represent? This variable is flag to actually set RestoreInprogress to false. This needed to workaround SeaMonkey implementation of navigator.js, which uses extra variables when opening new window.

Added ports of bug 449596 and bug 391397.

ported bugs - Bug 368677, Bug 346337, Bug 452975, Bug 439675, Bug 366572

ported bugs - bug 342635, bug 367052, bug 451702, bug 381349

Oops, I forgot to replace CC,CI stuff.

Many bugs fixed since last patch, so it's time to refresh this patch. Ported - Bug 355284 , Bug 339445, Bug 456465, Bug 408470, Bug 458959, Bug 458954, Bug 457195, Bug 458963, Bug 456342, Bug 459041

This is another bugfix respin of patch to keep in sync with ff. Ported Bug 345898, Bug 459906, Bug 460334, Bug 427186, Bug 463206, Bug 46420. Additionally copied some bits of code from private browsing patch, which reorganizes sessionstore's functions.

Comment on attachment 349629 [details] [diff] [review] another bugfix respin >diff --git a/suite/common/src/nsSessionStartup.js b/suite/common/src/nsSessionStartup.js >+ try { >+ // parse the session state into JS objects >+ var s = new Components.utils.Sandbox("about:blank"); >+ var initialState = Components.utils.evalInSandbox(this._iniString, s); >+ } initialState needs to be declared before the "try {" >+ else if (!lastSessionCrashed && doResumeSession) >+ this._sessionType = Components.interfaces.nsISessionStartup.RESUME_SESSION; this shouldn't happen if we failed to get initialState, but it will. The most straightforward thing to do would be to set doResumeSession to false within the catch block above. >diff --git a/suite/common/src/nsSessionStore.js b/suite/common/src/nsSessionStore.js >+ try { >+ // parse the session state into JS objects >+ this._initialState = this._safeEval(iniString); >+ } >+ catch (ex) { debug("The session file is invalid: " + ex); } >+ >+ let lastSessionCrashed = >+ this._initialState.session && this._initialState.session.state && >+ this._initialState.session.state == STATE_RUNNING_STR; you need to test this._initalState here (like you do in nsSessionStartup), since it will be undefined if _safeEval threw. >+ // if last session crashed, backup the session >+ if (lastSessionCrashed) { >+ } hmm... something not right here. Should this just be removed, or is there something that should be in that block? >+ // attempt to update the current URL we send in a crash report >+ this._updateCrashReportURL(aWindow); I'm having trouble imagining why crash report functionality should live in here. crash reports have almost nothing to do with session restoring. Shouldn't this live in one of our core (suite) files? Other than that, things look OK up to onTabLoad

Comment on attachment 349629 [details] [diff] [review] another bugfix respin >+ locale/@AB_CD@/communicator/sessionstore.properties (%chrome/common/sessionstore.properties) this file seems to be missing from your patch

(In reply to comment #207) > > initialState needs to be declared before the "try {" > Fixed. > > this shouldn't happen if we failed to get initialState, but it will. The most > straightforward thing to do would be to set doResumeSession to false within the > catch block above. Fixed. > > you need to test this._initalState here (like you do in nsSessionStartup), > since it will be undefined if _safeEval threw. > Fixed. > >+ // if last session crashed, backup the session > >+ if (lastSessionCrashed) { > >+ } > > hmm... something not right here. Should this just be removed, or is there > something that should be in that block? > Actually, the code in this block is in bug 459550. There is a little mess with all the patches porting for sessionstore, and aboutsessionrestore. I removed this if, will add to bug 459550 patch, also all work porting patches will go to that bug, keeping this in stable state. > >+ // attempt to update the current URL we send in a crash report > >+ this._updateCrashReportURL(aWindow); > > I'm having trouble imagining why crash report functionality should live in > here. crash reports have almost nothing to do with session restoring. > Shouldn't this live in one of our core (suite) files? > This is actually bug 375083, and nobody gives a reason there why this code should go to sessionstore. Give me suggestion where is the best to put this code, I'll do that. > this file seems to be missing from your patch Added.