588651 - Privacy options are not cleared on closing SeaMonkey and instead cleared when SeaMonkey is reopened

Status: VERIFIED FIXED

(SeaMonkey :: Startup & Profiles, defect)

Description

[NoOp]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.11) Gecko/20100701 Lightning/1.0b1 SeaMonkey/2.0.6
Build Identifier: Build identifier: Mozilla/5.0 (X11; Linux i686 (x86_64); rv:2.0b4pre)
 Gecko/20100817 Lightning/1.1a1pre SeaMonkey/2.1a3

Edit|Preferences|Privacy & Security|'Always clear
my private data when I close SeaMonkey' isn't working when you *close*
out SeaMonkey (File|Quit). All the cache, data etc., is retained and is
only cleared when you start up SeaMonkey again.

This appears on the following tested builds:
Build identifier: Mozilla/5.0 (X11; Linux x86_64; rv:2.0b4pre)
Gecko/20100817 Lightning/1.1a1pre SeaMonkey/2.1a3
[64bit from:
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/2.1a3-candidates/build2/linux-x86_64/en-US/seamonkey-2.1a3.tar.bz2

Build identifier: Mozilla/5.0 (X11; Linux i686 (x86_64); rv:2.0b4pre)
Gecko/20100817 Lightning/1.1a1pre SeaMonkey/2.1a3
[32bit from:
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/2.1a3-candidates/build2/linux-i686/en-US/seamonkey-2.1a3.tar.bz2

and 
Build identifier: Mozilla/5.0 (X11; Linux x86_64; rv:2.0b5pre) Gecko/20100818 SeaMonkey/2.1b1pre
[64bit from:
http://ftp.mozilla.org/pub/mozilla.org/seamonkey/nightly/latest-comm-central-trunk/seamonkey-2.1b1pre.en-US.linux-x86_64.tar.bz2

Reproducible: Always

Steps to Reproduce:
1. Select Edit|Preferences|Privacy & Security|'Always clear
my private data and 'Ask me before clearing private data'
2. Select 'When I ask SeaMonkey to clear my private data, it should erase:
o Browsing History
o Location Bar History
o Download History
o Saved Form and Search History
o Cache
o Cookies
3. Go to a graphic intensive website (Yahoo! or Google news for example) and observer the /home/<username>/.mozilla/seamonkey/<profile>/Cache folder the ensure that data/graphics are being cached properly.
4. Close SeaMonkey
Actual Results:  
SeaMonkey closes with all cache & other privacy data in place. Only upon reopening SeaMonkey is the user presented with the 'Clear Private Data' UI so the user can elect to clear the private data (Cancel/Clear Private Data Now).

Expected Results:  
Expect the 'Clear Private Data' UI to open so the user can elect to clear the private data (Cancel/Clear Private Data Now).

This should be considered a security issue as the user expects the private data to be cleared upon closing SeaMonkey (particularly if the 'Ask me before clearing private data' has not been checked. I'm leaving 'Many users could be harmed by this security problem: it should be kept hidden from the public until it is resolved.' unchecked as these are still Alpha/Beta builds. However I consider the issue serious  enough that the issue should be a showstopper/blocker for further releases.

Comment 1

[[e-mail]]

So, the problem here is that Places fires its places-shutdown notification asynchronously, which means it doesn't happen until after xpcom-shutdown, which is useless.

Comment 2

[Edward]

This may be indicative of a crash of SeaMonkey, as it is closing.  I have seen this numerous times on the Linux platform, with 2.06 and previous versions.  

Just two minutes ago, 2.0.6 for Linux crashed as I was attempting to read m.d.a.s and the crash reporter correctly launched.  Then when SM was relaunched, the window to clear everything then appeared.

Comment 3

[[e-mail]]

Firefox no longer shows the sanitize dialog at shutdown (bug 469158) so they don't have a problem to sanitize at places shutdown (bug 470348).

Comment 4

[[e-mail]]

...except Firefox browser glue also removes their observers on xpcom-shutdown, which also means that their places shutdown observer never fires, which means that they also never backup their bookmarks or clear private data on shutdown.

Comment 5

[Marco Bonardo [:mak]]

we could fire places-shutdown at profile-change-teardown and places-connection-close at profile-before-change, synchronously. I have just to check that we are not running the sanitizer too early.
You should follow Bug 580892 for these changes since they'll probably fall in there.

Comment 6

[[e-mail]]

The patch in bug 580892 fixes this.

Comment 7

[Robert Kaiser]

Anything from bug 580892 we need to port in here?

Comment 8

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #7)
> Anything from bug 580892 we need to port in here?

Only the comment changes (nsSuiteGlue.js for us):

-        // places-shutdown is fired on profile-before-change, but before
-        // Places executes the last flush and closes connection.
+        // places-shutdown is fired when the profile is about to disappear.

The rest is in Toolkit.

Comment 9

[Robert Kaiser]

So we don't have the test they also patched?

In that case, care to do a fast fix for the comment, just for correctness?

Comment 10

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #9)
> So we don't have the test they also patched?

Oh, err, we do have it. I'm just skipping tests in patches. Call it a reflex. ;-)

I have incorporated the changes to the test locally and compared the result to the FF version of the file. Differences:
- their browserglue is our suiteglue (expected)
- their privacy.clearOnShutdown.* prefs are our privacy.item.* prefs (and we seem to be missing the .siteSettings part). Is that expected?

Comment 11

[Robert Kaiser]

(In reply to comment #10)
> Oh, err, we do have it. I'm just skipping tests in patches. Call it a reflex.
> ;-)

Hehe. Well, I have learned meanwhile to not do that, as we already have too little test coverage and I'm trying hard to increase it. :)

> - their privacy.clearOnShutdown.* prefs are our privacy.item.* prefs (and we
> seem to be missing the .siteSettings part). Is that expected?

Probably. We have a much older sanitize implementation not yet allowing things like "delete everything from the last x hours/days/etc." - and our sanitize might just miss what .siteSettings covers (content prefs? rich site storage?).
Might make sense to file bugs for those, but given what's on our list still, I doubt we'll find someone to address it before 2.1 stabilizes.

Comment 12

[Jens Hatlak (:InvisibleSmiley)]

Attachment: test changes and comment fix [Checked in: Comment 14]

Feel free to request review and assign to yourself, I won't (I'm sick and really need to go to bed now, no test headaches for me!). Haven't checked whether the test runs/succeeds either.

Comment 13

[Bruno 'Aqualon' Escherl]

Comment on attachment 470887 [details] [diff] [review]
test changes and comment fix [Checked in: Comment 14]

I can't say much about those changes themselves, but the test passes with the patch applied. So it's definitely ready for review.

Comment 14

[Ian Neal]

Comment on attachment 470887 [details] [diff] [review]
test changes and comment fix [Checked in: Comment 14]

http://hg.mozilla.org/comm-central/rev/965db964f8ac

Comment 15

[Serge Gautherie (:sgautherie)]

V.Fixed, per bug 595633 comment 3.