328463 - Sanitize functionality should be extendable.

Status: RESOLVED INCOMPLETE

(Firefox :: Settings UI, enhancement)

Description

[Nils Maier [:nmaier]]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1

Sanitize is a great feature, but it is not extendable.
A lot of extensions store "private" data, and have no way to "fully" integrate with Sanitize.

Currently there is no unified interface to this functionality.
nsIBrowserGlue (implemented in components/nsBrowserGlue.js) provides the UI-less functionality (by interanlly loading sanitize.js as a subscript).
The UI on the other hand directly grabs sanitize.js.

Methods allowing the addition of items are not provided.

I could imagine an implementation like this:
All is done in a single component (eg. ns(I)BrowserGlue).
The UI uses this component too.

Extensions may provide simple components (based on a simple interface) to add themselves.
This might be done eg. by using a category (categorymanager) where these components add themselves upon registration.

Reproducible: Always

Steps to Reproduce:

Comment 1

[Dave Townsend [:mossop]]

I guess this is a valid request so changing to NEW.

Comment 2

[Nils Maier [:nmaier]]

Attachment: v1, trunk patch

first attempt of a patch.
Subject to discussion, as it affects a lot of stuff incl. UI

Comment 3

[Nils Maier [:nmaier]]

Attachment: v1, UI screenshot

Comment 4

[Nils Maier [:nmaier]]

Attachment: v1, sample extension

Sample extension to demonstrate extensible "Clear Private Data"

Comment 5

[Nils Maier [:nmaier]]

As nobody else seemed to work on this I did an attempt.
Actually not the optimal thing to work with being a n00b to mozilla development, as lots of stuff is involved.

Reasons
-------
It's damn nice that Firefox provides such functionality.
But a lot of extensions leave a lot of traces behind as well.
Currently there is no good way to extend built-in functionality. I know that, I wrote overlay code for downTHEMall.
So extension authors
* have the burden to overlay the existing functionality (which is always a point of failure due to incompatiblities)
* or they bake their own stuff which then does not integrate at all. So users would have to "clean" twice or more.
* or they simply don't implement such functionality.

Patch discussion
----------------

* Extensible sanitize functionality.
I decided to use CategoryManager, so that extensions can easily add their own SanitizerServices.
A pretty simple interface (mozISanitizerService) was written, which is easy to implement and "maintain". It is pretty similar to the current implementation.
Extension authors should be able to implement their sanitizers without having to overlay UI and fixing thing upon each FX (point-)release.

* User interface
There should be an icon, so that the user can find things more easily, and a description ("inline help").
Previous interface provided no real help, and e.g. I actually had a hard time figuring out what that "sessions" stuff is after all.

Issues
------
* Localization/standard text - my text is pretty bad, I'm not even a native English speaker.
* User interface - currently pleases just me, needs criticism from other people.
* Mac - No mac here, somebody else has to test
* The whole sanitizer stuff might receive an own directory; now that there are more and just one file.
* Newbie - I need guidance, as I don't know much about the whole mozilla developement/review/patch process.

Awaiting comments. ;)

Comment 6

[:Gavin Sharp [email: [e-mail]]]

I just took a quick look, but this looks like a great patch, Nils. One minor issue I noticed is the whitespace changes. Mozilla code does not use tabs for indentation. The standard for JavaScript is two space indentation.

It might be a good idea to post to the mozilla.dev.apps.firefox explaining the patch (or just post something like comment 5). When you think the patch is ready to receive formal review, you can ask for review from one of the people at http://www.mozilla.org/projects/firefox/review.html using the review? flag. I've granted you "editbugs" and "canconfirm" Bugzilla privileges, so you can also take ownership of the bug by assigning it to yourself.

Comment 7

[Nils Maier [:nmaier]]

Attachment: v2, trunk patch

Changes since v1:

* rewritten sanitize.js (Sanitizer.prototype.sanitize, Sanitizer.sanitize stuff was just confusing)
* internal result codes (boolean instead of object containing exceptions)
* fixed a small bug in original implementation: UI on shutdown would always save success
* description cutoff and tooltips
* indentation fixed
* locale (by mcdavis941)
* pinstripe theme modified accordingly to mozapps/extensions.css (still untested, no macs around)
* order: FX built-ins come first, then alphabetically after id.

Comment 8

[Nils Maier [:nmaier]]

Attachment: v2, try it yourself extension (patch-alike)

This extension will "patch" your FX via overrides.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 257916 [details] [diff] [review]
v2, trunk patch

Nils, I'm embarrassed to be canceling this review request two years after it was requested, but the code it's patching has changed a lot so it no longer applies. If you're still interested in pursuing a fix for this bug once the work in bug 480169 is complete, I'm sure it will get prompt attention. The lack of attention here is a failure on my part, and I'm determined to avoid failing this badly again.

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Note also bug 462041.

Comment 11

[Willy_ Foo_Foo]

I use this button relatively often for its intended purpose, and furthermore I have not once encountered a website where the image has been split up
so can there be a easy way to toggle this on off(firebug is not too friendly with casual users)

Comment 12

[Nils Maier [:nmaier]]

Outdated.