359284 - IMAP: Auto-expunge for IMAP mailboxes

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Matt Dudziak]

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 [:mkmelin]]

Thunderbird already have this, as a global option.
Preferences > Advanced > Network & Diskspace: Compact folders when it will save over [  ] KB

Comment 2

[Eric Moore]

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]

Changing to be under Thunderbird as the upcoming fix is implemented there.

Comment 4

[Jeff Beckley]

Attachment: 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 <[e-mail]>, so he will remain assigned to it.

Comment 5

[Magnus Melin [:mkmelin]]

Jeff/Dale, can you post a new patch excluding the fix for bug 399835.

Comment 6

[Dale Wiggins]

Attachment: Implementation for auto-expunge feature

New patch for auto-expunge excluding the fix for bug 399835.

Comment 7

[Nikolay Shopik]

David could you take a look into patch for review?

Comment 9

[Dale Wiggins]

Attachment: Unbitrotted version of my last patch

A fresh patch for this bug fix. No substantive changes.

Comment 10

[David :Bienvenu]

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]

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]

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]

Attachment: New patch with combined if statements

continuing sr=bienvenu

Comment 14

[Karsten Düsterloh]

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]

Attachment: Patch with brace and spacing changes

This patch uses the braces and spacing suggested in comment #14.

Comment 16

[Dale Wiggins]

Oops. Meant to add continuing sr=bienvenu, r=mnyromyr.

Comment 17

[Nikolay Shopik]

Dale, add flags to your patch otherwise you can stuck for reviewing forever.

Comment 18

[Magnus Melin [:mkmelin]]

Doesn't need more reviews, only checkin.

Comment 19

[Mark Banner (:standard8)]

This patch has bitrotted, please could you provide an updated patch against the latest comm-central code.

Comment 20

[Dale Wiggins]

Attachment: Unbitrotted version of my last patch

Continuing sr=bienvenu, r=mnyromyr.

Comment 23

[Magnus Melin [:mkmelin]]

changeset:   232:3013cce23498
http://hg.mozilla.org/comm-central/rev/3013cce23498
->FIXED

Dale, remember to add checkin-needed when it's needed:)

Comment 24

[[e-mail]]

(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]

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]

Will there be a Thunderbird preference UI change for this?

Comment 27

[Matthew Kanar]

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.

Comment 28

[Narcis Garcia]

Mozilla Thunderbird 60.9.0
mail.imap.expunge_option = 1
mail.imap.expunge_after_delete = true

Moving messages from account A to account B.
I still find messatges in IMAP folders with other mail clients, with only hidden/deleted mark.
Only going to File menu -> Quit (and waiting a slow M.Thunderbird finish) I reach those messages to be expunged.