Last Comment Bug 448624 - Remove imap specific "Empty trash" confirmation dialog.
: Remove imap specific "Empty trash" confirmation dialog.
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Backend (show other bugs)
: Trunk
: All All
: -- minor (vote)
: Thunderbird 22.0
Assigned To: :aceman
:
Mentors:
Depends on: 179891
Blocks:
  Show dependency treegraph
 
Reported: 2008-07-30 21:00 PDT by Mats Palmgren (vacation)
Modified: 2013-03-26 04:28 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (11.09 KB, patch)
2013-02-25 11:47 PST, :aceman
mkmelin+mozilla: review+
neil: review+
standard8: feedback+
Details | Diff | Splinter Review

Description Mats Palmgren (vacation) 2008-07-30 21:00:58 PDT
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.
Comment 1 Wayne Mery (:wsmwk, NI for questions) 2013-02-25 10:16:35 PST
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 ?
Comment 2 :aceman 2013-02-25 11:10:35 PST
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.
Comment 3 :aceman 2013-02-25 11:47:14 PST
Created attachment 718002 [details] [diff] [review]
patch
Comment 4 neil@parkwaycc.co.uk 2013-02-26 05:47:36 PST
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?
Comment 5 :aceman 2013-02-26 05:53:21 PST
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 neil@parkwaycc.co.uk 2013-02-26 05:53:47 PST
Comment on attachment 718002 [details] [diff] [review]
patch

Fair enough, this seems reasonable.
Comment 7 :aceman 2013-02-26 06:22:00 PST
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.
Comment 8 :aceman 2013-02-26 11:49:10 PST
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 Magnus Melin 2013-02-26 13:11:58 PST
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
Comment 10 :aceman 2013-02-26 13:42:12 PST
Yes, I suggest to migrate both prefs.
Comment 11 Magnus Melin 2013-02-26 23:08:29 PST
Comment on attachment 718002 [details] [diff] [review]
patch

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

Ah yes, sorry, i misread that. r=mkmelin
Comment 12 :aceman 2013-02-26 23:43:10 PST
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.
Comment 13 Mark Banner (:standard8) 2013-03-26 04:28:49 PDT
https://hg.mozilla.org/comm-central/rev/0baa9cf7f340

Note You need to log in before you can comment on or make changes to this bug.