198936 - [patch] "Do you wish to compact..." dialog has no "don't ask me again" checkbox

Status: VERIFIED FIXED

(MailNews Core :: Backend, enhancement)

Description

[Benjamin Schulz]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3) Gecko/20030312
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.3) Gecko/20030312

When you have got large E-mail folders, sometimes a dialog is appearing saying:
g "Do you want to compress all your mail folders to save disk space....".#
This dialog has no check box that the user can check to secure that the dialog
won't be appearing for the next time.
So, every time I open Mozilla a get that Dialog. 
That is enevering.

Reproducible: Always

Steps to Reproduce:
1.have a LARGE E-mail folder
2.run mozilla
Actual Results:  
a Dialog box appears that I don't want to see every time.

Expected Results:  
the Dialog box should have a check box where you can state that Mozilla should
not warn you again because of this reason.

none

Comment 1

[Tony Karavidas]

It bugs me too. There is a parameter: Edit:Preferences:Offline&DiskSpace and if
you crank up the number it won't ask you quite as often.

Comment 2

[Mike Cowperthwaite]

Updating summary to use correct terminology.

Comment 3

[Andreas Morawietz]

Attachment: could be something like that...

Comment 4

[Jens Bannmann]

This is not a suite-specific bug, as the dialog is not triggered from the front-end (Thunderbird/SeaMonkey), but from shared backend code. Not sure about the component, feel free to move to MailNews: Database if more appropriate.

For the record, the code showing the dialog is at http://lxr.mozilla.org/mozilla/source/mailnews/base/util/nsMsgDBFolder.cpp#1596 (and can easily be found by LXRing for "autoCompactAllFolders"). 

This looks quite easy to do, and I might even get around to doing it myself. (If so, I'll reassign this bug, until then feel free to take this yourself.)

Comment 5

[David :Bienvenu]

I think this is a reasonable thing to do. I'd also like to make it so we don't compact folders at the worst possible times, e.g., startup, and when the user selects a folder. I'd prefer that we do it when somewhat idle. The problem with doing it at startup is that if you get a bunch of new mail, and some of it is filtered to other folders, and we compact those other folders while the new mail is arriving, we don't filter that mail...

Comment 6

[Jens Bannmann]

Attachment: Add a "Do this automatically from now on" checkbox

Scott, could you r/sr this small change? It would be a huge usability win...

Comment 7

[Jens Bannmann]

(In reply to comment #5)
> I think this is a reasonable thing to do. I'd also like to make it so we don't
> compact folders at the worst possible times, e.g., startup, and when the user
> selects a folder. I'd prefer that we do it when somewhat idle.

ACK. We should take that issue to bug 321641 which could serve as a tracking bug.

IMO, a checkbox alone would already improve the situation a bit. The auto-compact feature is off by default, so users who activate it and subsequently tick the checkbox should know what they're doing.

Comment 8

[Lachlan Hunt]

(In reply to comment #7)
> The auto-compact feature is off by default...

Compacting folders should be *on* by default.  Leaving it off can have disastrous results over a prolongued period for users that don't know that they need to compact folders (see bug 324217).  The resulting performance issue when the file size builds up so much is far worse than any performance issue occuring when the folders are being compacted; especially if TB is smart enough to only compact during idle time and when the file sizes are still relatively small.

What is the reason for the dialogue anyway?  The vast majority of users should never need to prevent compacting folders and they have no reason to be bothered by necessary system maintenance tasks.  I'd very much like to see compacting folders implemented in a way similar to that which I described in bug 324217.

Comment 9

[Jens Bannmann]

(In reply to comment #8)
> What is the reason for the dialogue anyway?

IMO, it needs to be there as a warning to the user who just activated the feature. We need to announce that a potentially time-consuming process is about to start. As David wrote in comment 5, the process can get in the way of mail downloading, particularly but not limited to startup. The reason basically is that Thunderbird is *not* "smart enough to to only compact during idle time", at least not yet. 

Activating this only when idle is really the best option, and we probably could get rid of the dialog once we do that - however, that requires more work and will most likely not happen in time for Thunderbird 2. Your ideas in bug 324217 regarding purging mails by their date sound interesting, and we should discuss them in the more general bug 321641.

Comment 10

[Jens Bannmann]

Attachment: Add a "Do this automatically from now on" checkbox, v2

Changed the dialog title to "Compact Folders" per bug 116215 comment 8 by beltzner.

Comment 11

[Karsten Düsterloh]

Comment on attachment 226965 [details] [diff] [review]
Add a "Do this automatically from now on" checkbox, v2

Can you include the respective string changes in SeaMonkey's messenger.properties for  as well, please? Since you're changing shared files, this will break us...

Comment 12

[Jens Bannmann]

(In reply to comment #11)
> Can you include the respective string changes in SeaMonkey's
> messenger.properties for  as well, please? Since you're changing shared files,
> this will break us...

Sure, they will be in the next patch.

Comment 13

[David :Bienvenu]

Comment on attachment 226965 [details] [diff] [review]
Add a "Do this automatically from now on" checkbox, v2

I'm OK with this, except for the K&R brace that snuck in :-)

+             if (buttonPressed == 0) {
+               okToCompact = PR_TRUE;
+
+               if (checkValue)
+

Comment 14

[Jens Bannmann]

Attachment: Add a "Do this automatically from now on" checkbox, v3

Fixed the brace.

Comment 15

[Scott MacGregor]

Comment on attachment 227622 [details] [diff] [review]
Add a "Do this automatically from now on" checkbox, v3

+pref("mail.askBeforePurge",                true);
I'd probably call the pref:

mail.purge.ask or mail.purge.prompt.ask

that's more inline with the style we've been doing for new prefs, but that's just me. 

+             if (buttonPressed == 0)
+             {

can be

if (!buttonPressed)

+           else
+           {
+             okToCompact = PR_TRUE;
+           }

we can ditch the braces around the single statement in the else clause.

No need to re-request a review. Nice patch!

Comment 16

[Jens Bannmann]

Attachment: Add a "Do this automatically from now on" checkbox, v4

Carrying over r=bienvenu, sr=mscott.

Renamed the pref to mail.purge.ask and fixed Scott's nits.

Comment 17

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

mozilla/mail/locales/en-US/chrome/messenger/messenger.properties 	1.26
mozilla/mailnews/base/resources/locale/en-US/messenger.properties 	1.132
mozilla/mailnews/mailnews.js 	3.267
mozilla/mailnews/base/util/nsMsgDBFolder.cpp 	1.279

Comment 18

[Jens Bannmann]

Comment on attachment 227798 [details] [diff] [review]
Add a "Do this automatically from now on" checkbox, v4

This is a low-risk patch, it only adds a pref to skip a message box. Functionality is not touched. It would be a great usability win to have this in Thunderbird 2.

Comment 19

[Mike Cowperthwaite]

I would quibble a bit with the low-risk aspect of it.  There are known problems of mail processing not working correctly, especially filtering but also reports of lost mail, while compaction is in process.  This is why compaction hasn't been simply automatic up to this point.

Comment 20

[Axel Hecht [:Pike]]

Adding a late-l10n keyword here.

Comment 21

[Worcester12345]

(In reply to comment #8)
> (In reply to comment #7)
> > The auto-compact feature is off by default...
> 
> Compacting folders should be *on* by default.  Leaving it off can have
> disastrous results over a prolongued period for users that don't know that they
> need to compact folders (see bug 324217).  The resulting performance issue when
> the file size builds up so much is far worse than any performance issue
> occuring when the folders are being compacted; especially if TB is smart enough
> to only compact during idle time and when the file sizes are still relatively
> small.
> 
> What is the reason for the dialogue anyway?  The vast majority of users should
> never need to prevent compacting folders and they have no reason to be bothered
> by necessary system maintenance tasks.  I'd very much like to see compacting
> folders implemented in a way similar to that which I described in bug 324217.

That entire bug has been marked a duplicate to another bug which is a WONTFIX. Doesn't sound like they want this at all.

Can't say as I blame them at this point, as it could be a data loss issue. They need to figure out a way to run filters after compacting files or do background compacting when idle.

Comment 22

[Scott MacGregor]

Jens, do you need someone to check this in for you (i.e. me)?

Comment 23

[Mike Schroepfer]

MScott can you do 1.8.1 approval as appropriate?

Comment 24

[Jens Bannmann]

(In reply to comment #22)
> Jens, do you need someone to check this in for you (i.e. me)?

Yeah, if you could approve it for 1.8.1, feel free to also check it in on branch.

Comment 25

[Scott MacGregor]

Comment on attachment 227798 [details] [diff] [review]
Add a "Do this automatically from now on" checkbox, v4

approving for thunderbird 2. I don't have the ability to adjust 1.8.1 flags but we use the approval-thunderbird2 flag instead here anyway.

Comment 26

[Scott MacGregor]

I landed this on the 1.8.1 branch. As always, thanks for the patch Jens! We appreciate it.

Comment 27

[u88484]

*** Bug 321641 has been marked as a duplicate of this bug. ***

Comment 28

[Ekhart]

(In reply to comment #19)
> I would quibble a bit with the low-risk aspect of it.  There are known problems
> of mail processing not working correctly, especially filtering but also reports
> of lost mail, while compaction is in process.  This is why compaction hasn't
> been simply automatic up to this point.

So what does this now do in 2.0? Does TB now compact while it's online and downloading mail?! Wouldn't it be easy to have it compact only on exit, while keeping TB visible so that users don't shut down their computer during the process?

Comment 29

[David :Bienvenu]

compaction and downloading new mail both only proceed if they can get a lock on the folder, so it should be OK. 

Doing things on exit is not easy in our architecture, though I personally would prefer that we did do it on exit. Others disagree, of course. 

Comment 30

[Ekhart]

(In reply to comment #29)

>>This is why compaction hasn't
>> been simply automatic up to this point.
>
> compaction and downloading new mail both only proceed if they can get a lock on
> the folder, so it should be OK. 

So apparently compacting is now "simply automatic"?

Is this locking new? Is it newer than your comment #19? Would you now no longer "quibble", and would you say it is now very low or zero risk?

Comment 31

[Ekhart]

When set to other numbers, TB always reverts to 1000 kB (misspelled KB) after restarting; is that a known bug?

How does one get TB to stop compacting automatically if one has put a check mark in "Do this automatically from now on"? If one puts a check mark in there, one never gets that dialog box again, and there is no relevant setting in "network & disk space".

Comment 32

[David :Bienvenu]

No, the locking is not new. And I didn't make comment #19.

You could use the config editor to toggle "mail.prompt_purge_threshhold", I think.

Comment 33

[Tuukka Tolvanen (sp3000)]

> When set to other numbers, TB always reverts to 1000 kB (misspelled KB) after

worksforme on trunk, look for / file another bug

> How does one get TB to stop compacting automatically if one has put a check

tools -> options -> advanced -> general -> configeditor -> mail.purge.ask; look for / file another bug on the missing gui option

Comment 34

[Ekhart]

(In reply to comment #32)
> No, the locking is not new. And I didn't make comment #19.
> 
> You could use the config editor to toggle "mail.prompt_purge_threshhold", I
> think.

(Sorry about confusing you with the author of comment #19 and about making it look as if i were quoting myself (missing additional quote level).)

(TB kept reverting to 1000 kB because i'd forgotten about user.js)

To answer my own question about getting the dialog box back:
mail.purge.ask 

This should be made part of the UI

Comment 35

[Ekhart]

(In reply to comment #29)
> compaction and downloading new mail both only proceed if they can get a lock on
> the folder, so it should be OK. 

Do i understand correctly that when TB is compacting the inbox it first locks this, then makes a new compacted version of the inbox called "nstmp", then deletes the inbox, then renames the nstmp folder as "inbox", and only then lets new mail be downloaded into the inbox? 

Comment 36

[David :Bienvenu]

that's the idea, yes.

Comment 37

[John David Galt]

I would like to see this reopened, perhaps with Component = 'Mail Window Front End'.  A preference that allows the user to turn off automatic compaction is nice, but the bug actually asks for a way to permanently turn off the "Compact folders now?" prompt itself (an annoyance which in my experience is absolutely useless, since it only ever appears during or right after downloading, before I've had a chance to delete all the spams the filter didn't catch).

This is an example of the general problem of interruption by needless modal dialogs discussed in bug 255233.

Comment 38

[Jens Bannmann]

(In reply to comment #37)
> I would like to see this reopened, perhaps with Component = 'Mail Window Front
> End'.  A preference that allows the user to turn off automatic compaction is
> nice, but the bug actually asks for a way to permanently turn off the "Compact
> folders now?" prompt itself

John, I did not add a preference to turn off automatic compaction. I _did_ add a "Don't ask me again and do this automatically from now on" checkbox to the dialog, i.e. "a way to permanently turn off the 'Compact folders now?' prompt itself". Thus, this bug is truly fixed and I won't reopen it. Regarding the issue of compaction being started at the wrong time: you should find another bug for that, or file one, and mention the bug number here.

Oh, btw: verified using TB 2.0.0.6.