Last Comment Bug 741336 - Provide UI to easily clear cookies from the menus
: Provide UI to easily clear cookies from the menus
Product: Thunderbird
Classification: Client Software
Component: Mail Window Front End (show other bugs)
: Trunk
: All All
-- normal (vote)
: Thunderbird 17.0
Assigned To: awjohnston1
: 777867 (view as bug list)
Depends on:
Blocks: 789678
  Show dependency treegraph
Reported: 2012-04-02 03:53 PDT by Mark Banner (:standard8)
Modified: 2012-09-08 03:02 PDT (History)
5 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

clearHistoryPatchv1 (54.85 KB, patch)
2012-07-07 12:24 PDT, awjohnston1
no flags Details | Diff | Splinter Review
clearHistoryPatchv2 (54.47 KB, patch)
2012-07-07 16:08 PDT, awjohnston1
no flags Details | Diff | Splinter Review
interfacePatchv1 (57.37 KB, patch)
2012-07-09 15:01 PDT, awjohnston1
no flags Details | Diff | Splinter Review
interfacePatchv2 (57.93 KB, patch)
2012-07-09 16:16 PDT, awjohnston1
no flags Details | Diff | Splinter Review
clearHistoryPatchv3 (57.95 KB, patch)
2012-07-12 12:55 PDT, awjohnston1
no flags Details | Diff | Splinter Review
clearHistoryPatchv4 (57.93 KB, patch)
2012-07-12 13:13 PDT, awjohnston1
no flags Details | Diff | Splinter Review
clearHistoryv5 (57.03 KB, patch)
2012-07-12 13:48 PDT, awjohnston1
standard8: feedback+
Details | Diff | Splinter Review
clearHistoryPatchv6 (57.12 KB, patch)
2012-07-16 13:22 PDT, awjohnston1
no flags Details | Diff | Splinter Review
obsolete (57.00 KB, patch)
2012-07-16 14:05 PDT, awjohnston1
no flags Details | Diff | Splinter Review
clearHistoryPatchv7 (57.00 KB, patch)
2012-07-16 14:08 PDT, awjohnston1
bwinton: ui‑review+
Details | Diff | Splinter Review
clearHistoryPatchv8 (57.26 KB, patch)
2012-07-20 11:42 PDT, awjohnston1
no flags Details | Diff | Splinter Review
clearHistoryPatchv9 (57.26 KB, patch)
2012-07-20 12:13 PDT, awjohnston1
no flags Details | Diff | Splinter Review
clearHistoryUpdatev1 (5.26 KB, patch)
2012-07-30 13:17 PDT, awjohnston1
standard8: review+
Details | Diff | Splinter Review

Description User image Mark Banner (:standard8) 2012-04-02 03:53:09 PDT
As suggested by the privacy review of the web search feature, we should have a UI that allows the user to clear the recent history easily (currently cookie clearing is buried in preferences).

This should be something like the Tools -> Clear Recent History option that Firefox has, probably in the same place for Thunderbird.
Comment 1 User image Mark Banner (:standard8) 2012-04-25 07:37:13 PDT
The main work here is to copy sanitize.js/.xul from Firefox and to modify them for Thundebird's requirements - some of the options don't make sense for Thunderbird. Then obviously integrate those into the UI.
Comment 2 User image awjohnston1 2012-07-02 09:24:10 PDT
Notes for me: 

sanatize.js is located at

The pop-up dialog window for the clear recent history option is
Comment 3 User image awjohnston1 2012-07-04 11:50:03 PDT
The file
loads the sanitize.js script and implements the sanitize function of the interface nsIBrowserGlue. Should we have mailGlue.js inheriting from nsIBrowserGlue and add the required function?
Comment 4 User image Mark Banner (:standard8) 2012-07-05 02:28:02 PDT
(In reply to awjohnston1 from comment #3)
> The file
> nsBrowserGlue.js
> loads the sanitize.js script and implements the sanitize function of the
> interface nsIBrowserGlue. Should we have mailGlue.js inheriting from
> nsIBrowserGlue and add the required function?

What you'll need to do is create and nsIMailGlue.idl file based on nsIBrowserGlue.idl. The only function you'll need in nsIMailGlue is the sanitize function. You then need to hook that inferface into the (see XPIDLSRCS in the alongside nsIBrowserGlue), and into mailGlue.js.

For mailGlue.js, you should just have to add nsIMailGlue to the array passed to XPCOMUtils.generateQI, and then define the sanitize function itself.
Comment 5 User image awjohnston1 2012-07-07 12:24:47 PDT
Created attachment 639988 [details] [diff] [review]

Tested in Windows 7, clears cookies and cache from Tools=>Clear Recent History menu option. Still need to create and implement interface nsIMailGlue. No automated tests yet.
Comment 6 User image awjohnston1 2012-07-07 12:30:39 PDT
Also, should consider a Ctrl+Shift+Del command as in Firefox.
Comment 7 User image awjohnston1 2012-07-07 16:08:09 PDT
Created attachment 640002 [details] [diff] [review]

Error in last patch, file listed in jar manifest which was not necessary and not tracked by hg. Also some debugging lines I forgot to remove. Should work now.
Comment 8 User image awjohnston1 2012-07-09 15:01:10 PDT
Created attachment 640379 [details] [diff] [review]

Tried to use the interface nsIMailGlue.idl. The instructions for the Windows build of .xpt found here
generate nothing but errors on my system. Don't have access to a Linux box right now, if anyone can give me tips on using or can run this patch it would be much appreciated.
Comment 9 User image Kent James (:rkent) 2012-07-09 15:25:21 PDT
You don't normally need to run for files located in the tree, because it is done automatically.

I loaded your patch, and compiled the mail/components directory and it nsIMailGlue is compiling fine. You can see the file mail/components/_xpidlgen/nsIMailGlue.h in the object directory for example.

You should not call this nsIBrowserGlue though.

You need to be more specific than "nothing but errors" to say what is failing for you.
Comment 10 User image awjohnston1 2012-07-09 16:16:17 PDT
Created attachment 640407 [details] [diff] [review]

corrected some typos.
Comment 11 User image awjohnston1 2012-07-12 12:55:43 PDT
Created attachment 641568 [details] [diff] [review]

Fixed a syntax error. The UI now appears to clear data successfully.
Comment 12 User image awjohnston1 2012-07-12 13:13:32 PDT
Created attachment 641572 [details] [diff] [review]

removed an unnecessary variable declaration.
Comment 13 User image awjohnston1 2012-07-12 13:19:42 PDT
What needs to be implemented for testing? Looking at the tests from Firefox, the following tests are related to this bug:


I think that the first is not relevant to us. In sanitize.js we can see that the timespan is never applied to the cache items. They don't have a time stamp and all are deleted. The cookies are not tested in sanitize_timespan.js  A comment I read in there states that nsICookieManager does not allow fake creation times.

The second seems relevant, although only for data in Tbird. Ie not history and downloads etc.
Comment 14 User image awjohnston1 2012-07-12 13:21:15 PDT
I also flagged the latest patch for review. Is there anything else I should be doing in regards to this?
Comment 15 User image awjohnston1 2012-07-12 13:48:46 PDT
Created attachment 641582 [details] [diff] [review]

removed some code which is not necessary for thunderbird.
Comment 16 User image Mark Banner (:standard8) 2012-07-16 04:40:01 PDT
Comment on attachment 641582 [details] [diff] [review]

Review of attachment 641582 [details] [diff] [review]:

Hi Andrew, this is looking really great. So I'm going to give it feedback+ and request ui-review from Blake, so we can get his input on it before we refine the patch fully (note: to request review, set the flag to '?' and then put the name/email in the field that appears).

I have gone through it and added some comments anyway, mainly about improving the style of the code, but there's some things like making sure we add default prefs so the dialog looks good for first time users.

::: mail/base/content/mailCore.js
@@ +351,5 @@
>  }
> +function toSanitize()
> +{
> +  Components.classes[";1"].getService(Components.interfaces.nsIMailGlue).sanitize(window);

nit: please start the .getService on the next line with the dot aligned with the dot of the Components.classes on the previous line.

You could probably wrap .sanitize onto a new line as well.

This just helps to keep line lengths down and makes it easier to read.

::: mail/base/content/mailWindowOverlay.xul
@@ +1672,5 @@
>          <menuitem id="menu_import" label="&importCmd.label;"
>                    accesskey="&importCmd.accesskey;"
>                    oncommand="toImport();"/>
>          <menuitem id="javascriptConsole" label="&errorConsoleCmd.label;" accesskey="&errorConsoleCmd.accesskey;" key="key_errorConsole" oncommand="toJavaScriptConsole();"/>
> +		<menuitem id="sanitizeHistory"

nit: In various places in the patch you've got tabs rather than spaces. We only use spaces so that formatting is maintained everywhere.

::: mail/base/content/sanitize.js
@@ +1,1 @@
> +# -*- Mode: Java; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*-

I think we can just drop this modeline.

@@ +1,2 @@
> +# -*- Mode: Java; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*-
> +# This Source Code Form is subject to the terms of the Mozilla Public

Please switch to the /* ... */ form ( and with the change to processing mentioned below, you can then drop the * at the start of the line for this file in as preprocessing will no longer be required.

@@ +33,5 @@
> +  sanitize: function ()
> +  {
> +    var psvc = Components.classes[";1"]
> +                         .getService(Components.interfaces.nsIPrefService);
> +    var branch = psvc.getBranch(this.prefDomain);

At the start of the file include the line:


Then you can make this:

var branch = Services.prefs.getBranch(this.prefDomain);

(and remove the var psvc above it)

@@ +81,5 @@
> +                          getService(Ci.nsICacheService);
> +        try {
> +          // Cache doesn't consult timespan, nor does it have the
> +          // facility for timespan-based eviction.  Wipe it.
> +          cacheService.evictEntries(Ci.nsICache.STORE_ANYWHERE);


@@ +96,5 @@
> +    cookies: {
> +      clear: function ()
> +      {
> +        var cookieMgr = Components.classes[";1"]
> +                                  .getService(Ci.nsICookieManager);

cookieMgr can be replaced by Services.cookies

@@ +147,5 @@
> +        // clear any network geolocation provider sessions
> +        var psvc = Components.classes[";1"]
> +                             .getService(Components.interfaces.nsIPrefService);
> +        try {
> +            var branch = psvc.getBranch("geo.wifi.access_token.");


@@ +223,5 @@
> +
> +// Shows sanitization UI
> +Sanitizer.showUI = function(aParentWindow) 
> +{
> +  var ww = Components.classes[";1"]

ww can be replaced by Services.ww

@@ +226,5 @@
> +{
> +  var ww = Components.classes[";1"]
> +                     .getService(Components.interfaces.nsIWindowWatcher);
> +#ifdef XP_MACOSX
> +  ww.openWindow(null, // make this an app-modal window on Mac

Here we can use:

ww.openWindow(Application.platformIsMac ? null : aParentWindow, ...

We can then drop the #ifdefs, and remove the need for preprocessing.

@@ +245,5 @@
> +{
> +  Sanitizer.showUI(aParentWindow);
> +};
> +
> +Sanitizer.onStartup = function() 

I'm not convinced we need the startup/shutdown functions at the moment (you're not calling them at the moment anyway). Cookies can already have session expiry, and there's an item for that in the preferences window. We also don't have an option for clearing the shutdown cache, and I think given the way TB works we don't need that at the moment (although it could always be done in a follow-up if we wanted it).

::: mail/base/content/sanitize.xul
@@ +1,3 @@
> +<?xml version="1.0"?>
> +
> +# -*- Mode: Java; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*-

This isn't a java file, so we should just drop this line. We generally don't need it anyway.

@@ +2,5 @@
> +
> +# -*- Mode: Java; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*-
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at

Please use the <!-- --> form of this (available from

Then drop the star at the start of the line in which avoids the need for preprocessing.

@@ +21,5 @@
> +            xmlns=""
> +            dlgbuttons="accept,cancel"
> +            title="&sanitizeDialog2.title;"
> +            noneverythingtitle="&sanitizeDialog2.title;"
> +            style="width: &dialog.width;;"

nit: only one ; necessary.

@@ +35,5 @@
> +    <script type="application/javascript"
> +            src="chrome://messenger/content/sanitizeDialog.js"/>
> +
> +    <preferences id="sanitizePreferences">
> +      <preference id="privacy.cpd.cookies"               name="privacy.cpd.cookies"               type="bool"/>

You need to copy the relevant default preferences for privacy.cpd.cookies and privacy.cpd.cache from Firefox's firefox.js to the all-thunderbird.js file. This will ensure that the first time user of the dialog has pre-seeded values for the check boxes.

@@ +40,5 @@
> +      <preference id="privacy.cpd.cache"                 name="privacy.cpd.cache"                 type="bool"/>
> +    </preferences>
> +    
> +    <preferences id="nonItemPreferences">
> +      <preference id="privacy.sanitize.timeSpan"

Also add privacy.sanitize.timeSpan to all-thunderbird.js

@@ +70,5 @@
> +
> +    <separator class="thin"/>
> +
> +
> +      <vbox id="sanitizeEverythingWarningBox">

nit: only one blank line before this.

::: mail/base/
@@ +109,5 @@
>      content/messenger/safeMode.js                   (content/safeMode.js)
> +*	content/messenger/sanitize.xul					(content/sanitize.xul)
> +*	content/messenger/sanitize.js					(content/sanitize.js)
> +	content/messenger/sanitizeDialog.css			(content/sanitizeDialog.css)
> +*	content/messenger/sanitizeDialog.js				(content/sanitizeDialog.js)

sanitizeDialog.js doesn't need pre-processing, so the * can be dropped from the start of the line.

::: mail/components/
@@ +14,5 @@
> +XPIDL_MODULE = mailcompsbase
> +
> +nsIMailGlue.idl \
> +$(NULL)

nit: 2-space indent nsIMailGlue.idl \ and the $(NULL) please.

::: mail/components/mailGlue.js
@@ +35,1 @@
>  MailGlue.prototype = {

nit: no need for the added lines just before this.

@@ +109,5 @@
>    // for XPCOM
>    classID: Components.ID("{eb239c82-fac9-431e-98d7-11cacd0f71b8}"),
> +  QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver,
> +										 Ci.nsIMailGlue]),

nit: Ci.nsIMailGlue should align with the Ci. on the previous line.
Comment 17 User image awjohnston1 2012-07-16 13:22:39 PDT
Created attachment 642696 [details] [diff] [review]

made requested changes from Comment 16. everything works except for timespan. if select a timespan other than everything, cookies are not cleared appropriately. i will try to fix this.
Comment 18 User image awjohnston1 2012-07-16 14:05:20 PDT
Created attachment 642720 [details] [diff] [review]
Comment 19 User image awjohnston1 2012-07-16 14:08:17 PDT
Created attachment 642721 [details] [diff] [review]

fixed error, dialog appears to function as it should.
Comment 20 User image Blake Winton (:bwinton) (:☕️) 2012-07-16 14:34:47 PDT
This patch doesn't apply cleanly for me on the latest trunk build.  Could you please update your repository and attach a new one?

(In the meantime, I'll try to hand-edit it, and get it compiling…)

Comment 21 User image Blake Winton (:bwinton) (:☕️) 2012-07-20 09:03:31 PDT
Comment on attachment 642721 [details] [diff] [review]

The details seem small enough that I would be tempted to just inline them, but leaving it this way makes it consistent with Firefox, so I'll say ui-r=me.

Comment 22 User image awjohnston1 2012-07-20 11:42:46 PDT
Created attachment 644407 [details] [diff] [review]

trying to rebase. currently this patch fails to build.
Comment 23 User image awjohnston1 2012-07-20 12:13:35 PDT
Created attachment 644415 [details] [diff] [review]

rebased the patch with source I downloaded this morning. seems to be functional now. mconley has built and found an idiosyncracy with the build system. it does not like the fact that I try to add nsIMailGlue.idl He is trying a build to resolve.
Comment 24 User image Blake Winton (:bwinton) (:☕️) 2012-07-20 12:17:28 PDT
(Did you change the UI between v7 and v9?  Cause if not, you can just carry forward my previous ui-r+…)
Comment 25 User image awjohnston1 2012-07-20 12:47:49 PDT
no, I did not change the UI. Didn't realize you had already reviewed :)
Comment 26 User image awjohnston1 2012-07-23 11:36:37 PDT
mconley gave clearHistoryPatchv9 a try build. build was successful except for a known random orange in the mozmill testing. what is the next step I should take to get the patch committed?
Comment 27 User image Ryan VanderMeulen [:RyanVM] 2012-07-23 16:32:57 PDT

Thanks for the patch, Andrew! One request - to make life easier for those checking in on your behalf, please make sure your future patches follow the directions at the link below so that all the necessary commit information is present. Thanks!
Comment 28 User image Mark Banner (:standard8) 2012-07-30 07:13:15 PDT
Hi Andrew,

Unfortunately we missed a step that your patch needs a complete review+ before landing.

Rather than back it out, I'll go through the patch and review it, then we can address an issues in a follow-up patch, if that is ok with you?
Comment 29 User image Mark Banner (:standard8) 2012-07-30 07:24:43 PDT
Comment on attachment 644415 [details] [diff] [review]

Review of attachment 644415 [details] [diff] [review]:

Ok, there's a few comments here, if you can provide a patch which fixes those, and request review from me, then we can get those landed, and I think this will be fully fixed then.

BTW, sorry for not commenting earlier, I was on holiday.

::: mail/base/content/sanitize.js
@@ +210,5 @@
> +{
> +  return Sanitizer._prefs ? Sanitizer._prefs
> +    : Sanitizer._prefs = Components.classes[";1"]
> +                         .getService(Components.interfaces.nsIPrefService)
> +                         .getBranch(Sanitizer.prefDomain);

Should be able to use Services.prefs.getBranch here.

::: mail/base/
@@ +106,5 @@
>      content/messenger/browserRequest.js             (content/browserRequest.js)
>      content/messenger/browserRequest.xul            (content/browserRequest.xul)
>  *   content/messenger/safeMode.xul                  (content/safeMode.xul)
>      content/messenger/safeMode.js                   (content/safeMode.js)
> +	content/messenger/sanitize.xul					(content/sanitize.xul)

The entries in this file are still using tabs.

::: mail/components/
@@ +10,5 @@
>  include $(DEPTH)/config/
> +
> +MODULE = mailcomps
> +XPIDL_MODULE = mailcompsbase

You need to add mailcompsbase to this file somewhere around the indicated line.

Otherwise the interface details aren't shipped in release builds, and the dialog won't work.

::: mail/components/mailGlue.js
@@ +19,5 @@
>   * MailUtils.js instead.
>   */
>  function MailGlue() {
> +	XPCOMUtils.defineLazyGetter(this, "_sanitizer",

You've also still got tabs in this function, and when adding the sanitize function further down in the file.
Comment 30 User image Mark Banner (:standard8) 2012-07-30 07:32:51 PDT
*** Bug 777867 has been marked as a duplicate of this bug. ***
Comment 31 User image awjohnston1 2012-07-30 13:17:03 PDT
Created attachment 647271 [details] [diff] [review]

Thanks for the feedback Mark. I made the requested changes to the latest trunk and had a successful build on my machine. The .xpt has been added to the manifest, hopefully this will resolve the issue noticed by others.
Comment 32 User image Mark Banner (:standard8) 2012-07-31 02:26:28 PDT
Comment on attachment 647271 [details] [diff] [review]

That's great, thanks, I'll check this in in a few minutes.
Comment 33 User image Mark Banner (:standard8) 2012-07-31 02:55:40 PDT
Checked in:

Many thanks Andrew!

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