140983 - [regression]SC IME doesn't work properly and JA IME works wrong

Status: VERIFIED FIXED

(Core :: Internationalization, defect)

Description

[Yuying Long]

Build: 04-25 branch build on WinXP-SimpChinese

Steps:
1. Find any area can do IME: search field in browser, Composer window...etc.

2a. Start SimpChinese IME:
Result: 
The candidate panel is far away from current imput position.

2b. Start global Japanese IME:
Result:
The candidate string and commit characters are display garbled.

This is a regression between 04-23 and 04-25 RC1 branch build.

Comment 1

[Yuying Long]

Attachment: screen shot on 04-25 RC1 build

2 problems on this:
1. Japanese characters are displayed garbled.
2. SimpChinese candidate panel is away from current input position.

Comment 2

[Yuying Long]

Attachment: Screen shot on 04-23 RC1 branch build

04-23 RC1 build worked fine with both IME.

Comment 3

[Yuying Long]

This is a very bad regression, also it happened very recently -> nsbeta1.

Comment 4

[Yuying Long]

I saw this also on 04-24 trunk build and 04-29 RC1 branch build.

Also for SimpChinese IME:
The problem in screen shot is for MS PinYing IME.  
The other IMEs(ZhiNeng, QuanPin, ShuanPin and ZhengMa) seems OK except NeiMa -
the curent input line will be covered by candidate panel.

Comment 5

[Frank Tang]

this is a very bad regression for cjk users. 
[adt1] bug. We should find out what happen in that several days.
We cannot ship to any asian users without fixing this bug. Users won't be able
to type in any CJK text with this bug. 

Comment 6

[Yuying Long]

Not reproducible on WinME-JA on both 04-29 branch build and 04-25 trunk build.

Comment 7

[Yuying Long]

On same WinXP-SimpChinese system, Traditional Chinese IME is corrupted also.

Comment 8

[Frank Tang]

look at 92491. This looks like one of the suspect. 

Comment 9

[Roy Yokoyama]

Ja-IME and Korean-IME in Win2000-Ja is fine.

Comment 10

[Yuying Long]

Win2k-SimpChinese:
Global JA and TChinese IME display garbled, SChinese IME works fine though.

WinXP-JA:
SimpChinese IME has problem with candidate panel, but JA and TradChinese IME
works fine.

Comment 11

[:deanis74]

From my recollection, bug 92491 doesn't have anything to do with positioning
popups or displaying text.  It is only about selecting text based on keyboard
input.  Does backing out the patch for that bug fix this problem?

Comment 12

[Frank Tang]

we know this happen between 2002042406 and 2002042508 build.

We test on the following configuration, 
       
OS  IME   WinME   Win2K   WinXP  WinNT4
-------------------------------------------
EN  JA            ok
    SC            ok
    TC            ok
    ko            ok
-------------------------------------------
JA  JA    ok      ok      ok     ok
    SC    ok              BAD
    TC    ok              ok
    KO
-------------------------------------------
SC  JA            BAD     BAD
    SC            ok      BAD
    TC            BAD     BAD
    KO
-------------------------------------------

Comment 13

[Frank Tang]

the problem is the state of input method got damage and cause the IME to show up
the bad input and pop up ime candidate window when it should not. 
Input Method is dealing with keyborad input. therefore we suspect 92491 cause
this problem .

Comment 14

[Frank Tang]

IME is heavely related to key event. If some code hajack the keyboard event when
it should not, all the IME state will be damage and cause the problem in this
bug report. 
IT looks like the patch of 92491 is dealing with keyevents....

Comment 15

[Frank Tang]

This could also be intrdouced by 04/24/2002 17:24dbaron%fas.harvard.edu checkin
on b=5693

Comment 16

[Frank Tang]

dbaron%fas.harvard.edu check in fix to trunk for 5603 on 4/10 and we don't heard
any problem with trunk around those days.
pete.zha%sun.com land the fix for 92491 to trunk at Apr 24 00:15 and we know the
trunk 0424 trunk build have this problem
pete.zha%sun.com land the fix for 92491 to branch at 04/25/2002 03:44 and we
know the 2002042408 branch build have no problem but 2002042506 build have problem. 
It is more likely to casued by fix for 92491 rather than 5603. 
We are still trying to have a up-to-date build to verify that the patch 92491 IS
the cause or not.

Comment 17

[Frank Tang]

I am wrong, we try the 0412 trunk build. IT is already broken. I think
dbaron%fas.harvard.edu's trunk build are more likly cause this issue. 

Comment 18

[Frank Tang]

add [e-mail] to the cc list

Comment 19

[Frank Tang]

it is bug 5693, not 5603. 

Comment 20

[Frank Tang]

the previous commet about it break at 0412 build is based on the trunk build on
ylong's machine. When I try to install different trunk build to my machine, I
cannot reproduce the problem on 0412 trunk build. Actually, the problem on trunk
start between 2002041903 and 2002042209 build.
trunk 2002041903 build is ok, trunk 2002042209 build is bad

Comment 21

[Frank Tang]

how about mac?

Comment 22

[Frank Tang]

mac work fine with 0429 branch build
still not sure which patch cause the regression.

Comment 23

[Shanjian Li]

I have a old tree build on 04/04, and I could reproduce the problem there. 

Comment 24

[Roy Yokoyama]

My debug reveals that ::MultiByteToWideChar() API is returning 
0xFF64 and 0xFF62 for shift-jis 0x82 0xa0 (japanese 'a')

::MultiByteToWideChar() gets called on Win2K/XP-Simplified Chinese
only because of crasher bug 125573 which is fixed Mar 18.

If I force to call W API's then this problem doesn't occure in Win2K-SC.
However, my finding doesn't cover Franks' matrics (#12)
which include Japanese OS.

Comment 25

[Roy Yokoyama]

I tested with W2K-ja using SC-IMEs and works fine.
So I suspect my patch (bug 125573) may have caused the regression. :(

Comment 26

[Frank Tang]

ok, here is the update information
There are TWO seperate problem
problem 1. Garbage input on SC version of Win2K/WinXP with TC/JA/KO input method
problem 2. on Window XP Simplified Chinese IME show the candidate window in the
wrong place.
We figure out they are two different issue.
we will spin off the problem 2 into a different bug but use this bug for the
problem 1.
We now know the problem ONLY happen  on Simplified Chinese Win2K/XP with non
Simplified Chinese IME. 

There are 33.7M user in China who will use Simplified Chinese windows.
Not sure how many percent of them will use Win2K/XP
Not sure how many percent of them will use TC/JA/KO input method. Probably not
that many of them. 

Therefore, let's downgrad this bug from adt1 to adt2
I think we can ship beta without this got fixed. 
I think we need this for RTM since there are still fair amount of users in China
will use TC IME

Comment 27

[ji]

I think problem 2 is an adt1 bug though, since Simplified Chinese Windows XP is
our  1st platform on the QA list.

Comment 28

[Yuying Long]

Bug 141513 filed for the candidate window problem.

Comment 29

[Roy Yokoyama]

Attachment: Use IMEGetProperty to determine if the IME supports unicode

I found an IME API ImmGetProperty(hKL, IGP_PROPERTY)
which returns value with IME_PROP_UNICODE set if the IME
is viewed as an Unicode IME.
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/intl/ime_2ws9.asp

We can remove the GetVersionEx() codes all together which
checks the System version.
Patch fixes this bug; but I need to verify if it doesn't generate the 
same old crash (bug 125573)

Comment 30

[Roy Yokoyama]

Ahhh  Now, my WinXP-CS doesn't boot.  Great!! I may have to install WinXP-CS
again to verify my patch.

shanjian/ftang: 
  If you have WinXP-CS, can you try my patch to see if it causes to crash?
  It could take me a while to reinstall the OS.  

Comment 31

[Shanjian Li]

I have win2k SC, but not a XP SC. If that's ok, let me know. 

Comment 32

[Roy Yokoyama]

shanjian: please try the patch on win2k SC.
I got my XP running and it looks like it doesn't crash
and still I can type Japanese.  

I will do more testing; but if we can't crash using 
the ZhiNeng ABC IME (described in 125573), then I believe
we have a fix for both. :)

Comment 33

[Shanjian Li]

I tried your patch on win2k Sc, I could not see any problem.  

Comment 34

[Roy Yokoyama]

Attachment: Add nsToolkit::mW2KXP_CP936

Previous patch may be too risky for GM because it impacts all Windows OSs.
http://bugzilla.mozilla.org/attachment.cgi?id=81971

So instead of global IME change, we added nsToolkit::mW2KXP_CP936
to focus only in Win2000 and WinXP - Simplified Chinese.

shanjian: can you review? Thanks

Comment 35

[Roy Yokoyama]

Comment on attachment 81971 [details] [diff] [review]
Use IMEGetProperty to determine if the IME supports unicode

We may want to check 
this patch for trunk
after moz 1.0 ships

Comment 36

[Shanjian Li]

Comment on attachment 82114 [details] [diff] [review]
Add nsToolkit::mW2KXP_CP936

I am OK with the patch for now. To restrict the impact on branch, checking for
ACP is reasonable. But I would like to suggest to make this more generic once
mozilla 1.0 and netscape commercial is released. I believe we can soly rely on
get property. 
r=shanjian

Comment 37

[Roy Yokoyama]

> I believe we can soly rely on get property. 
I agree.  Thanks for the review.

Just to note that I was running the test on WinXP-SC and 
the ZhiNeng ABC IME was returning as NON-UNICODE IME :)
Other IMEs, such as IME-Ja and other IME-SCs were returning 
as UNICODE IME.  

brendan: can you super review?

Comment 38

[:deanis74]

     static PRBool    mUseImeApiW;
+    static PRBool    mW2KXP_CP936;

Would it be better to make these PRPackedBool?

Comment 39

[kinmoz]

Comment on attachment 82114 [details] [diff] [review]
Add nsToolkit::mW2KXP_CP936

sr=[e-mail]

As dean points out, people should consider using PRPackedBool whenever
possible.

Comment 40

[Shanjian Li]

This variable exist only for purpose of minimize potential risk. It will be
removed in near future. 

Comment 41

[Shanjian Li]

checked in for roy to trunk. 

Comment 42

[Roy Yokoyama]

thanks, shanjian

Comment 43

[Yuying Long]

Fix(garbled part) verified on 05-09 trunk build / WinXP-SC.

Comment 44

[scottputterman]

adding adt1.0.0+.  Please get drivers approval and then checkin to the 1.0 branch.

Comment 45

[Dawn Endico]

Comment on attachment 82114 [details] [diff] [review]
Add nsToolkit::mW2KXP_CP936

a=chofmann,brendan,scc

approved for mozilla 1.0 branch. please check in by midnight.

Comment 46

[Roy Yokoyama]

Checked into the 1.0 branch

Comment 47

[scottputterman]

changing to adt1.0.1+ for checkin to the 1.0 branch for the Mozilla1.0.1
milestone.  Please get drivers approval before checking in.

Comment 48

[Roy Yokoyama]

This bug already check into the branch on 05-21 (see comment #46)
I forgot to add keyword "fixed1.0.0".  (Adding now)
ylong: can you verify it on the branch and add "verified1.0.0"?
putterman: do i need to check into MOZILLA_1_0_BRANCH? 
           Is this a different tree than MOZILLA_1_0_0_BRANCH? 

Comment 49

[Yuying Long]

Verified fixed on 05-30 branch build / Win2k-SC(garbled part).