450712 - "INBOX" is always displayed as selected folder name of "When I delete a message/Move it to this folder"

Status: RESOLVED FIXED

(Thunderbird :: Account Manager, defect)

Description

[WADA:World Anti-bad-Duping Agency]

Checked with Tb Trunk 2008/8/14 build on MS Win-XP SP3.
> Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/20080814031842 Shredder/3.0b1pre

"INBOX" is always displayed as selected folder name of "When I delete a message / Move it to this folder".
 (1) Choose "When I delete a message/Move it to this folder",
     and select "Trash" folder at selection list.
     => "Trash" is set in mail.server.serverX.trash_folder_name as expected
 (2) Close "Server Settings" panel
 (3) Open "Server Settings" panel => INBOX is always displayed as chosen folder
mail.server.serverX.trash_folder_name is not modified until other folder is intentionally selected via selection list. So UI only issue.

Comment 1

[Magnus Melin [:mkmelin]]

See it on linux too. I think it's basically the same as bug 437056 (~ same code), at least if trash is a subfolder. I see it without it being a subfolder also though.

Comment 2

[WADA:World Anti-bad-Duping Agency]

I think good fiend of bug 437056 is Bug 427311, instead of this bug.

Comment 4

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

This fixes this regression, but not bug 427311 (which never worked).

It seems there were several issues here
 - didn't update the widget after selecting => subfolder couldn't be selected
 - in the selectFolder method the isAncestorOf stuff looks bogus to me, and that was causing the inbox to always get reselected after reopening preferences

Comment 5

[Magnus Melin [:mkmelin]]

Joey: can you take a look at the fix also?

Comment 6

[David :Bienvenu]

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

this looks ok to me, Thx, Magnus. I'll let Neil look at it, though. 

one nit - I don't think you need the temp var here:

+  // Set the value to be persisted.
+  var hiddenLabel = document.getElementById("imap.trashFolderName");
+  hiddenLabel.setAttribute("value", folder.name);

Comment 7

[[e-mail]]

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

> function folderPickerChange(aEvent)
> {
>+  var folder = aEvent.target._folder;
>+
>+  // Set the value to be persisted.
>+  var hiddenLabel = document.getElementById("imap.trashFolderName");
>+  hiddenLabel.setAttribute("value", folder.name);
>+
>+  // Update the widget to show/do correct things even for subfolders.
>+  var trashFolderPicker = document.getElementById("msgTrashFolderPicker");
>+  trashFolderPicker._folder = folder;
>+  trashFolderPicker.setAttribute("label", folder.prettyName);
I don't understand this, the event target should already be the folder picker.

Comment 8

[Magnus Melin [:mkmelin]]

Actually no, msgTrashFolderPicker is the menulist, the event target is a node in the menupopup (msgTrashFolderPopup).

Without this change, you can't set the trash folder to a top level folder that has subfolders - since you'd select "choose this folder" in a sub popup.

Comment 9

[[e-mail]]

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

>+  trashFolderPicker._folder = folder;
OK, so what does this do?

Comment 10

[Magnus Melin [:mkmelin]]

Well, since we don't use the value from the picker anywhere later, nothing.

Comment 11

[Magnus Melin [:mkmelin]]

Attachment: proposed fix, v2

Carrying fwd r=bienvenu

Comment 12

[Magnus Melin [:mkmelin]]

changeset:   512:06222c5c8bf1
http://hg.mozilla.org/comm-central/rev/06222c5c8bf1

->FIXED

Comment 13

[Onno Ekker [:nONoNonO UTC+1]]

This isn't fixed for me on Shredder 20081006095651.
Selecting subfolder "[Gmail]/Trash" won't work, reverts to last selected top level folder.

Comment 14

[Magnus Melin [:mkmelin]]

Is that a nightly, or alpha3? This didn't make alpha3.

Comment 15

[Onno Ekker [:nONoNonO UTC+1]]

Alpha 3 :-)