Closed Bug 632746 Opened 13 years ago Closed 13 years ago

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

Categories

(SeaMonkey :: Passwords & Permissions, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
seamonkey2.1b3

People

(Reporter: sgautherie, Assigned: InvisibleSmiley)

References

()

Details

(Whiteboard: [FF2SM])

Attachments

(1 file, 1 obsolete file)

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

NB: I wonder why SM has Sanitizer.jsm whereas FF has sanitize.js...
Whiteboard: [FF2SM]
Is this a dup of bug 632789
(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
Attached patch patch (obsolete) — Splinter Review
Yet untested, thus neither requesting review nor assigning to myself.
Bah, s/phInterface/phI/
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 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.
Attachment #512305 - Flags: review?(neil)
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?
Attachment #512305 - Flags: review?(neil) → review-
Assignee: nobody → jh
Attachment #512305 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #512865 - Flags: review?(neil)
(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 on attachment 512865 [details] [diff] [review]
patch v2 [Checkin: comment 11]

Looks reasonable.
Attachment #512865 - Flags: review?(neil) → review+
Comment on attachment 512865 [details] [diff] [review]
patch v2 [Checkin: comment 11]

http://hg.mozilla.org/comm-central/rev/01ed52fae3a7
Attachment #512865 - Attachment description: patch v2 → patch v2 [Checkin: comment 11]
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.1b3
(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 ;-)
blocking-seamonkey2.1: ? → ---
Verified with Flash 10.3 beta on Win 7. :-)
Status: RESOLVED → VERIFIED
Blocks: 653418
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?
(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.
(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...
(In reply to comment #16)
> If it really happens, I'll file a bug...

Filed bug 665336
Blocks: 665336
You need to log in before you can comment on or make changes to this bug.