Last Comment Bug 874042 - Cookies in a private session are purged when opening a new private window
: Cookies in a private session are purged when opening a new private window
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: UI Design (show other bugs)
: Trunk
: All All
: -- major (vote)
: seamonkey2.21
Assigned To: neil@parkwaycc.co.uk
:
Mentors:
http://www.html-kit.com/tools/cookiet...
Depends on:
Blocks: 460895 872521 872000 873032
  Show dependency treegraph
 
Reported: 2013-05-20 06:44 PDT by rsx11m
Modified: 2013-06-23 04:37 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed
fixed


Attachments
Proposed patch (994 bytes, patch)
2013-06-11 03:30 PDT, neil@parkwaycc.co.uk
iann_bugzilla: review+
rsx11m.pub: feedback+
philip.chee: feedback+
bugspam.Callek: approval‑comm‑aurora+
bugspam.Callek: approval‑comm‑beta+
Details | Diff | Splinter Review

Description rsx11m 2013-05-20 06:44:24 PDT
Per observation in bug 872000 comment #6 and following, cookies are purged when opening the first private window (expected), but also when opening subsequent private windows (unexpected). This makes using web applications which open new windows impossible to use in a private session.

Firefox only purges cookies on opening of the first private window, then maintains the same cookie space for all private windows until the last one is closed. The respective web applications run fine with this implementation.

The optimum might be what I've suggested in Firefox bug 864640 comment #1 to open a new cookie space for each new manually invoked private window, but that's likely the most difficult way to implement this (and I'm not sure which other unexpected effects this may introduce).
Comment 1 neil@parkwaycc.co.uk 2013-06-11 03:30:14 PDT
Created attachment 760810 [details] [diff] [review]
Proposed patch

So, I did some debugging, and I found the backend code has a funky idea of what constitutes a private browsing session.

In particular, if you set the windowtype attribute, this immediately removes the window from the private browsing session, thus clearing all the cookies (because the "last" window was removed from the session).

The workaround is to remove the windowtype attribute, which doesn't remove the window from the private session but does hide it from getMostRecentWindow().
Comment 2 Josh Matthews [:jdm] 2013-06-11 08:46:02 PDT
Ah, yes, I forgot that Firefox doesn't care about any window that's not a navigator:browser.
Comment 3 rsx11m 2013-06-11 15:47:40 PDT
(In reply to neil@parkwaycc.co.uk from comment #1)
> Created attachment 760810 [details] [diff] [review]
> Proposed patch

This fixes it for me with the web applications I've tested before, so that's good. Also bug 873032 seems to be fixed by this, given that "navigator:private" does no longer create its own window type.

> The workaround is to remove the windowtype attribute, which doesn't remove the
> window from the private session but does hide it from getMostRecentWindow().

This will make a solution for bug 872521 a bit trickier though, given that a theme can no longer identify a private-browsing window based on its windowtype attribute. Maybe adding a privatebrowsingmode="temporary" attribute like Firefox does would help to compensate for the lack of a private-specific windowtype?
Comment 4 neil@parkwaycc.co.uk 2013-06-11 16:09:40 PDT
(In reply to rsx11m from comment #3)
> This fixes it for me with the web applications I've tested before, so that's
> good. Also bug 873032 seems to be fixed by this, given that
> "navigator:private" does no longer create its own window type.

I see that too, but I have no idea how that even works...
Comment 5 rsx11m 2013-06-11 16:11:25 PDT
Comment on attachment 760810 [details] [diff] [review]
Proposed patch

(In reply to neil@parkwaycc.co.uk from bug 872521 comment #6)
> (In reply to rsx11m from bug 872521 comment #5)
> > I've added a dependency on bug 874042 given that it may remove the
> > windowtype attribute specific for private windows.
> You can still style based on that, something like this perhaps:
>   :root:not([windowtype]) #throbber-box {

Ok, I didn't try that but it sounds feasible.
Comment 6 rsx11m 2013-06-11 16:11:56 PDT
(In reply to neil@parkwaycc.co.uk from comment #4)
> I see that too, but I have no idea how that even works...

Yet another case of "solved by magic" ;-)
Comment 7 Philip Chee 2013-06-17 08:14:23 PDT
Comment on attachment 760810 [details] [diff] [review]
Proposed patch

f=me Although it's a bit hacky.

Any chance this could get fixed in the backend? Or is it too complicated?
Comment 8 Josh Matthews [:jdm] 2013-06-17 08:31:21 PDT
Too complicated. There are complex issues involving non-browser windows and private session lifetimes which caused the current focus on navigator:browser windows.
Comment 9 rsx11m 2013-06-17 08:42:35 PDT
For the time being, I'm sure happy with a "hacky but works" solution. ;-)
It would be nice to still get this into 2.19 to make the feature work properly.
Comment 10 neil@parkwaycc.co.uk 2013-06-20 15:44:05 PDT
Pushed comm-central changeset 7d9d4b2d5284.
Comment 11 neil@parkwaycc.co.uk 2013-06-20 15:51:54 PDT
Comment on attachment 760810 [details] [diff] [review]
Proposed patch

[Approval Request Comment]
User impact if declined: Very hard to stay logged in while using private windows.
Risk to taking this patch (and alternatives if risky): Very low
String changes made by this patch: None

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