Last Comment Bug 632746 - Port |Bug 625496 - Clear Adobe Flash Cookies (LSOs) when Cookies is selected in Clear Recent History| to SeaMonkey
: Port |Bug 625496 - Clear Adobe Flash Cookies (LSOs) when Cookies is selected ...
Status: VERIFIED FIXED
[FF2SM]
:
Product: SeaMonkey
Classification: Client Software
Component: Passwords & Permissions (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: seamonkey2.1b3
Assigned To: Jens Hatlak (:InvisibleSmiley)
:
Mentors:
http://mxr.mozilla.org/comm-central/s...
Depends on: 625496
Blocks: 653418 665336
  Show dependency treegraph
 
Reported: 2011-02-09 01:26 PST by Serge Gautherie (:sgautherie)
Modified: 2011-06-20 08:19 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (2.27 KB, patch)
2011-02-14 15:18 PST, Jens Hatlak (:InvisibleSmiley)
neil: review-
Details | Diff | Splinter Review
patch v2 [Checkin: comment 11] (2.53 KB, patch)
2011-02-16 11:38 PST, Jens Hatlak (:InvisibleSmiley)
neil: review+
Details | Diff | Splinter Review

Description Serge Gautherie (:sgautherie) 2011-02-09 01:26:38 PST
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 2011-02-09 09:03:46 PST
Is this a dup of bug 632789
Comment 2 Jens Hatlak (:InvisibleSmiley) 2011-02-09 16:03:31 PST
(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) 2011-02-14 15:18:22 PST
Created attachment 512305 [details] [diff] [review]
patch

Yet untested, thus neither requesting review nor assigning to myself.
Comment 4 Jens Hatlak (:InvisibleSmiley) 2011-02-14 15:20:24 PST
Bah, s/phInterface/phI/
Comment 5 Jens Hatlak (:InvisibleSmiley) 2011-02-14 15:50:46 PST
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) 2011-02-15 13:52:57 PST
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 neil@parkwaycc.co.uk 2011-02-16 04:43:57 PST
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) 2011-02-16 11:38:25 PST
Created attachment 512865 [details] [diff] [review]
patch v2 [Checkin: comment 11]
Comment 9 Robert Kaiser 2011-02-16 16:19:50 PST
(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 neil@parkwaycc.co.uk 2011-02-16 16:27:20 PST
Comment on attachment 512865 [details] [diff] [review]
patch v2 [Checkin: comment 11]

Looks reasonable.
Comment 11 Jens Hatlak (:InvisibleSmiley) 2011-02-17 09:34:40 PST
Comment on attachment 512865 [details] [diff] [review]
patch v2 [Checkin: comment 11]

http://hg.mozilla.org/comm-central/rev/01ed52fae3a7
Comment 12 neil@parkwaycc.co.uk 2011-02-20 14:29:32 PST
(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) 2011-03-08 09:57:45 PST
Verified with Flash 10.3 beta on Win 7. :-)
Comment 14 H. Hofer 2011-06-17 13:06:38 PDT
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) 2011-06-17 13:18:13 PDT
(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 2011-06-18 00:47:52 PDT
(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 2011-06-19 00:43:18 PDT
(In reply to comment #16)
> If it really happens, I'll file a bug...

Filed bug 665336

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