125573 - Crash when typing after switch to "ZhiNeng ABC" SimpChinese IME

Status: VERIFIED FIXED

(Core :: Internationalization, defect)

Description

[Yuying Long]

Build: N6.2.1 and 02-14 trunk build on WinXP-SimpChinese

Steps:
1. Launch browser and change to Chinese IME - I have a few Simp. Chinese IME on
my machine.
2. Ctrl-space to turn on MS SimpChinese IME, type some word, e.g. "ka" then
convert it to a SimpChinese character.
3. Ctrl-Shift or using mouse select to switch to another IME, e.g. "Zhineng ABC".
4. Type something.

Result:
Crash

Comment 1

[Yuying Long]

Not available to get the talkback data right now, will attach it later.

Comment 2

[Yuying Long]

-> nsbeta1

Comment 3

[Yuying Long]

Talk back ID:
2920334, 2920063,  2920090,  2919901.

One of them:
Stack Signature  IMM32.DLL + 0x48b9 (0x763048b9) 8b6742ed
Trigger Time 2002-02-14 13:11:56
Email Address [e-mail]
URL visited
Build ID 2002021410
Product ID MozillaTrunk
Platform
Operating System Win32
Module
Trigger Reason Access violation
User Comments composer
Stack Trace
IMM32.DLL + 0x48b9 (0x763048b9)
IMM32.DLL + 0x5da5 (0x76305da5)
msimtf.dll + 0x1bba1 (0x7466bba1)
msimtf.dll + 0x1ce76 (0x7466ce76)
msimtf.dll + 0x1a816 (0x7466a816)
nsWindow::OnIMEComposition
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 5365]
nsWindow::ProcessMessage
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 3735]
nsWindow::WindowProc
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 1116]
USER32.dll + 0x3a5f (0x77d13a5f)
USER32.dll + 0x3b2e (0x77d13b2e)
USER32.dll + 0x3d6a (0x77d13d6a)
USER32.dll + 0x41fd (0x77d141fd)
nsAppShellService::Run
[d:\builds\seamonkey\mozilla\xpfe\appshell\src\nsAppShellService.cpp, line 308]
netscp6.exe + 0x1f95 (0x00401f95)
netscp6.exe + 0x1b45 (0x00401b45)
netscp6.exe + 0x3254 (0x00403254)
kernel32.dll + 0x1eb69 (0x77e5eb69) 

Comment 4

[Rui Xu]

it seems only heppened when switching the IME to "Zhineng ABC".

Comment 5

[Yuying Long]

Yes, switch to ZhiNeng ABC. - I saw it on Mail, IM on both N6.2.1 and 02-14 trunk.

See another very similar bug 90595, seems they are same crash but on WinXP-CN,
the steps is more clear.

Comment 6

[Frank Tang]

somehow my english winxp do not come with such IME.

Comment 7

[Frank Tang]

nsbeta1+ per triage meeting. 

Comment 8

[Roy Yokoyama]

Can I repro this in W2K-SC?

Comment 9

[Yuying Long]

We used had bug 90595 for win2k-cn, but it's really hard to reproduce on win2k,
it's just happens occasionally, and seems same crash on WinXP-CN.

Comment 10

[Yuying Long]

This crash can not be reproduced on Win98-CN.  

Also using this IME on win98, I don't see the IME panel will cover the current
input line problem (bug 90583). 

Comment 11

[Roy Yokoyama]

Ok, I installed WinXP-SC and it definitely crash; but not with the debug build.
I am search for some clues here......

Comment 12

[Roy Yokoyama]

Man, Optimized build doesn't crash.   This is really hard.
Is this only happening with Netscape commercial?

Comment 13

[Yuying Long]

Yes, I installed 03-06 mozilla build, and can not reproduce the crash there.

Comment 14

[Roy Yokoyama]

Ok, I have no idea why this is crashing.
"ZhiNeng ABC" sometime crashes; but not all the time.
When it crashes, it crashes at NS_IMM_GETCOMPOSITIONSTRINGW(..,GCS_COMPCLAUSE,)
which is consistant with the Talkback report.

The problem is when I _printf_ the parameters of NS_IMM_GETCOMPOSITIONSTRINGW()
between the crash and the non-crash, they are both valid values.

What really surprises me is that the crash only happens with NS build. 
(we may want to move this bug to bugscape).  
The Theme may have something to do with the crash, 
so I changed the theme to Classic to test. It still crashes intermittently.

Comment 15

[Roy Yokoyama]

I added HRESULT hr = nsToolkit::gAIMMApp->GetDescriptionW();
just for testing purpose before NS_IMM_GETCOMPOSITIONSTRINGW()
and it returns ERROR code.  Every other IMEs are fine.

Comment 16

[Roy Yokoyama]

If I call A API instead of W API,then this doesn't crash.
( ie make nsToolkit::mIsNT to be PR_FALSE)

Comment 17

[Roy Yokoyama]

I could potentially only do the following 		
-NS_IMM_GETCOMPOSITIONSTRINGW(hIMEContext,
+NS_IMM_GETCOMPOSITIONSTRING(hIMEContext,
Above doesn't cause to crash as well.

However, I'd prefer making globale nsToolkit::mIsNT to be PR_FALSE, because
1) easy to maintain
2) need to make sure to call MultiByteToWideChar() in some cases; 
   but not the other. (ie. messy)

Comment 18

[Roy Yokoyama]

Attachment: Detect WinXP-Simplified Chinese OS and make nsToolkit::mIsNT = PR_FALSE;

When I switch to problematic IME, bunch of Dlls (PINTLGNT.IME, mscandui.dll)
got loaded into memory.  After these Dlls, "ZhiNegn ABC" crashes at
NS_IMM_GETCOMPOSITIONSTRINGW() for unknown reason.

I used ::GetACP() instead of ::GetSystemDefaultLangID().
User can change the system locale to non-Simplified Chinese
from the Control Panel; but he/she still uses the same crashing IME. 
::GetACP() returns OS lang as Simplified-Chinese despite the Control Panel 
setting.

ftang/shanjian: This is very hacky patch, but can you review? We need to
		fix the CRASH.

Comment 19

[Shanjian Li]

roy,
I tried to fix bug 90595 before but failed to find a solution. Some IME 
implementations might be the one to be blamed. Since the crash is related with 
IME switching, I guess some of the IME do not clean up themselves well or fail 
to perform some necessary initialization. What you have found seems suggesting 
the problem only exist in W version. These 2 assumptions might be verified 
through following precedures:
1) On win2k and winxp, if GetACP is S Chinese, set an flag mIsSChineseACP.
2) In ResetInputState, set mNeedReinitialization to true.
3) In OnIMEComposition, when both those flags are set, call a addition A version 
of NS_IMM_GETCOMPOSITIONSTRING before NS_IMM_GETCOMPOSITIONSTRINGW. I hope this 
will take care of the initilization.
4) Keep everything else unchanged, ie. still use W for real operation.

If the test is successful, we should have better understanding of what's 
happening. Various approaches can be judged from there. Setting mIsNT to 
PR_FALSE looks too hacky to me. I would prefer to just use A version for 
NS_IMM_GETCOMPOSITIONSTRING when ACP is S chinese if we are out of option. Win2k 
should be taken care of as well.

Comment 20

[Roy Yokoyama]

shanjian:  
>Calling NS_IMM_GETCOMPOSITIONSTRING before NS_IMM_GETCOMPOSITIONSTRINGW
It didn't help.  It still crashes. 

>Setting mIsNT to PR_FALSE looks too hacky to me. 
Why?  I think mixing W and A calls based on some Conditions are hackyer.
Especially in this place where it's trying to get Clause index of Composition 
string.  You should look at MapDBCSAttributeArrayToUnicodeOffsets().
Granted, I don't completely understand this iteration code, 
I don't feel comfortable getting Clause index from A api and setting the offsets
of Composition string retrieved from W api.

Comment 21

[Roy Yokoyama]

Attachment: Covering W2k-CS and WinXP-CS

Comment 22

[Shanjian Li]

mIsNT should mean what it says in its name. I do not have strong objection to 
using A version of IME api if it fix the crash. But I don't agree using mIsNT to 
means something else. If mIsNT is only used to choose A and W version, we can 
change its name to mIsUseImeApiW or something similar. If it is used to control 
some other things that we don't want to interfere, we might have to add this new 
flag instead of replace mIsNT. 

Comment 23

[Roy Yokoyama]

>mIsNT should mean what it says in its name.
I LXR'ed for mIsNT and it is only used to choose between W and A for IME.
I am going to rename the variable and post a new patch.

Comment 24

[Roy Yokoyama]

Attachment: Rename from mIsNT to mUseImeApiW

I spent few days on this trying to find out the real cause; but I couldn't
pin point.  Changing to A API was purely accidental, because
I was running out of options.  I tried Purify, but it corrupted my MSVC++
and I was forced to uninstall Purify.  I guess Purify doesn't like XP (?)

shanjian: can you review or any help will be appreciated.

Comment 25

[Shanjian Li]

Comment on attachment 74150 [details] [diff] [review]
Rename from mIsNT to mUseImeApiW

r=shanjian. As I know, those IME modules were developed by non-microsoft
vendors, and most likely they are using native encoding internally.  I guess
their W API is not correctly implemented.

Comment 26

[Brendan Eich [:brendan]]

Comment on attachment 74150 [details] [diff] [review]
Rename from mIsNT to mUseImeApiW

Two if styles ('if(' and 'if (') at war in the same patch hunk:

+    if(!(osVersionInfoEx = GetVersionEx ((OSVERSIONINFO *) &osversion))) {
+      // If OSVERSIONINFOEX doesn't work, try OSVERSIONINFO.
+      osversion.dwOSVersionInfoSize = sizeof (OSVERSIONINFO);
+      if (! GetVersionEx ( (OSVERSIONINFO *) &osversion) ) {
+	 // maybe we are running on very old Windows OS. Assign FALSE.
+	 nsToolkit::mUseImeApiW = PR_FALSE; 
+	 return;
+      }
+    }
+
+    if (osversion.dwPlatformId != VER_PLATFORM_WIN32_NT)  {

Various space-around-(-and-) style too.

This code:

+    if (osversion.dwPlatformId != VER_PLATFORM_WIN32_NT)  {
+      // none Win32 platform.
+      nsToolkit::mUseImeApiW = PR_FALSE;
+    }
+    else {
+      nsToolkit::mUseImeApiW = PR_TRUE;
+      // XXX Hack for stopping the crash (125573)
+      if (osversion.dwMajorVersion == 5 && (osversion.dwMinorVersion == 0 ||
osversion.dwMinorVersion == 1))  { 
+	 // "Microsoft Windows 2000 " or "Microsoft Windows XP "
+	 if (936 == ::GetACP())  // Chinese (PRC, Singapore)
+	   nsToolkit::mUseImeApiW = PR_FALSE;
+      }
+    }
+

could be just this:

+      nsToolkit::mUseImeApiW = (osversion.dwPlatformId !=
VER_PLATFORM_WIN32_NT);
+    if (nsToolkit::mUseImeApiW)  {
+      // XXX Hack for stopping the crash (125573)
+      if (osversion.dwMajorVersion == 5 && (osversion.dwMinorVersion == 0 ||
osversion.dwMinorVersion == 1))  { 
+	 // "Microsoft Windows 2000 " or "Microsoft Windows XP "
+	 if (936 == ::GetACP())  // Chinese (PRC, Singapore)
+	   nsToolkit::mUseImeApiW = PR_FALSE;
+      }
+    }

Fix what makes sense here, sr=[e-mail].

/be

Comment 27

[Roy Yokoyama]

Attachment: Fixing styles

Comment 28

[Asa Dotzler [:asa]]

Comment on attachment 74240 [details] [diff] [review]
Fixing styles

approval noted in bug.

--Asa

Comment 29

[Roy Yokoyama]

Fix checked in.

Comment 30

[Yuying Long]

Fixed verified on 03-19-10 trunk build.