632746 - Port |Bug 625496 - Clear Adobe Flash Cookies (LSOs) when Cookies is selected in Clear Recent History| to SeaMonkey

Status: VERIFIED FIXED

(SeaMonkey :: Passwords & Permissions, enhancement)

Description

[Serge Gautherie (:sgautherie)]

It looks like that bug (code) should apply to SeaMonkey too.

NB: I wonder why SM has Sanitizer.jsm whereas FF has sanitize.js...

Comment 1

[Philip Chee]

Is this a dup of bug 632789

Comment 2

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #1)
> Is this a dup of bug 632789

I don't think so: That bug is for Data Manager integration while this one is for Sanitizer integration (Clear Private Data). And I think we should have this for 2.1 if possible.

Porting this should suffice:
http://hg.mozilla.org/mozilla-central/diff/37094ed97c9e/browser/base/content/sanitize.js

Comment 3

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch

Yet untested, thus neither requesting review nor assigning to myself.

Comment 4

[Jens Hatlak (:InvisibleSmiley)]

Bah, s/phInterface/phI/

Comment 5

[Jens Hatlak (:InvisibleSmiley)]

Seems to me like this currently cannot be tested at all. I have Flash 10.2 installed on Win7, used Clear Private Data / Cookies both with current Minefield and SM with the attached (fixed) patch. Still the below page still showed LSOs:
http://www.macromedia.com/support/documentation/en/flashplayer/help/settings_manager07.html

Maybe this is related to the status of this Flash bug report:
https://bugs.adobe.com/jira/browse/FP-6044

Comment 6

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 512305 [details] [diff] [review]
patch

If a code-only review is not OK, let me know. We'll have to wait until something to test against is available then, but that might be after 2.1 release, so please consider it.

Comment 7

[[e-mail]]

Comment on attachment 512305 [details] [diff] [review]
patch

>+        // Clear plugin data.
>+        let ph = Components.classes["@mozilla.org/plugin/host;1"]
>+                           .getService(Components.interfaces.nsIPluginHost);
>+        const phI = Components.interfaces.nsIPluginHost_MOZILLA_2_0_BRANCH;
>+        const FLAG_CLEAR_ALL = phInterface.FLAG_CLEAR_ALL;
>+        ph.QueryInterface(phI);
This is annoying :-( Try
// change this to nsIPluginHost after 2.0
const nsIPluginHost = Components.interfaces.nsIPluginHost_MOZILLA_2_0_BRANCH;
var ph = Components.classes["@mozilla.org/plugin/host;1"]
                   .getService(nsIPluginHost);
// delete this after 2.0
ph.QueryInterface(Components.interfaces.nsIPluginHost);

And you probably can use nsIPluginHost.FLAG_CLEAR_ALL below.

>+        // Determine age range in seconds. (-1 means clear all.) We don't know
>+        // that this.range[1] is actually now, so we compute age range based
>+        // on the lower bound. If this.range results in a negative age, do
>+        // nothing.
We don't support sanitising by range. So we should just try to clear all.

>+                  ph.clearSiteData(tags[i], null, FLAG_CLEAR_ALL, -1);
Should clearing the cache try to clear with FLAG_CLEAR_CACHE?

Comment 8

[Jens Hatlak (:InvisibleSmiley)]

Attachment: patch v2 [Checkin: comment 11]

Comment 9

[Robert Kaiser]

(In reply to comment #0)
> NB: I wonder why SM has Sanitizer.jsm whereas FF has sanitize.js...

Because FF had plans to move this to a module and an existing patch for that when I added this on our side, so I anticipated that move on our side - only that it then never happened on their side, and meanwhile, the sanitize code changed a real lot on their side as well, with the possibility to select a time frame for deletion, which was never ported to our side. Nice, isn't it?

Comment 10

[[e-mail]]

Comment on attachment 512865 [details] [diff] [review]
patch v2 [Checkin: comment 11]

Looks reasonable.

Comment 11

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 512865 [details] [diff] [review]
patch v2 [Checkin: comment 11]

http://hg.mozilla.org/comm-central/rev/01ed52fae3a7

Comment 12

[[e-mail]]

(In reply to comment #9)
> (In reply to comment #0)
> > NB: I wonder why SM has Sanitizer.jsm whereas FF has sanitize.js...
> 
> Because FF had plans to move this to a module and an existing patch for that
> when I added this on our side, so I anticipated that move on our side - only
> that it then never happened on their side, and meanwhile, the sanitize code
> changed a real lot on their side as well, with the possibility to select a time
> frame for deletion, which was never ported to our side. Nice, isn't it?

If only you'd blindly cut & pasted code instead of doing some real programming then Jens wouldn't have had any work to do ;-)

Comment 13

[Jens Hatlak (:InvisibleSmiley)]

Verified with Flash 10.3 beta on Win 7. :-)

Comment 14

[H. Hofer]

Is it intentional, that all LSOs are cleared on cache deletion?

Shouldn't it be on cookies only? FF definitely does it that way.
(http://hg.mozilla.org/mozilla-central/rev/37094ed97c9e, http://mxr.mozilla.org/mozilla-central/source/browser/base/content/sanitize.js)

Follow-up bug?

Comment 15

[Jens Hatlak (:InvisibleSmiley)]

(In reply to comment #14)
> Is it intentional, that all LSOs are cleared on cache deletion?
> 
> Shouldn't it be on cookies only? FF definitely does it that way.
> (http://hg.mozilla.org/mozilla-central/rev/37094ed97c9e,
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/sanitize.
> js)

The difference between the FF implementation and ours is that we call clearSiteData with flag FLAG_CLEAR_CACHE when we clear the cache, while FF doesn't call that function at all when clearing the cache. See comment 7.

From http://mxr.mozilla.org/comm-central/source/mozilla/dom/plugins/base/nsIPluginHost.idl:

FLAG_CLEAR_ALL: clear all data associated with a site.
FLAG_CLEAR_CACHE: clear cached data that can be retrieved again without loss of functionality. To be used out of concern for space and not necessarily privacy.

> Follow-up bug?

I have to admit that we implemented this before a version of Flash was released that we could test against. From the IDL description it sounded like the right thing to do, but if it's unexpected or wrong, maybe that behavior needs to be changed. Feel free to file a follow-up bug if you think this is wrong.

Comment 16

[H. Hofer]

(In reply to comment #15)
> The difference between the FF implementation and ours is that we call
> clearSiteData with flag FLAG_CLEAR_CACHE when we clear the cache, while FF
> doesn't call that function at all when clearing the cache. See comment 7.

Ah, my bad! I didn't look hard enough ;-)

> I have to admit that we implemented this before a version of Flash was
> released that we could test against. From the IDL description it sounded
> like the right thing to do, but if it's unexpected or wrong, maybe that
> behavior needs to be changed.

Well, it still sounds right, but the last time I checked, _all_ my LSOs were gone. I have to check, if it happens with LSOs that contain valuable data. (Does the YouTube volume setting count as valuable :-) ? )

If it really happens, I'll file a bug...

Comment 17

[H. Hofer]

(In reply to comment #16)
> If it really happens, I'll file a bug...

Filed bug 665336