Closed Bug 448624 Opened 12 years ago Closed 7 years ago

Remove imap specific "Empty trash" confirmation dialog.

Categories

(MailNews Core :: Backend, defect, minor)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 22.0

People

(Reporter: mats, Assigned: aceman)

References

Details

Attachments

(1 file)

11.09 KB, patch
mkmelin
: review+
neil
: review+
standard8
: feedback+
Details | Diff | Splinter Review
Followup from bug 179891 comment 9:

David Bienvenu   2007-02-01 14:26:28 PDT

There's some overlap with this code:

http://lxr.mozilla.org/mailnews/source/mailnews/imap/src/nsImapMailFolder.cpp#1406

We might want to just remove that imap code, and the associated pref, once this
goes in.
Product: Core → MailNews Core
Severity: normal → minor
i guess this is talking about mail.imap.confirm_emptyTrashFolderDeletion and the code at http://mxr.mozilla.org/comm-central/source/mailnews/imap/src/nsImapMailFolder.cpp#1474

obsoleted by mail.emptyTrash.dontAskAgain ?
We actually have also mailnews.emptyTrash.dontAskAgain, see https://bugzilla.mozilla.org/show_bug.cgi?id=179891#c19 . Maybe we could clean it up here.
Attached patch patchSplinter Review
Assignee: nobody → acelists
Status: NEW → ASSIGNED
Attachment #718002 - Flags: review?(mkmelin+mozilla)
Attachment #718002 - Flags: feedback?(mbanner)
Attachment #718002 - Flags: review?(neil)
I don't use IMAP Trash, so I'm not sure what to expect when I empty it... are you saying that both the UI and backend try to prompt in this case?
I have not tried that yet, but that is quite possible as there are 2 places that could prompt. Each of them using a different pref. But if both are set to prompt, they could. The difference is, the IMAP C++ code only prompts when there are any subfolders in Trash (at least that is how I read it), so when testing you need to produce such a testcase.
Comment on attachment 718002 [details] [diff] [review]
patch

Fair enough, this seems reasonable.
Attachment #718002 - Flags: review?(neil) → review+
Thanks for the review, I will still try to test it on some IMAP server, whether we really got 2 prompts before.

I still need the review from mkmelin or standard8 on the TB pref changes.
Ok, I can confirm, if you set the mail.emptyTrash.dontAskAgain pref to non-default value of true, you get 2 prompts about the same thing. The hg blame does not show how that code got in (revision is 0), but when bienvenu says it can go, it is probably right.
Comment on attachment 718002 [details] [diff] [review]
patch

Review of attachment 718002 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/base/content/folderPane.js
@@ +2300,3 @@
>      let showPrompt = true;
>      try {
> +      showPrompt = !Services.prefs.getBoolPref("mailnews." + aCommand + ".dontAskAgain");

This would break mail.emptyJunk.dontAskAgain
Attachment #718002 - Flags: review?(mkmelin+mozilla) → review-
Yes, I suggest to migrate both prefs.
Comment on attachment 718002 [details] [diff] [review]
patch

Review of attachment 718002 [details] [diff] [review]:
-----------------------------------------------------------------

Ah yes, sorry, i misread that. r=mkmelin
Attachment #718002 - Flags: review- → review+
It is a bit confusing as I explicitly said it only in bug 179891 comment 19.

So the one downside of the pref change, that users get reprompted at next delete. Once they check the checkbox they will be using the new pref from now on.
Depends on: 179891
Attachment #718002 - Flags: feedback?(mbanner) → feedback+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/0baa9cf7f340
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 22.0
You need to log in before you can comment on or make changes to this bug.