Remove imap specific "Empty trash" confirmation dialog.

RESOLVED FIXED in Thunderbird 22.0

Status

MailNews Core
Backend
--
minor
RESOLVED FIXED
9 years ago
4 years ago

People

(Reporter: mats, Assigned: aceman)

Tracking

Trunk
Thunderbird 22.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

11.09 KB, patch
Magnus Melin
: review+
neil@parkwaycc.co.uk
: review+
standard8
: feedback+
Details | Diff | Splinter Review
(Reporter)

Description

9 years ago
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 ?
(Assignee)

Comment 2

4 years ago
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.
(Assignee)

Comment 3

4 years ago
Created attachment 718002 [details] [diff] [review]
patch
Assignee: nobody → acelists
Status: NEW → ASSIGNED
Attachment #718002 - Flags: review?(mkmelin+mozilla)
Attachment #718002 - Flags: feedback?(mbanner)
(Assignee)

Updated

4 years ago
Attachment #718002 - Flags: review?(neil)

Comment 4

4 years ago
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?
(Assignee)

Comment 5

4 years ago
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 6

4 years ago
Comment on attachment 718002 [details] [diff] [review]
patch

Fair enough, this seems reasonable.
Attachment #718002 - Flags: review?(neil) → review+
(Assignee)

Comment 7

4 years ago
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.
(Assignee)

Comment 8

4 years ago
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 9

4 years ago
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-
(Assignee)

Comment 10

4 years ago
Yes, I suggest to migrate both prefs.

Comment 11

4 years ago
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+
(Assignee)

Comment 12

4 years ago
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+
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/0baa9cf7f340
Status: ASSIGNED → RESOLVED
Last Resolved: 4 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.