1687412 - folderLookupService doesn't handle folder creation correctly if URI contains accent/diacritic (e.g. Envoyés).

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Ben Campbell]

The initMoveToFolderAgainMenu() function in mailWindowOverlay.js doesn't seem to handle diacritic/accent characters.
The upshot is that it creates a new folder (in memory) instead of finding the existing one!

To replicate:

1. add printfs to dump out info in nsMsgDBFolder::Init() and nsMsgDBFolder::parseURI() - I'll upload a patch.
2. create a gmail IMAP account in TB
3. In that account, create a subfolder with an accent/diacritic, e.g. "testé" (location of the folder doesn't matter)
4. move any old message into that folder
5. Select the folder
6. right click on the message to trigger initMoveToFolderAgainMenu().
7. check through the output on stdio

So, when TB starts up, it picks up the folder just fine and the URI is correct:

XYZZY: 0x0x7fc306944000 (Init() uri='imap://[e-mail]/testé' len=54 0x69 0x6d 0x61 0x70 0x3a 0x2f 0x2f 0x74... 
XYZZY: parseURI 0x0x7fc306944000 escapedFileName='test%C3%A9' len=10 0x74 0x65 0x73 0x74 0x25 0x43 0x33 0x25 

But, at step 6, it's invoked initMoveToFolderAgainMenu(), which calls MailUtils.getOrCreateFolder() with a badly-encoded URI:

XYZZY: 0x0x7fc305263c00 (Init() uri='imap://[e-mail]/test' len=53 0x69 0x6d 0x61 0x70 0x3a 0x2f 0x2f 0x74...
XYZZY: parseURI 0x0x7fc305263c00 escapedFileName='test%E9' len=7 0x74 0x65 0x73 0x74 0x25 0x45 0x39 

Because it's a different URI, it doesn't find the existing folder, and creates a new one in memory (ugh!).

As far as I can tell, the URI in "mail.last_msg_movecopy_target_uri" is coming out borked... (maybe it was borked when set...)

Comment 1

[Ben Campbell]

Attachment: print-init-parseURI.patch

Actually, probably don't really need this patch - could just sprinkle dump() calls about on the JS side... but hey.

Comment 2

[Ben Campbell]

I'm pretty sure it's because nsIMsgFolder.init(string uri) takes a string. Works fine from C++ - the param is a const char * and utf-8 passes through just fine. But calling it from javascript screws up the encoding.
I'm changing it to a AUTF8String (and adding a unit test to exercise it). But tomorrow.

Comment 3

[Ben Campbell]

Attachment: 1687412-fix-up-string-types-1.patch

This changes the IDL definition of an important folder-creation step:

nsIMsgFolder.Init(in string uri)

to:

nsIMsgFolder.Init(in AUTF8String uri)

This means that URIs containing unicode characters passed in from javascript won't get all screwed up - they'll instead be converted to UTF-8.

It's not at all clear which places use percent-encoded (ie ASCII-safe) URIs, and which places use non-percent-encoded UTF-8 URIs, but at least nsIMsgFolder.Init() won't be responsible for screwing it up :-)

It also changes a bunch of related functions to take an abstract nsACstring as a uri param rather than the overly-concrete nsCString.

And yes, the idl Init() signature should probably be lower-cased, but I think I'd rather like to try and get rid of it completely as part of the gradual folder-creation tidy-up.

Try run here,
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=7e42ec5c5dc4d433b8ab3801cbfd2c40f075e37e

Comment 4

[Ping Chen [:rnons] (away)]

Seems with 1687412-fix-up-string-types-1.patch, if I have a testé folder, with every restart, a few new testé folders are created.

Comment 5

[Ping Chen [:rnons] (away)]

Change to use AUTF8String in nsIFolderLookupService.idl fixes comment 4 for me.

Comment 6

[Magnus Melin [:mkmelin]]

Comment on attachment 9198042 [details] [diff] [review]
1687412-fix-up-string-types-1.patch

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

LGTM, r=mkmelin

Comment 7

[Magnus Melin [:mkmelin]]

(In reply to Ping Chen (:rnons) from comment #5)

Change to use AUTF8String in nsIFolderLookupService.idl fixes comment 4 for me.

Too many patches in the air right now. Is this in one of them, or something still needed?

Comment 8

[Ping Chen [:rnons] (away)]

It's need(In reply to Magnus Melin [:mkmelin] from comment #7)

(In reply to Ping Chen (:rnons) from comment #5)

Change to use AUTF8String in nsIFolderLookupService.idl fixes comment 4 for me.

Too many patches in the air right now. Is this in one of them, or something still needed?

It's needed in my testing, otherwise duplicated folders are created on every restart. I will test again, and upload a new patch here.

Comment 9

[Ping Chen [:rnons] (away)]

Attachment: 1687412-fix-up-string-types-1.patch

Ben's patch with two line changes in nsIFolderLookupService.idl

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/3410cfd4332d
Fix up string types in folder creation to better handle unicode URIs. r=mkmelin

Comment 11

[Ben Campbell]

(In reply to Ping Chen (:rnons) from comment #8)

It's needed in my testing, otherwise duplicated folders are created on every restart. I will test again, and upload a new patch here.

Doh! I totally missed that.
Thanks for testing it and patching it up!