62322 - Unchecking the 'Confirm when moving folders to trash' checkbox not working

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect, P3)

Description

[Dipu Joseph]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win95; en-US; m18) Gecko/20001108 Netscape6/6.0
BuildID:    2000111320

Even after checking the checkbox 'Confirm when moving folders to trash',
the confirmation dialog box comes up on trying to delete a selected folder.

Reproducible: Always
Steps to Reproduce:
1.Select Edit->Preferences menu item from browser; in the Preferences
window, select Mail and Newsgroups.
2.Uncheck the checkbox 'Confirm when moving folders to trash' and press OK in
Preferences window.
3.Select any folder under a mail account; select Edit->Delete menu item.

Actual Results:  The confirmation dialog box comes up.


Expected Results:  The selected folder should be removed without showing
confirmation dialog box.

Comment 1

[Fabian Guisset]

Worksforme build 2000120420 win32. Reporter can you reproduce this with a latest 
build? (http://ftp.mozilla/org/pub/mozilla/nightly and pick the newest 
directory). Thank you,
Fabian.

Comment 2

[HJ]

Confirmed, WFM build:2000120504 WinNT Sp6b

Friendly HJ

Comment 3

[scottputterman]

marking WFM since we have two people agreeing in this report.

Comment 4

[Sheela Ravindran]

The reporter is not clear in stating the problem.  The first two lines
describing the problem should have been " Even after unchecking the checkbox......"
Commercial build 2000122806 on win98,mac,linux has this problem.  I am not sure
if this was fixed earlier.  Now that I see it I am reopening and updating the
platforms to all(win32,mac,linux)

steps to reproduce:
Open mail
select any account
Go to edit/preferences
select Mail and Newsgroups
Uncheck the checkbox - confirm when moving folders to the Trash
Click OK
Go back to any account and delete a folder from edit/delete folder

Expected Results:  There should not be any confirmation dlg appearing again and
the message should just be deleted.
Actual Results:  The confirmation dlg pops up again. So setting the preference
does not matter since the dlg appears anyways.

Comment 5

[scottputterman]

since sheela's seeing this I'm going to confirm it and reassign to naving.

Comment 6

[scottputterman]

nominating for nsbeta1

Comment 7

[scottputterman]

marking nsbeta1-

Comment 8

[John Levon]

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

Comment 9

[Mike Kaply [:mkaply]]

Attachment: Fix for problem

Fix for problem with POP and Local Mail folders. I haven't looked at IMAP.

This required a little bit of code restructuring which is why the diff is so
big.

Comment 10

[Mike Kaply [:mkaply]]

Attachment: Fix for IMAP side

This makes IMAP use the checkbox as well.

Comment 11

[Navin Gupta]

Comment on attachment 87241 [details] [diff] [review]
Fix for problem

r=naving, Can you use NS_PREF_CONTRACTID ? rest looks ok

Comment 12

[Navin Gupta]

Comment on attachment 87242 [details] [diff] [review]
Fix for IMAP side

same thing here, r=naving

Comment 13

[Mike Kaply [:mkaply]]

Attachment: use proper prefID and combine patches

Comment 14

[Navin Gupta]

Comment on attachment 87285 [details] [diff] [review]
use proper prefID and combine patches

r=naving

Comment 15

[David :Bienvenu]

could you do a diff -uw to get rid of the whitespace changes? Also, please use
the prevailing braces style, which is not K&R. It's

if (a)
{
  ...
}

I'll look at the diff when there aren't whitespace changes in the diff.

Comment 16

[Mike Kaply [:mkaply]]

Attachment: Patch per bienvenu's instructions

Updated patch.

Comment 17

[David :Bienvenu]

you don't need the extra boolean here - you can just GetBoolPref directly into
confirmDeletion. There are two instances of this.
+            PRBool confirmDeletion = PR_TRUE;
+            nsCOMPtr<nsIPref> prefs(do_GetService(NS_PREF_CONTRACTID, &rv));
+            if (NS_SUCCEEDED(rv))
+            {
+                PRBool prefBool;
+                rv = prefs->GetBoolPref("mailnews.confirm.moveFoldersToTrash",
&prefBool);
+                if (NS_SUCCEEDED(rv))
+                    confirmDeletion = prefBool;
+            }
+

you don't need braces here since both the if and else clauses just have single
statement (our style is that the if and else clauses should match)

+            else
+            {
+                confirmed = PR_TRUE;
+            }
+

Comment 18

[Mike Kaply [:mkaply]]

I didn't think there was a guarantee that the input variable wouldn't be 
touched if the preference didn't exists?

Comment 19

[David :Bienvenu]

I think there is - we assume that all over the code - we init the variable to
what we want the default value to be, then get the pref.

Comment 20

[Mike Kaply [:mkaply]]

Attachment: New patch

Thank you for the information.

This patch addresses all your issues.

Comment 21

[David :Bienvenu]

Comment on attachment 88991 [details] [diff] [review]
New patch

sr=bienvenu - I doubt, however, that there will be much hope of getting this in
7.0

Comment 22

[Mike Kaply [:mkaply]]

Fix checked into trunk.

ADT, this is lowhanging fruit for a function that simply never worked.

Comment 23

[scottputterman]

adding adt1.0.1-.  Let's do this in the next release.

Comment 24

[Sheela Ravindran]

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

Comment 25

[Sheela Ravindran]

verified
2002-009-11 win98, mac os 10.1, linux rh6.2

No prompt when deleting folder with pref unchecked. Works as it should now.