500970 - Activity Manager: Removing account causes lots of "Folder Deleted" messages.

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Not quite sure step 1 is needed, but this is what I was doing when I came across this problem. I think this is blocking-tb3, since the user might be confused and panic, if he sees messages that all of his folders were deleted.

STR:


1. Let Thunderbird index your folders
2. Delete the account that is being indexed


Actual:

* For each folder, the activity manager says the folder was deleted.


Expected:

* One message that the account was deleted, or none at all.

Comment 1

[David Ascher (:davida)]

Marking as blocking until we can investigate how hard.

Comment 2

[David :Bienvenu]

Attachment: proposed fix

removing an account removes the server from the list of servers, so the call to FindServer will fail in that case, and succeed in other cases, so this should fix it.

Comment 3

[David :Bienvenu]

I'm not particularly inclined to add an activity manager item for this action at this point, so I just went with the removing of the deleted folder notifications.

Comment 4

[David :Bienvenu]

removing the [has l10n impact] status whiteboard - if we decide to add an activity mgr notification, then we should add that back to status whiteboard. I think there is some sort of notification when an account is removed; we needed that for the folder pane.

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 395464 [details] [diff] [review]
proposed fix

Looks reasonable to me, with some minor nits
 
>   folderDeleted : function(aFolder) {
>+    let acctMgr = Components.classes["@mozilla.org/messenger/account-manager;1"]
>+                    .getService(Components.interfaces.nsIMsgAccountManager);

Align .getService with .classes

>+    let server = aFolder.server;
>+    // if the account has been removed, we're going to ignore this notification.

Capitalize 

>+    try {
>+      acctMgr.FindServer(server.username, server.hostName, server.type);
>+    }
>+    catch(ex) {dump(ex); return;}
>+    

spaces on emtpy line ^^^
As the exception is expected, maybe it's not need to dump it

Comment 6

[David :Bienvenu]

fix checked in, with nits addressed.