486696 - Closing browser / switching profile keeps and later restores state / open tabs and history

Status: RESOLVED FIXED

(SeaMonkey :: Session Restore, defect)

Description

[Boris 'pi' Piwinger]

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]

Can you try on clean profile ? Anybody confirmed this bug ? Detail steps to reproduce ?

Comment 2

[Boris 'pi' Piwinger]

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]

This happens, on my Fedora 10 too and didn't exist on Alpha 2. Sessionstore ?

Comment 4

[Boris 'pi' Piwinger]

Since I end the session regularly it would mean that this state is not saved correctly.

pi

Comment 5

[Boris 'pi' Piwinger]

With 2.0b it works again. I just got used to it;-)

Comment 6

[Boris 'pi' Piwinger]

I don't get it. It's back.

Comment 7

[Robert Kaiser]

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]

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]

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]

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]

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]

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]

Attachment: 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

[[e-mail]]

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]

Attachment: 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

[[e-mail]]

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]

Attachment: for checkin [Checkin: Comment 18 & 24]

For checkin, Nits fixed, carrying forward r+ and sr+ from Neil.

Comment 18

[Serge Gautherie (:sgautherie)]

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)]

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]

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)]

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]

(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]

Please check in to 1.9.1 branch too.

Comment 24

[Serge Gautherie (:sgautherie)]

Comment on attachment 425767 [details] [diff] [review]
for checkin
[Checkin: Comment 18 & 24]


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