Closed Bug 1664572 Opened 4 years ago Closed 3 years ago

nsImapIncomingServer::DeleteNonVerifiedFolders() doesn't seem to do what the name implies.

Categories

(MailNews Core :: Networking: IMAP, task)

Tracking

(thunderbird_esr78 wontfix, thunderbird86 wontfix)

RESOLVED FIXED
87 Branch
Tracking Status
thunderbird_esr78 --- wontfix
thunderbird86 --- wontfix

People

(Reporter: benc, Assigned: benc)

Details

Attachments

(1 file, 1 obsolete file)

nsImapIncomingServer::DeleteNonVerifiedFolders() seems very odd. It does a lot of checking, then unconditionally deletes the folder.
Is it doing as intended? Is it just badly named? Are we relying on some non-obvious side effects in the flag checks?
Definitely some scope for clarification/cleanup.

Component: General → Networking: IMAP
Product: Thunderbird → MailNews Core

(I rediscovered this bug while working on Bug 1682941, and did a little more digging)

The function was only called from one place, so this patch ditches it entirely and just inlines the (tiny) relevant section.

Assignee: nobody → benc
Attachment #9202768 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9202768 [details] [diff] [review]
1664572-remove-DeleteNonVerifiedFolders-1.patch

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

::: mailnews/imap/src/nsImapIncomingServer.cpp
@@ +1478,5 @@
>        }
> +    } else {
> +      nsCOMPtr<nsIMsgFolder> parent;
> +      rv = currentFolder->GetParent(getter_AddRefs(parent));
> +      if (NS_SUCCEEDED(rv) && parent) {

I think we for this we skip the rv and just check parent like we do elsewhere

@@ -1532,5 @@
> -  nsCOMPtr<nsIPrefBranch> prefBranch =
> -      do_GetService(NS_PREFSERVICE_CONTRACTID, &rv);
> -  if (NS_SUCCEEDED(rv))
> -    prefBranch->GetBoolPref("mail.imap.auto_unsubscribe_from_noselect_folders",
> -                            &autoUnsubscribeFromNoSelectFolders);

Should remove the pref from mailnews.js as well. Apparently unused on many levels.
Attachment #9202768 - Flags: review?(mkmelin+mozilla) → review+
Status: NEW → ASSIGNED
Target Milestone: --- → 87 Branch

Thanks Magnus, here's the tweaked patch. It removes the pref from mailnews.js and cleans up that parent check.

Attachment #9202768 - Attachment is obsolete: true
Attachment #9202938 - Flags: review+

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/fd9b0dbd830d
Remove cruft in nsImapIncomingServer::DeleteNonVerifiedFolders(). r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: