Closed Bug 29543 Opened 25 years ago Closed 24 years ago

New created Japanese local folder name is displayed as dots.

Categories

(MailNews Core :: Internationalization, defect, P3)

Product:
MailNews Core
Component:
Internationalization
Platform:
All
Windows 95
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: ji, Assigned: scottputterman)

References

Details

(Whiteboard: [NEED INFO])

Attachments

(5 files)

a patch of XPCOM/IO
15.64 KB, patch
Details | Diff | Splinter Review
a patch of mailnews
9.33 KB, patch
Details | Diff | Splinter Review
new patch of mozilla/netwerk/base/src/nsURLHelper.cpp
1.01 KB, patch
Details | Diff | Splinter Review
new patch of mozilla/mailnews/*
7.19 KB, patch
Details | Diff | Splinter Review
assertion in mork using ToNewCString
5.12 KB, text/plain
Details 
Build: 2000-02-28

The new created Japanese local folder name is displayed as dots.

Steps of reproduce:
1. Launch messenger.
2. Highlight Local Folders and select File | New | Folder..
3. Enter a japanese folder name which doesn't contain 0x5c chars, like kanji
4. Click on OK button

you'll see the new created Japanese folder is displayed as dots.
Reassign to putterman@netscape.com.
This seems to be a Japanese (or CJK) specific. MacRomman does not have this 
problem.
Assignee: nhotta → putterman
Blocks: 29546
The mailbox file does not seem to get cerated. 
The only thing created when trying to use JPN name
is the ....msf file -- it actually has these dots. So, 
when you quit and re-start, you will not see the folder 
you were supposed to have created in the prior session.
Status: NEW → ASSIGNED
Target Milestone: M16
Same results for subfolder under POP account.
*** Bug 33608 has been marked as a duplicate of this bug. ***
This is cross-platform and will be included in the Beta 1
Intl relnotes.
Keywords: relnote
Blocks: 14744
Target Milestone: M16 → M17
Putting beta2 for i18n beta2 criteria items. Contact bobj for question.
Keywords: relnote → beta2
Blocks: 35851
Add me to CC.

please review the patches.  This patch is inclued bug 29546 and bug 29145.
these patches will be able to fix bug33608, bug 33624 and 33643, too.
this is fantastic.  Thank you!  Could someone with a Japanese machine try this 
out?  I will try this out on my machine to make sure non Japanese folders are 
still working.
I looked at the file spec part of the change with ftang. The code looks fine but 
could you give us a brief description of your change?
Also adding mkaply@us.ibm.com since the change indicates that the similar change 
may be needed for OS/2.

I applied the patch and tested on WinNT 4 (Japanese localized).
I was able to create/rename Japanese folders. Folders were displayed in Japanese 
correctly. Also able to see the messages in Japanese folders (including folder 
name with 0x5c).

I tried out this patch and it seems to be breaking adding a new folder to imap 
so far on my current Windows build.  It seems that the function IsDirectory is 
return that a directory that doesn't exist is in fact a directory.  I haven't 
tried local folders yet.
same with local folders.  I can't create a new folder with local folders either.
I didn't try imap but I was able to create local folders.
Did you apply both patches?
Yes. I think the problem is that GetFileAttributes is returning failure for the 
case when the directory doesn't exist already and we are returning PR_TRUE 
instead of PR_FALSE.
When I change IsDirectory to the following code, it works for me again.  this is 
in nsFileSpecWin.cpp

PRBool nsFileSpec::IsDirectory() const
//------------------------------------------------------------------------------
----------
{
	if (mPath.IsEmpty())
		return PR_FALSE;
	
	DWORD attr = GetFileAttributes(mPath);

	if(attr == 0xFFFFFFFF)
		return PR_FALSE;

	return (attr & FILE_ATTRIBUTE_DIRECTORY) ? PR_TRUE : PR_FALSE;
} // nsFileSpec::IsDirectory

new folder and rename are working for me for both pop and imap on my Windows 
build after the one fix I wrote above.  I won't have the ability to test this 
out on Mac and Linux until Thursday.  Can anyone else try them out?  Also, have 
we verified this works on Japanese systems?  If so, then we are set.  
m_kato@ga2.so-net.ne.jp  thanks again for writing this.
I missed nhotta's comment about trying this out on a Japanese system.  cool.

I just have 2 other questions.  1. How come we had to change to use 
GetFileAttributes for IsDirectory on Windows?  2.  Eventually we will have to 
switch to use nsIFile.  Is there a solution that will work for this (this 
doesn't effect this patch, I'm just curious if we will find these bugs reopened 
again).
> I looked at the file spec part of the change with ftang. The code looks fine
> but could you give us a brief description of your change?

nsFileSpec has two bad-cast issues.

1. nsCAutoString::nsCAutoString(nsString)
2. nsSimpleCharString::operator = (const nsString& inString)

If this code uese, it cannot convert correctly.  And this code is inclued other 
0x5c problems..


> 1. How come we had to change to use GetFileAttributes for IsDirectory on
> Windows?

Sorry, it doesn't need this code.  But if it uses GetFileAttributes() API,
over-head becomes smaller then current code...


> 2.  Eventually we will have to switch to use nsIFile.

nsIFile interface has many bugs of i18n.  So you should not rewrite to IFile, 
yet.
Keywords: nsbeta2
It is expected that ftang is going to change nsIFileSpec along with his changes 
of nsIFile. He said that will include the patch of this bug (xpcom/io part).
After his check in, we can try mailnews part of the patch and check that in.
About the migration to nsIFile, I am familiar with the code but as we currently 
subclassing nsIFileSpec, it may need extensive change.
Anywasy, as this bug is blocking other i18n testing, we probably want to fix 
this early for beta2.
Keywords: beta2
QA contact to ji.
QA Contact: momoi → ji
I'm sorry I've been sitting on this.  Since I've had these changes in my tree, 
there have been a bunch of conflicts with other checkins. Could you make sure 
these still work with these new checkins? If they don't could you post another 
patch and if you can get it today, I will verify them on all of the platforms by 
tomorrow.
m_kato-
I have check in all the nsFileSpec changes (I hope) and mjudge change a lot of 
nsString stuff so now the char* to / from nsString have to use a different and 
more explicit name (xxxWithConversion) 
Can you submit a new patch for the mailnews ? (I assume the XPCOM/IO have 
already covered and in the tree. If not, please let me know)

Here is my comment about the old mailnews patch, please address them in your new 
patch. Thanks !!!!

1. nsLocalFolderSummarySpec.cpp- make sense. I don't think we need new patch for 
that part. putmann- I think we should check in that part now.
2. nsNewsSummarySpec.cpp- make sense. I don't think we need new patch for that 
part. putmann- I think we should check in that part now.
3. nsLocalMailFolder.cpp (477,508 => 477,502)
I think it make sense from i18n view. However, mailnews folks should review the 
following lines carefully-

!               rv = AddSubfolder(&currentFolderNameStr, getter_AddRefs(child));
!               if (NS_FAILED(rv))
!                       return rv;
!               child->SetName(currentFolderNameStr.GetUnicode());

is the if(NS_FAILED(rv)) and the followling correct error handling ?

4. nsLocalMailFolder.cpp (518,530 => 512,541)
a. We should use nsITextToSubURI::ConvertAndEscape for this
b. Is this the right thing to do with the RDF data source ? I am not sure what 
the other end (the RDF data source) expect. Is the other end (RDF data source) 
expect a file: url ?If yes, then the code is right. But we better may this clear 
in the interface...
5. nsLocalMailFolder.cpp (630,638=>640,650) Make sense, I think we should check 
in now.
6. nsLocalMailFolder.cpp (955,962 => 955,962)- This looks like a logical 
addition changes. mailnews folks, you have to review this by yourself. I have no 
idea what does this mean. m_kato, please explain.
7. nsLocalMailFolder.cpp (1213,1218 => 1227,1233)- This looks like a logical 
addition changes. mailnews folks, you have to review this by yourself. I have no 
idea what does this mean. m_kato, please explain.
8. nsLocalMailFolder.cpp (1288,1311 => 1303,1342)-
a. Do we still need to convert if the nsFileSpec::Rename support the nsString 
version correctly (Does it ?). Is that true we no longer need to convert after 
we fixed the nsFileSpec
b. If we need to perform conversion for oldPathSpec->Rename and 
oldSummarySpec.Rename, why we don't need it for dirSpec.Rename ?
c. You add a new if (NS_SUCCEEDED(rv))  checking and mailnews folks should 
review this carefuly about the error handling. I cannot review this for you :)
d. You add a newFolder->SetName() after the if (NS_SUCCEEDED(rv)) . m_kato, 
please explain. mailnews folks, you have to review this by yourself 

End of my comment.

I suggest we break down this bug. This looks like a patch solve MANY problems. I 
don't think QA can easily track and verify this bug this way.
frank and all, I have a question.
Should I rewrite from nsFileSepc to nsLocalFile??
what I meant was that when I run this patch, it no longer works for me.  I was 
wondering if I could get a patch based on a recent build. Also, did the filespec 
changes get the Windows IsDirectory change I made? 
please use new 2 patches.

ftang,

> 4. nsLocalMailFolder.cpp (518,530 => 512,541)

new patch doesn't use escaped URI.  Because when it use non-escaped URI inclued 
0x5c, folder will always have child folder.  But new patch is inclueded a patch 
of necko.  So it doesn't occur.

> You add a new if (NS_SUCCEEDED(rv))

error check.  If rv isn't NS_OK, AV alway occurs!
cc'ing andreas.  Andreas, could you review the nsURLHelper part of the bug?

Also, this is working for me on Windows.  I will now try Linux and Mac.  I'm 
only making sure that it works on my machine.  If someone can, could we make 
sure this works on the three platforms on a Japanese machine since the patches 
have been changed since originally posted?
the patches worked on my linux and mac machines.  If we can get the 
nsURLHelper.cpp code review and if someone can verify this fixes Japanese 
machines then we can check this in.
The patch to nsURLHelper.cpp is in an XP_PC only section, there should be no
effects on unix and mac. Looking at nsURLHelper.cpp we might have to move some
more of the \ stuff into the XP_PC section, which could also be a problem for
this bug, but only under very rare circumstances. Beside that the patch looks
good to me.
there were changes made to some mail files which aren't only PC which is why I 
was asking for Linux and Mac reviews.
I applied the patches and ran with japanese localized WinNT 4.
I tested local folder creation and rename. 
Creating japanese folder name, I got an assertion in mork which doesn't seem to 
be related with the patch (in ToNewCString since the data contains japanese, I 
will attach the call stack). Otherwise it works fine, I was able to create 
japanese folders (including names with 0x5c).
Rename also works fine. Although I occasionally saw "Unknown error" dialog for 
the folders which name contains 0x5c. This happened after I rename a folder and 
click the folder or a message thread. Also in the profile directory, extra .msf 
are found for the folders with 0x5c. Looks like they didn't renamed correctly. 
So they keep grow when the folder is renamed.
Anyway, we haven't gone this far without the patch so we want to check them in 
so the QA can test for more detail.


Need bobj to assist with the nsbeta+ OR minus call.  bobj?
Whiteboard: [NEED INFO]
OK, I checked in the patch.  Can I mark this fixed, in which case we'll open up 
new bugs for any other problems, or do we want to keep this bug open and 
continue in it?

Thanks again for the patch, m_kato@ga2.so-net.ne.jp 
This is really [nsbeta+].
Given the foregoing discussions, it seems that the remaining problem might be in the
area covered by Bug 29546 involving 0x5c characters in Shift_JIS. In that case, we can resolve this bug 
as fixed and deal with a new problem there and elsewhere unless of course we find that the names
still show up as dots, in which case we need to re-open this bug. In short, my suggestion would be 
to deal with:

1) "dots" problem on a newly created folder in this bug. For other problems, re-open one of the following three
    or file a new bug.

2) 0x5c problem in Bug 29546.
3) Not able to rename JA POP folder in Bug 33624
4) POP server duplicate folder name problem in Bug 33643
Blocks: 33643
ok, then I will mark this fixed.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Verified as fixed both on win32 2000050309 and linux 2000050308 comm build.
Status: RESOLVED → VERIFIED
Blocks: 28424
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: