The default bug view has changed. See this FAQ.

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

VERIFIED FIXED in seamonkey2.1b3

Status

SeaMonkey
Passwords & Permissions
--
enhancement
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: sgautherie, Assigned: InvisibleSmiley)

Tracking

Trunk
seamonkey2.1b3
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [FF2SM], URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

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

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

Updated

6 years ago
Whiteboard: [FF2SM]

Comment 1

6 years ago
Is this a dup of bug 632789
(Assignee)

Comment 2

6 years ago
(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
(Assignee)

Comment 3

6 years ago
Created attachment 512305 [details] [diff] [review]
patch

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

Comment 4

6 years ago
Bah, s/phInterface/phI/
(Assignee)

Comment 5

6 years ago
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
(Assignee)

Comment 6

6 years ago
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 7

6 years ago
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)

Comment 8

6 years ago
Created attachment 512865 [details] [diff] [review]
patch v2 [Checkin: comment 11]
Assignee: nobody → jh
Attachment #512305 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #512865 - Flags: review?(neil)

Comment 9

6 years ago
(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+
(Assignee)

Comment 11

6 years ago
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]
(Assignee)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 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 ;-)

Updated

6 years ago
blocking-seamonkey2.1: ? → ---
(Assignee)

Comment 13

6 years ago
Verified with Flash 10.3 beta on Win 7. :-)
Status: RESOLVED → VERIFIED

Updated

6 years ago
Blocks: 653418

Comment 14

6 years ago
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?
(Assignee)

Comment 15

6 years ago
(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

6 years ago
(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

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

Filed bug 665336

Updated

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