61100 - Cannot import non-Latin1 folder name in Outlook Express

Status: VERIFIED FIXED

(MailNews Core :: Internationalization, defect, P3)

Description

[Makoto Kato [:m_kato]]

releted to bug 58897.  But this problem is other.
I think that this bug is very very critical for Japanese User.  Hotta-san, I 
hanve fix code.  please review...

REPRO STEP
==========
1. Run Mozilla mail
2. select Import
3. select Outlook Express
4. select [ok] and [next]

Comment 1

[Makoto Kato [:m_kato]]

Attachment: fix code for OE

Comment 2

[Makoto Kato [:m_kato]]

Attachment: new fixes (included bug 58897)

Comment 3

[nhottanscp]

Reassign to Kato san.

Comment 4

[nhottanscp]

My comments for the patch.
* Please use nsICharsetConverterManager2 instead of nsICharsetConverterManager.
* Please check the return value of SystemStringFromUnicode in ConvertToUnicode.
* Release the encoder in nsImportService destructor.

Comment 5

[Scott MacGregor]

Let me know when you post new patches with Naoki's changes and I'll look at
super reviewing that patch. Thanks!

Comment 6

[Makoto Kato [:m_kato]]

Attachment: new patch included in hotta's commnet.

Comment 7

[Scott MacGregor]

Couple comments,
in nsMsgFolder we should be using a nsXPIDLString for the variable: folderName.
Can you change that:
PRUnichar *folderName = nsnull;
should be
nsXPDILString folderName.

Then you can get rid of the two nsMemory::Free(folderName) calls that you had to
add.

I didn't understand the comment in this line:
if (!gService || NS_FAILED(rv)) // XXX bad cast
what was the bad cast? I don't see anything that looked strange.

Everyting else looks good to me.

Comment 8

[nhottanscp]

In the patch (11/29), there are still no return value checks for
SystemStringToUnicode() and SystemStringFromUnicode() in nsImportMail.cpp.
Please added those checks.

About Scott's comment, that's a fallback when charset conversion not available.
It could be a bad cast for non Latin1 but it's okay for Latin1. So you may want
to add more words in the comment or change to "fallback to Latin1".

Comment 9

[Makoto Kato [:m_kato]]

mscott, is this no problem by the following code??

nsMsgFolder.cpp
 :
 :
if(NS_SUCCEEDED(rv))
{
  nsXPIDLString folderName;
  rv = folder->GetName(getter_Copies(folderName));
  // case-insensitive compare is probably LCD across OS filesystems
  if (NS_SUCCEEDED(rv) && nsCRT::strcasecmp(folderName, uniName.GetUnicode()) 
== 0)
  {
    *aChild = folder;
    NS_ADDREF(*aChild);
    return NS_OK;
  }
}

And nsString::AssignWithConversion occurs bad cast when string is no-latin1 
string.  These codes uses in many parts (mailnews, widget etc)

Comment 10

[Scott MacGregor]

Thanks for making the change to use nsXPIDLString. These changes look good to
me. sr=mscott. I think you still need a final r= from Naoki though. 

Comment 11

[nhottanscp]

Where is the new patch (after 11/29)?
I need to see the patch in order to review.

Comment 12

[Makoto Kato [:m_kato]]

Attachment: new patches

Comment 13

[Makoto Kato [:m_kato]]

I already send via mail (Date: Thu, 07 Dec 2000 02:13:43 +0900).
But I attach this bug.  Hotta-san, please review

Comment 14

[nhottanscp]

As I mentioned before, return value checks are needed for
SystemStringToUnicode() and SystemStringFromUnicode() in nsImportMail.cpp.

I don't think I recived the patch by e-mail. Anyway, it's good to attach the
patch to bug report. See the mozilla code review page for detail.
http://www.mozilla.org/hacking/reviewers.html

Comment 15

[nhottanscp]

I put r=nhotta to reduce an extra review process, please include the error check
before you check in.

Comment 16

[Makoto Kato [:m_kato]]

check in

Comment 17

[Katsuhiko Momoi]

** Checked with 1/7/2001 Win32 build **

Th original problem reported was the following:

1. Have Japanese or other non-Latin 1 Outlook Express with
   mail folders names using non-Latin 1 folder names, e.g. Japanese.
2. Start Mozilla Mail and engage File | Import menu and choose Mail.
3. From the selection window, choose Outlook Express and start importing.
4. It seems to import the folders but when you look at the imported 
   folder names, the ones with non-Latin 1 names are unreadable.
5. When you re-start Mozilla, the unreadable folder names are gone.
6. Therefore it seems what you see in step 5 is just a temporary situation.
   The folders seem to be there but not really and they disappear 
   upon re-starting. 

With the above new build, not only the import succeeds but we can also
read the Japanese or non-ASCII names and they stay in the 
imported account after a re-start. 

Marking the fix as verified as fixed.

Comment 18

[Katsuhiko Momoi]

CC'ing IQA people.