User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.8) Gecko/20071004 Iceweasel/2.0.0.8 (Debian-2.0.0.6+2.0.0.8-0etch1) Build Identifier: version 3.0a1pre (2007120804) If you send any email to an utf8-domain, for instance to "alice@пример.com" and the option to automatically add any outgoing email address to an address book is chosen, the address is stored as "alice@пÑимеÑ.com". Cause this is not the same as the recipients address, it get stored again and again the wrong way, every time you try to send a mail. Reproducible: Always Steps to Reproduce: 1. just activate the option to store every email you send a mail to 2. send a mail to "alice@пример.com" (address gets added before mail is sended, so you can abort the sending process) 3. have a look in your addressbook. same behavior with version 2.0.0.9 (20071031)

xref bug 127399

I've reproduced this locally, from what I've seen on looking at various RFC & other documents on the internet we should be handling this correctly. I've a fix for the code including some clean up, I'm just trying to put together some unit tests to go with it.

This patch fixes this bug, and also fixes a bug whereby when finding the card for working out what we should send the email as (plain/html) it was not finding it if the email address was in utf-8. The specific fix for this bug is in nsAbAddressCollector::CollectAddress where I change NS_ConvertASCIItoUTF16 to NS_ConvertUTF8toUTF16. The specific fix for the finding a card for plain/html lookup is in nsMsgCompose::CheckAndPopulateRecipients where I change NS_LossyConvertUTF16toASCII to NS_ConvertUTF16toUTF8. Whilst writing the unit test for this bug, I came across other inconsistencies. As discussed on #maildev, we decided to drop "collectUnicodeAddress" and just require that "collectAddress" takes a UTF-8 value. Following on from that, it then became apparent that in using js to collect the address in the unit test, I had to make nsIAbMDBDirectory::cardForEmailAddress explictly take AUTF8String as well. Hence the nsIAddrDatabase change followed on from that as they are linked, and the other changes made sense as well. I believe this shouldn't hurt our existing functionality just make us more consistent in how we do things. Unit test could probably do with expanding (see comments at top), but for now it at least covers this bug and I'll deal with the remaining items in follow up bugs/further fixes.

Comment on attachment 292470 [details] [diff] [review] The fix and unit test Note: the tests themselves completed, but the cleanup code was unable to remove abook.mab (presumably this problem doesn't occur on Linux). >+ * nsIAbAddressCollector is the interface to the address collector service. Wishful thinking. Or are you planning on correcting its name? ;-) >+ nsCString(aName).get(), I'd prefer [note: no ns] PromiseFlatCString(aName).get(), >+ do_check_eq(card.preferMailFormat, aDetails[1]); I think you should check the email address too. In fact, you should probably check all the fields the collector can set.

(In reply to comment #4) > (From update of attachment 292470 [details] [diff] [review]) > Note: the tests themselves completed, but the cleanup code was unable to remove > abook.mab (presumably this problem doesn't occur on Linux). Hmm, I wonder if the directories are still holding on to it. I'll deal with that in a separate bug then, though if you could post me the error code, that'd be useful. > >+ * nsIAbAddressCollector is the interface to the address collector service. > Wishful thinking. Or are you planning on correcting its name? ;-) Not at the moment. I'll correct it later. > >+ do_check_eq(card.preferMailFormat, aDetails[1]); > I think you should check the email address too. In fact, you should probably > check all the fields the collector can set. See the XXX at the top of the file. I've got it on the list of things to do - my main reason for doing it separately is that I want to closely examine what the collector is doing and I think some of it may need changing, hence I didn't want to confuse this bug by doing too many things at once.

Fixed Neil's nits, but see also comment 5 - I'll look at those in future bugs. Requesting sr.

Patch checked into trunk. Fixed.

thanks for your fast engagement to fix the problem. regards.