Closed Bug 198936 Opened 18 years ago Closed 15 years ago

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

Categories

(MailNews Core :: Backend, enhancement)

x86
All
enhancement
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.8.1

People

(Reporter: Benjamin_Schulz, Assigned: jens.b)

References

Details

(Keywords: fixed1.8.1, late-l10n)

Attachments

(2 files, 3 obsolete files)

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
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Dialog "Do you want to compress your mail...." has no checkbox that it won't appear again → "Do you want to compress... folder..." dialog has no "don't ask me again" checkbox
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.
Updating summary to use correct terminology.
Summary: "Do you want to compress... folder..." dialog has no "don't ask me again" checkbox → "Do you wish to compact..." dialog has no "don't ask me again" checkbox
Product: Browser → Seamonkey
Assignee: sspitzer → mail
Blocks: 321641
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.)
Assignee: mail → nobody
Component: MailNews: Main Mail Window → MailNews: Backend
Product: Mozilla Application Suite → Core
QA Contact: esther
Assignee: nobody → jens.b
Target Milestone: --- → mozilla1.8.1
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...
Scott, could you r/sr this small change? It would be a huge usability win...
Attachment #226713 - Flags: superreview?(mscott)
Attachment #226713 - Flags: review?(mscott)
(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.
Flags: blocking-thunderbird2?
Summary: "Do you wish to compact..." dialog has no "don't ask me again" checkbox → [patch] "Do you wish to compact..." dialog has no "don't ask me again" checkbox
Blocks: 115499
(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.
(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.
Changed the dialog title to "Compact Folders" per bug 116215 comment 8 by beltzner.
Attachment #226713 - Attachment is obsolete: true
Attachment #226965 - Flags: superreview?(mscott)
Attachment #226965 - Flags: review?(mscott)
Attachment #226713 - Flags: superreview?(mscott)
Attachment #226713 - Flags: review?(mscott)
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...
(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 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)
+
Attachment #226965 - Flags: review?(mscott) → review?(bienvenu)
Fixed the brace.
Attachment #226965 - Attachment is obsolete: true
Attachment #227622 - Flags: superreview?(mscott)
Attachment #227622 - Flags: review?(bienvenu)
Attachment #226965 - Flags: superreview?(mscott)
Attachment #226965 - Flags: review?(bienvenu)
Attachment #227622 - Flags: review?(bienvenu) → review+
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!
Attachment #227622 - Flags: superreview?(mscott) → superreview+
Carrying over r=bienvenu, sr=mscott.

Renamed the pref to mail.purge.ask and fixed Scott's nits.
Attachment #227622 - Attachment is obsolete: true
Attachment #227798 - Flags: superreview+
Attachment #227798 - Flags: review+
Whiteboard: [checkin needed]
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
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [checkin needed]
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.
Attachment #227798 - Flags: approval1.8.1?
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.
Adding a late-l10n keyword here.
Keywords: late-l10n
(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.
Jens, do you need someone to check this in for you (i.e. me)?
MScott can you do 1.8.1 approval as appropriate?
(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.
Flags: blocking-thunderbird2? → blocking-thunderbird2+
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.
Attachment #227798 - Flags: approval-thunderbird2+
I landed this on the 1.8.1 branch. As always, thanks for the patch Jens! We appreciate it.
Keywords: fixed1.8.1
*** Bug 321641 has been marked as a duplicate of this bug. ***
(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?
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. 
(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?


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".
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.
> 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
(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
(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? 


that's the idea, yes.
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.
(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.
Status: RESOLVED → VERIFIED
Product: Core → MailNews Core
See Also: → 716412
You need to log in before you can comment on or make changes to this bug.