1427940 - non-ASCII char in folder name trash folder name (and displays Choose Folder again)

Status: RESOLVED FIXED

(Thunderbird :: Account Manager, defect)

Description

[gene smith]

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0
Build ID: 20171130114032

Steps to reproduce:

In server settings, select the "move to trash" folder as a folder with a name consisting of at least one non-ascii character (e.g., Russian/cyrillic) such as Удаленные -- I have no idea what this means. Notice that this folder now has the trash icon and deletes go to it (after a TB restart unless the fixes for Bug 1427507 are in effect).




Actual results:

Now return to the server setting again and notice that the prompt for the trash folder  entry says "Choose Folder..."    


Expected results:

The prompt for the trash folder entry should say "Удаленные on <my-account>".  However, re-selecting this folder does not help, but the folder actually remains the trash destination and has the appropriate icon even though it doesn't display here.

When the characters are all "standard" ascii such as folder name "start-with-quote" the bug is not triggered. But with only one "non-standard" char such as what looks like the first 'e' in "start-withеquote", this bug occurs.

Possibly this bug is invalid since my locale is US? 
This bug was found while testing fixes for Bug 1427507.

I placed the debug statement
dump("gds: aFolder is " + aFolder + "\n");
after line 730 of mailnews/base/content/folderWidgets.xml and it prints "gds: aFolder is undefined" when server setting screen is entered and showing "Choose Folder:". If pure ascii folder name is used, info on the folder is printed instead but not the actual folder name.

Comment 1

[Jorg K (CEST = GMT+2)]

I was just about to file a bug "Account manager stores localised trash path in pref trash_folder_name" with this comment:

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.

If we do that, then this problem here should also be solved.

Comment 2

[Jorg K (CEST = GMT+2)]

OK, on my test account I created a Träsh folder and selected it. It immediately received the trash icon and the pref received INBOX/Träsh. So far so good. However, the prompt reverts back to "Choose Folder...".

Comment 3

[Jorg K (CEST = GMT+2)]

OK, I have a fix, patch coming.

That won't fix what I mentioned in comment #1, so I'll file a separate bug after trying tomorrows Spanish Daily build. Sorry about the hijack attempt.

Comment 4

[Jorg K (CEST = GMT+2)]

Attachment: 1427940-non-ascii-trash-folder.patch (v1)

One review will do, whoever comes first.

Note that you need to refresh your tree to pick up the patches from bug 1335982, bug 1428666 and bug 1427507, otherwise this won't work.

Note that
+  // Escape backslash and double-quote with another backslash before encoding.
+  let trashMutf7 = manager.unicodeToMutf7(trashFolderName.replace(/([\\"])/g, '\\$1'));
comes from mailnews/test/fakeserver/imapd.js. Joshua confirmed elsewhere that is required.

Comment 5

[Jorg K (CEST = GMT+2)]

Attachment: 1427940-non-ascii-trash-folder.patch (v1a)

Oops, left-over dump() :-(

Comment 6

[Jorg K (CEST = GMT+2)]

Filed bug 1430168 for the stuff mentioned in comment #1.

Comment 7

[:aceman]

Comment on attachment 8942250 [details] [diff] [review]
1427940-non-ascii-trash-folder.patch (v1a)

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

OK, if this work. I can't test it right now.
Thanks.

::: mailnews/base/prefs/content/am-server.js
@@ +327,5 @@
>    trashPopup._parentFolder = MailUtils.getFolderForURI(aServerId);
>    trashPopup._ensureInitialized();
>  
> +  // Convert the folder path in Unicode to MUTF-7.
> +  let manager = Components.classes['@mozilla.org/charset-converter-manager;1']

Maybe just use a less generic variable name, e.g. charsetConverter.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e129b4cce936
encode folder name into MUTF-7 for folder URI. r=aceman DONTBUILD

Comment 9

[Jorg K (CEST = GMT+2)]

Yes, this does in fact work :-) I left the 'manager' variable name since I've copied that from elsewhere where Sir Aceman approved it (https://hg.mozilla.org/comm-central/rev/df37b1887f4d).

Comment 10

[gene smith]

This fixes the problem for me with the long Russian string as the trash folder. Also, for the short folder name with one non-ascii "Träsh" it works.

Comment 11

[Jorg K (CEST = GMT+2)]

Comment on attachment 8942250 [details] [diff] [review]
1427940-non-ascii-trash-folder.patch (v1a)

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.