180.92 KB, image/jpeg
132.06 KB, image/jpeg
5.31 KB, patch
Shanjian Li: review+
Dawn Endico: approval+
|Details | Diff | Splinter Review|
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.
Created attachment 81521 [details] 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.
Created attachment 81522 [details] Screen shot on 04-23 RC1 branch build 04-23 RC1 build worked fine with both IME.
This is a very bad regression, also it happened very recently -> nsbeta1.
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.
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.
Not reproducible on WinME-JA on both 04-29 branch build and 04-25 trunk build.
On same WinXP-SimpChinese system, Traditional Chinese IME is corrupted also.
look at 92491. This looks like one of the suspect.
Ja-IME and Korean-IME in Win2000-Ja is fine.
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.
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?
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 -------------------------------------------
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 .
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....
This could also be intrdouced by 04/24/2002 17:24dbaron%fas.harvard.edu checkin on b=5693
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.
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.
add email@example.com to the cc list
it is bug 5693, not 5603.
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
how about mac?
mac work fine with 0429 branch build still not sure which patch cause the regression.
I have a old tree build on 04/04, and I could reproduce the problem there.
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.
I tested with W2K-ja using SC-IMEs and works fine. So I suspect my patch (bug 125573) may have caused the regression. :(
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
I think problem 2 is an adt1 bug though, since Simplified Chinese Windows XP is our 1st platform on the QA list.
Bug 141513 filed for the candidate window problem.
Created attachment 81971 [details] [diff] [review] 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)
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.
I have win2k SC, but not a XP SC. If that's ok, let me know.
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. :)
I tried your patch on win2k Sc, I could not see any problem.
Created attachment 82114 [details] [diff] [review] 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 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 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
> 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?
static PRBool mUseImeApiW; + static PRBool mW2KXP_CP936; Would it be better to make these PRPackedBool?
Comment on attachment 82114 [details] [diff] [review] Add nsToolkit::mW2KXP_CP936 firstname.lastname@example.org As dean points out, people should consider using PRPackedBool whenever possible.
This variable exist only for purpose of minimize potential risk. It will be removed in near future.
checked in for roy to trunk.
Fix(garbled part) verified on 05-09 trunk build / WinXP-SC.
adding adt1.0.0+. Please get drivers approval and then checkin to the 1.0 branch.
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.
Checked into the 1.0 branch
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.
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?
Verified fixed on 05-30 branch build / Win2k-SC(garbled part).