Last Comment Bug 359284 - IMAP: Auto-expunge for IMAP mailboxes
: IMAP: Auto-expunge for IMAP mailboxes
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: -- normal with 4 votes (vote)
: Thunderbird 3.0a3
Assigned To: Dale Wiggins
:
:
Mentors:
: 187487 327374 330772 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-11-02 14:06 PST by Matt Dudziak
Modified: 2009-10-29 11:08 PDT (History)
15 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Implementation of IMAP auto-expunge option (6.18 KB, patch)
2007-10-15 16:16 PDT, Jeff Beckley
no flags Details | Diff | Splinter Review
Implementation for auto-expunge feature (3.69 KB, patch)
2007-12-17 13:29 PST, Dale Wiggins
no flags Details | Diff | Splinter Review
Unbitrotted version of my last patch (3.60 KB, patch)
2008-07-14 10:11 PDT, Dale Wiggins
mozilla: superreview+
Details | Diff | Splinter Review
New patch with combined if statements (3.42 KB, patch)
2008-07-17 08:42 PDT, Dale Wiggins
mnyromyr: review+
Details | Diff | Splinter Review
Patch with brace and spacing changes (3.42 KB, patch)
2008-07-22 08:10 PDT, Dale Wiggins
no flags Details | Diff | Splinter Review
Unbitrotted version of my last patch (2.85 KB, patch)
2008-08-28 08:17 PDT, Dale Wiggins
no flags Details | Diff | Splinter Review

Description Matt Dudziak 2006-11-02 14:06:12 PST
Penelope should have the option like the current Eudora to automatically expunge mailboxes based upon user preference. Eudora support the options:
Always
Never
When deleted message % exceeds a user-settable percentage.

This should be a per-account option (and perhaps per-mailbox?).
Comment 1 Magnus Melin 2007-06-28 05:48:25 PDT
Thunderbird already have this, as a global option.
Preferences > Advanced > Network & Diskspace: Compact folders when it will save over [  ] KB
Comment 2 Eric Moore 2007-09-06 05:07:41 PDT
They're not the same thing, especially for IMAP users. 

1. You're overlooking the fact that you typically want quite different settings for POP accounts (where your main worry is mbox files getting corrupted, especially the inbox folder) and IMAP accounts (where your main worry is filling up your mailbox). 

2. It doesn't work for IMAP accounts as far as I can tell.

On my system tools -> options -> advanced -> network & disk space -> disk space is configured to automatically compact a folder when it will save over 25 KB. That works as expected with POP accounts and the local folders directory, both of which use mbox files. I deleted several hundred KB of messages in both a remote junk folder and a remote backscatter folder, exited, and restarted Thunderbird. It didn't compact either remote folder. I've never seen that feature work with remote folders. "Clean up (expunge) inbox on exit" and "empty trash on exit" compact the remote inbox and trash folders respectively on exit. I've verified that using IMAP logging. However, thats a unrelated setting.

I'm using Thunderbird 2.0.0.6 with XP pro SP2.

Please don't make the Penelope extension a second class citizen. 
Comment 3 Jeff Beckley 2007-10-15 15:49:12 PDT
Changing to be under Thunderbird as the upcoming fix is implemented there.
Comment 4 Jeff Beckley 2007-10-15 16:16:44 PDT
Created attachment 285016 [details] [diff] [review]
Implementation of IMAP auto-expunge option

This patch implements the IMAP auto-expunge option.  The new pref for the option is "mail.imap.expunge_option", which can have one of three values:

0 - Never auto expunge messages
1 - Auto-expunge messages right away
2 - Expunge messages after a number of messages threshold (see bug 399835)

Note that this patch also includes the fixes for bug 399835.

The code was written by Dale Wiggins <dwiggins@qualcomm.com>, so he will remain assigned to it.
Comment 5 Magnus Melin 2007-12-17 10:43:36 PST
Jeff/Dale, can you post a new patch excluding the fix for bug 399835.
Comment 6 Dale Wiggins 2007-12-17 13:29:39 PST
Created attachment 293564 [details] [diff] [review]
Implementation for auto-expunge feature

New patch for auto-expunge excluding the fix for bug 399835.
Comment 7 Nikolay Shopik 2008-07-10 12:19:45 PDT
David could you take a look into patch for review?
Comment 8 Nikolay Shopik 2008-07-10 12:49:38 PDT
*** Bug 187487 has been marked as a duplicate of this bug. ***
Comment 9 Dale Wiggins 2008-07-14 10:11:03 PDT
Created attachment 329480 [details] [diff] [review]
Unbitrotted version of my last patch

A fresh patch for this bug fix. No substantive changes.
Comment 10 David :Bienvenu 2008-07-15 16:16:00 PDT
Can we combine the if checks? Maybe something like this:
+        if (GetShowDeletedMessages())
+        {
+          // If using the IMAP Delete model (mark messages as deleted) then expunge
+          // according to the expunge option.
+          if (gExpungeOption == kAutoExpungeAlways || ((gExpungeOption == kAutoExpungeOnThreshold) &&
+                   (m_flagState->GetNumberOfDeletedMessages() >= gExpungeThreshold)))
+            Expunge();
+        }
+        else if (m_flagState->GetNumberOfDeletedMessages() >= gExpungeThreshold)
+            Expunge();
+        }
Comment 11 David :Bienvenu 2008-07-15 16:17:09 PDT
I agree that doing it all in one big if statement would be hard to read, but the above seems like a reasonable compromise...
Comment 12 David :Bienvenu 2008-07-16 15:55:31 PDT
Comment on attachment 329480 [details] [diff] [review]
Unbitrotted version of my last patch

sr=bienvenu, if you combine the ifs as suggested...
Comment 13 Dale Wiggins 2008-07-17 08:42:51 PDT
Created attachment 330051 [details] [diff] [review]
New patch with combined if statements

continuing sr=bienvenu
Comment 14 Karsten Düsterloh 2008-07-21 14:47:50 PDT
Comment on attachment 330051 [details] [diff] [review]
New patch with combined if statements

>+        }
>+        else if (m_flagState->GetNumberOfDeletedMessages() >= gExpungeThreshold)
>+            Expunge();

For enhanced readability, if one if branch needs braces, all branches should have braces.
Furthermore, Expunge() should be indented two spaces less.

r=me with that.
Comment 15 Dale Wiggins 2008-07-22 08:10:13 PDT
Created attachment 330775 [details] [diff] [review]
Patch with brace and spacing changes

This patch uses the braces and spacing suggested in comment #14.
Comment 16 Dale Wiggins 2008-07-22 08:11:18 PDT
Oops. Meant to add continuing sr=bienvenu, r=mnyromyr.
Comment 17 Nikolay Shopik 2008-08-16 09:46:51 PDT
Dale, add flags to your patch otherwise you can stuck for reviewing forever.
Comment 18 Magnus Melin 2008-08-16 12:30:40 PDT
Doesn't need more reviews, only checkin.
Comment 19 Mark Banner (:standard8) 2008-08-18 00:56:38 PDT
This patch has bitrotted, please could you provide an updated patch against the latest comm-central code.
Comment 20 Dale Wiggins 2008-08-28 08:17:02 PDT
Created attachment 335894 [details] [diff] [review]
Unbitrotted version of my last patch

Continuing sr=bienvenu, r=mnyromyr.
Comment 21 Magnus Melin 2008-08-30 10:24:49 PDT
*** Bug 330772 has been marked as a duplicate of this bug. ***
Comment 22 Magnus Melin 2008-08-30 11:28:31 PDT
*** Bug 327374 has been marked as a duplicate of this bug. ***
Comment 23 Magnus Melin 2008-08-30 12:23:03 PDT
changeset:   232:3013cce23498
http://hg.mozilla.org/comm-central/rev/3013cce23498
->FIXED

Dale, remember to add checkin-needed when it's needed:)
Comment 24 neil@parkwaycc.co.uk 2008-08-30 13:53:25 PDT
(In reply to comment #2)
> 2. It doesn't work for IMAP accounts as far as I can tell.
Oh, it does, I've lost mail because of that "feature"...
Comment 25 Peter Lairo 2008-09-13 17:01:12 PDT
Did this patch cause a regression?

I use trunk builds + IMAP + Mark As Deleted

Since a few days the following features I heavily rely on are gone:

1. "Compact" icon grayed-out
2. "File / Compact Folders" is grayed-out
3. "Tools / Account Settings / Server Settings / When I delete a message:" 
   has no selectable setting

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080913070841 Shredder/3.0b1pre
Comment 26 Brett Leber 2008-09-18 06:27:18 PDT
Will there be a Thunderbird preference UI change for this?
Comment 27 Matthew Kanar 2009-07-02 08:31:41 PDT
This functionality does not seem to work as per recent builds of Shredder nor does it work in the last released beta, Thunderbird 3 beta 2.

mail.imap.expunge_option | 1
mail.imap.expunge_after_delete | true

Regarding the UI, it seems logical that the "When I delete" option "Remove it immediately" both mark and expunge the deleted message, including messages that are moved (copied/deleted in IMAP) to another folder, preferable without requiring modification to advanced configuration options.  If there is a demand, add the option "hide message when marked as deleted" to the UI.

The typical user doesn't know or want to deal with expunging.  Users want a message to be deleted when they delete a message, with the message most often moved to trash.  However, in the case of gmail which has such a large userbase that Thunderbird offers an account setup wizard specifically for gmail IMAP, the message should not be moved to trash which is why the "Remove it immediately" option is important to those users and should be the default when using the wizard.

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