Closed Bug 359284 Opened 13 years ago Closed 11 years ago

IMAP: Auto-expunge for IMAP mailboxes

Categories

(Thunderbird :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0a3

People

(Reporter: mdudziak, Assigned: dwiggins)

References

Details

Attachments

(1 file, 5 obsolete files)

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?).
Assignee: mozilla-bugs → dwiggins
Status: NEW → ASSIGNED
Thunderbird already have this, as a global option.
Preferences > Advanced > Network & Diskspace: Compact folders when it will save over [  ] KB
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. 
Changing to be under Thunderbird as the upcoming fix is implemented there.
Product: Penelope → Thunderbird
Target Milestone: --- → Thunderbird 3
Version: 0.1 → Trunk
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.
Attachment #285016 - Flags: superreview?(bienvenu)
Attachment #285016 - Flags: review?(mscott)
QA Contact: general → general
Jeff/Dale, can you post a new patch excluding the fix for bug 399835.
New patch for auto-expunge excluding the fix for bug 399835.
Attachment #285016 - Attachment is obsolete: true
Attachment #293564 - Flags: superreview?(bienvenu)
Attachment #293564 - Flags: review?(mscott)
Attachment #285016 - Flags: superreview?(bienvenu)
Attachment #285016 - Flags: review?(mscott)
David could you take a look into patch for review?
Duplicate of this bug: 187487
A fresh patch for this bug fix. No substantive changes.
Attachment #293564 - Attachment is obsolete: true
Attachment #329480 - Flags: superreview?(bienvenu)
Attachment #329480 - Flags: review?(mnyromyr)
Attachment #293564 - Flags: superreview?(bienvenu)
Attachment #293564 - Flags: review?(mscott)
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();
+        }
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 on attachment 329480 [details] [diff] [review]
Unbitrotted version of my last patch

sr=bienvenu, if you combine the ifs as suggested...
Attachment #329480 - Flags: superreview?(bienvenu) → superreview+
continuing sr=bienvenu
Attachment #329480 - Attachment is obsolete: true
Attachment #330051 - Flags: review?(mnyromyr)
Attachment #329480 - Flags: review?(mnyromyr)
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.
Attachment #330051 - Flags: review?(mnyromyr) → review+
This patch uses the braces and spacing suggested in comment #14.
Attachment #330051 - Attachment is obsolete: true
Oops. Meant to add continuing sr=bienvenu, r=mnyromyr.
Dale, add flags to your patch otherwise you can stuck for reviewing forever.
Doesn't need more reviews, only checkin.
Keywords: checkin-needed
This patch has bitrotted, please could you provide an updated patch against the latest comm-central code.
Keywords: checkin-needed
Continuing sr=bienvenu, r=mnyromyr.
Attachment #330775 - Attachment is obsolete: true
Duplicate of this bug: 330772
Duplicate of this bug: 327374
changeset:   232:3013cce23498
http://hg.mozilla.org/comm-central/rev/3013cce23498
->FIXED

Dale, remember to add checkin-needed when it's needed:)
Target Milestone: Thunderbird 3 → Thunderbird 3.0b1
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
(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"...
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
Will there be a Thunderbird preference UI change for this?
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.
You need to log in before you can comment on or make changes to this bug.