Last Comment Bug 486696 - Closing browser / switching profile keeps and later restores state / open tabs and history
: Closing browser / switching profile keeps and later restores state / open tab...
Status: RESOLVED FIXED
: fixed-seamonkey2.0.4, regression
Product: SeaMonkey
Classification: Client Software
Component: Session Restore (show other bugs)
: Trunk
: All All
: -- major (vote)
: seamonkey2.1a1
Assigned To: Misak Khachatryan
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-04-03 09:13 PDT by Boris 'pi' Piwinger
Modified: 2010-02-11 16:26 PST (History)
3 users (show)
kairo: blocking‑seamonkey2.0.1-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
check if we should restore when restarting (557 bytes, patch)
2010-02-07 04:30 PST, Misak Khachatryan
neil: superreview-
Details | Diff | Splinter Review
check environment to detect profile switching (1.17 KB, patch)
2010-02-07 22:46 PST, Misak Khachatryan
neil: review+
neil: superreview+
Details | Diff | Splinter Review
for checkin [Checkin: Comment 18 & 24] (1.16 KB, patch)
2010-02-08 02:20 PST, Misak Khachatryan
misak.bugzilla: review+
misak.bugzilla: superreview+
kairo: approval‑seamonkey2.0.4+
Details | Diff | Splinter Review

Description Boris 'pi' Piwinger 2009-04-03 09:13:21 PDT
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20090223 SeaMonkey/2.0a3

I assume it is a dupe, but cannot find it. When I close the browser (either quitting SeaMonkey or switching the profile) and later restart that profile, SM starts with the last page, even though I have set it to start with my home page.

This is particularly critical if the page loaded calls a function.

pi
Comment 1 Misak Khachatryan 2009-04-03 12:25:34 PDT
Can you try on clean profile ? Anybody confirmed this bug ? Detail steps to reproduce ?
Comment 2 Boris 'pi' Piwinger 2009-04-04 05:11:26 PDT
Yes, it happens with a new profile, too. Create new profile, define a home page, set to start with home page. Navigate to whatever other page, switch profile, switch back, observe this is not your home page.

pi
Comment 3 Misak Khachatryan 2009-04-04 06:52:39 PDT
This happens, on my Fedora 10 too and didn't exist on Alpha 2. Sessionstore ?
Comment 4 Boris 'pi' Piwinger 2009-04-04 07:01:43 PDT
Since I end the session regularly it would mean that this state is not saved correctly.

pi
Comment 5 Boris 'pi' Piwinger 2009-08-01 12:17:12 PDT
With 2.0b it works again. I just got used to it;-)
Comment 6 Boris 'pi' Piwinger 2009-08-01 13:55:10 PDT
I don't get it. It's back.
Comment 7 Robert Kaiser 2009-09-17 17:52:12 PDT
I don't see any reason to give this special priority, esp. as it's not clear if this happens for more than one user and what any real impact is, but if it's found to be a bug, fixing it is also not unwanted, so cancelling the wanted flag.
Comment 8 Boris 'pi' Piwinger 2009-10-22 09:16:53 PDT
The problem seriously worsened with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1.4) Gecko/20091017 SeaMonkey/2.0. Now it is no longer the latest stage but with a previous. So some older pages come up. This is certainly irritating and a privacy issue.
Comment 9 Robert Kaiser 2009-12-09 07:35:19 PST
Not blocking security updates on this.

Misak, any idea what's up here and how we can solve this in a way that everyone is happy?

Additionally, I'm still not completely sure what the nature of this bug is. Can we have complete steps to reproduce along with expected and actual behavior?
Comment 10 Boris 'pi' Piwinger 2009-12-09 07:46:21 PST
Not quite sure what to say. Let me repeat. I have two profiles (I only use the browser). I switch between the two.

Expectation (old behavior): Get clear browser, i.e. one tab start page as defined

Actual result: Some old state, looks like the state a few seconds before swiching, in most cases not the actual state when switching profiles. This leads in particular to reloading of dynamic pages which may have actions associated. Very bad.

pi
Comment 11 Robert Kaiser 2009-12-09 07:54:31 PST
I'd expect the last state of that profile to be restored, actually, just like in a reload or possibly crash scenario.

You say the actual difference is just that it's not that state but a slightly earlier one, i.e. that the real bug is that we don't save the current state when switching?
Comment 12 Boris 'pi' Piwinger 2009-12-09 08:14:32 PST
When the effect first came up (when I originally reported the bug) it was the last state.

Later the behavior changed and it is now often not the last state.
Comment 13 Misak Khachatryan 2010-02-07 04:30:45 PST
Created attachment 425697 [details] [diff] [review]
check if we should restore when restarting

I've found time to look at this bug, it seems that we should check if we should restore when restarting. One liner fix.
Comment 14 neil@parkwaycc.co.uk 2010-02-07 13:21:28 PST
Comment on attachment 425697 [details] [diff] [review]
check if we should restore when restarting

The problem is that the switch profile dialog simulates a restart to switch profile, but the session store code assumes that we should always save the session before a restart. Unfortunately not doing this will break existing code that assumes that a restart will save the session.

What you could do is to check for one of the environment variables that the switch profile dialog sets during the restart. Normally these variables get cleared on startup so if they are set it must be because of a profile switch.
Comment 15 Misak Khachatryan 2010-02-07 22:46:51 PST
Created attachment 425756 [details] [diff] [review]
check environment to detect profile switching

I thought that _doResumeSession will be enough ...
Here is the new patch. It checks existance of only one environment variable, hope this will be enough.
Comment 16 neil@parkwaycc.co.uk 2010-02-08 02:05:35 PST
Comment on attachment 425756 [details] [diff] [review]
check environment to detect profile switching

>+  _isSwitchingProfile: function sss__isSwitchingProfile() {
Nit: Don't double your underlines. sss_isSwitchingProfile is enough.

>+      var env = Components.classes["@mozilla.org/process/environment;1"]
>+                          .getService(Components.interfaces.nsIEnvironment);
>+      return env.exists("XRE_PROFILE_NAME");
Nit: you've used a 4-space indent here for some reason, should be two spaces.

(In reply to comment #15)
> I thought that _doResumeSession will be enough ...
No, that actually reads sessionstore.resume_session_once ;-)
Comment 17 Misak Khachatryan 2010-02-08 02:20:42 PST
Created attachment 425767 [details] [diff] [review]
for checkin
[Checkin: Comment 18 & 24]

For checkin, Nits fixed, carrying forward r+ and sr+ from Neil.
Comment 18 Serge Gautherie (:sgautherie) 2010-02-08 08:03:51 PST
Comment on attachment 425767 [details] [diff] [review]
for checkin
[Checkin: Comment 18 & 24]


http://hg.mozilla.org/comm-central/rev/71964d95103f
Comment 19 Jens Hatlak (:InvisibleSmiley) 2010-02-08 08:21:41 PST
Hmm, is wanted-seamonkey2.0 still being monitored? I thought seeking approval‑seamonkey2.0.4 would be the way to go. :-/

@Misak: Good work! I think it would be nice to have it on branch as well. Can you write a short risk assessment comment to help KaiRo/IanN/... judge the possible impactand request branch approval for the patch? Thanks.
Comment 20 Misak Khachatryan 2010-02-08 08:47:34 PST
Comment on attachment 425767 [details] [diff] [review]
for checkin
[Checkin: Comment 18 & 24]

Unfortunately i don't have 1.9.1 branch to prepare cleanly applying patch, but if nothing changed in nsIEnvironment implementation between 1.9.1 and 1.9.2 it's completely safe IMHO.
Comment 21 Jens Hatlak (:InvisibleSmiley) 2010-02-08 09:13:54 PST
Comment on attachment 425767 [details] [diff] [review]
for checkin
[Checkin: Comment 18 & 24]

Patch already applies cleanly to c-191 as-is. :-)
Comment 22 Robert Kaiser 2010-02-08 09:30:55 PST
(In reply to comment #19)
> Hmm, is wanted-seamonkey2.0 still being monitored? I thought seeking
> approval‑seamonkey2.0.4 would be the way to go. :-/

It is. I'll probably get wanted-seamonkey2.0 retired soon, we don't really need it any more.
Comment 23 Misak Khachatryan 2010-02-09 22:03:21 PST
Please check in to 1.9.1 branch too.
Comment 24 Serge Gautherie (:sgautherie) 2010-02-11 16:25:34 PST
Comment on attachment 425767 [details] [diff] [review]
for checkin
[Checkin: Comment 18 & 24]


http://hg.mozilla.org/releases/comm-1.9.1/rev/8d34a41d8a74

Note You need to log in before you can comment on or make changes to this bug.