1430168 - Account manager stores localised trash path in pref trash_folder_name

Status: RESOLVED FIXED

(Thunderbird :: Account Manager, enhancement)

Description

[Jorg K (CEST = GMT+2)]

In bug 1335982, bug 1428666 and bug 1427507 we improved the trash folder handling.

I also noticed that a localised version, for example Spanish, will store a localise name, for example "Bandeja de entrada/Trash" as the trash folder path with the user select a folder in the account manager.

The Account Manager should obtain the URI of the selected folder and store it after converting MUTF7 to regular unicode.

Comment 1

[Jorg K (CEST = GMT+2)]

Attachment: 1430168-trash-folder.patch (v1)

The code in am-server.js was fundamentally flawed. It needs to be done like in nsImapIncomingServer::DiscoveryDone().

It's hard to test that it works if you don't have a localised version, but you can do a basic test.

Create a folder with a non-ASCII character, like Träsh. Select it as trash, then select a normal folder. Observe the preference trash_folder_name while you're doing it.

If all is well, it will behave with the patch just like without. But with the patch the localised version will also work.

One review will do ;-)

Comment 2

[gene smith]

I saw the same result with and without the patch.

Comment 3

[gene smith]

Comment on attachment 8942306 [details] [diff] [review]
1430168-trash-folder.patch (v1)

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

Since I am not running localized I don't know exactly what would happen here. I did the test you mentioned and got the same result with and without the patch. I dumped the variables and see something like this when I pick a new trash folder:

gds: picked folder is [xpconnect wrapped nsIMsgFolder @ 0x7f46b05b4b80 (native @ 0x7f46bbe36000)]
gds: picked folder uri is INBOX/Tr&AOQ-sh
gds: picked folder uri unicode is INBOX/Träsh
gds: picked folder is [xpconnect wrapped nsIMsgFolder @ 0x7f46b05b4b80 (native @ 0x7f46bbe36000)]
gds: picked folder uri is INBOX/Tr&AOQ-sh
gds: picked folder uri unicode is INBOX/Träsh

Note: the function folderPickerChange(aEvent) occurs twice on each pick.

::: mailnews/base/prefs/content/am-server.js
@@ -392,5 @@
> -    path = parentFolder.name + "/" + path;
> -    parentFolder = parentFolder.parent;
> -  }
> -  // IMAP Inbox URI's start with INBOX, not Inbox.
> -  return path.replace(/^Inbox/, "INBOX");

This replacement of Inbox at start of string with INBOX seems to be removed from the code by this patch. I never actually see the uri start with Inbox anyhow but only INBOX, e.g., INBOX/child-of-inbox/Ta-rash. Not sure why this was considered important.

Comment 4

[Jorg K (CEST = GMT+2)]

Comment on attachment 8942306 [details] [diff] [review]
1430168-trash-folder.patch (v1)

Thanks, I'll go with one review and land that this morning before the Daily run. Then we can all try it out in a localised version ;-)

Comment 5

[Jorg K (CEST = GMT+2)]

(In reply to gene smith from comment #3)
Thanks, Gene, I dumped out the same variables before submitting the patch ;-)

path.replace(/^Inbox/, "INBOX") was necessary before, since this originally used the folder name. Now we're using the URI and then of course that clumsy hack is no longer required.

Comment 6

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/35d2dc1476b40f7414ade6efffe758e3e10a2cc9
fix the way the account manager handles the trash folder. r=GeneSmith
(Pulsebot is taking the day off?)

Comment 7

[Jorg K (CEST = GMT+2)]

I tried this in a Spanish Daily from
http://ftp.mozilla.org/pub/thunderbird/nightly/latest-comm-central-l10n/
and it works :-) The preference now stores "INBOX/..." and no longer "Bandeja de entrada/..." \o/
You can now switch between various localisations all on the same profile.

Finally the issue I complained about in bug 1156669 comment #28 is fixed, after three years!
And thanks to Gene, I had forgotten about that bug.

Comment 8

[Jorg K (CEST = GMT+2)]

Comment on attachment 8942306 [details] [diff] [review]
1430168-trash-folder.patch (v1)

This would need to go with four more bugs and it's not worth it at TB 52.7 when TB 60 ESR is coming out six weeks later.