Build: 1.0 branch and trunk. OS: Windows Under some SC IMEs, e.g. NeiMa, GBK, WuBi, ZhengMa, etc..., cannot enter some double byte puntuation marks, e.g. ",", ".", "?", etc...
It is reproducible on both browser and composer.
what will happen if you try to enter it?
nsbeta1+ roy, please work closely with rui
I have sat down with Ruixu last week. >what will happen if you try to enter it? Nothing got entered. <blank>
We are receiving WM_CHAR message for the double byte punctuations for these IMEs. We dont' receive WM_IME_COMPOSITION nor WM_IME_CHAR msgs.
Created attachment 85529 [details] [diff] [review] Use ::IsDBCSLeadByte(charToConvert) to determine the LeadByte Another hack for Win SC. This time IME bypass the Composition to input the double byte chars. Thus, we need to add a code to see if wParam in WM_CHAR is lead byte or not. This impacts Win NT/XP-SC platform only. I tested using <Alt>+<num> for lead byte input as well. shanjian: what do you think?
roy, the idea looks fine. But you might have to define "waitForTrailingByte" as a member variable of nsWindows. User may open multiple windows. With a global/static variable, we might have problem in certain situation. It might be very rare, but it is possible.
Created attachment 85625 [details] [diff] [review] making waitForTrailingByte as a member of nsWindow and removing un-necessary memset() ruixu: is this bug only on XP-SC? or happens on other platform? I made my patch only to affect on Win2K/XP-SC. Is it enough? shanjian: can you try this patch on your Win2K-SC and review it?
Created attachment 85632 [details] [diff] [review] Oops, I need to follow the naming convention shanjian: can you review?
Comment on attachment 85632 [details] [diff] [review] Oops, I need to follow the naming convention r=shanjian, I tried this patch on win2kSc, it works fine.
> ruixu: is this bug only on XP-SC? or happens on other platform? > I made my patch only to affect on Win2K/XP-SC. Is it enough? No, It also happens on non-NT based Windows, but maybe we can try your patch 1st to see if still reproducible or not.
Created attachment 85797 [details] IME msg test app ruixu: can you run attached test2.exe app in your various non-NT based Windows and post the result? Here is the sample result I expect when you enter the _double-byte punctuation marks_ using NeiMa and other IMEs: OS | windows message -----+--------------------------------------- XP | WM_CHAR 2K | WM_CHAR NT | ??????? 9x | ??????? Me | ??????? ( msg should be one of followings: WM_CHAR, WM_IME_CHAR, WM_IME_COMPOSITION) Please focus on _double-byte punctuation marks_ only. Thanks
1. I think your patch should not only take effect on simp chinese window. 2. since you are changeing the size of the buffer from 2 to 3, you should change it to 5 because GB18030 could be 4 bytes.
quick report from ruixu: OS | windows message -----+--------------------------------------- NT | WM_IME_COMPOSITION 9x | WM_IME_COMPOSITION Me | WM_CHAR Above are all tested in native Chinese Simpified Windows. It looks like I have to remove the OS check for this code.
Oops lost some... http://www.microsoft.com/downloads/release.asp?ReleaseID=32385
Comment on attachment 85632 [details] [diff] [review] Oops, I need to follow the naming convention PRBool and BOOL are mixed in no coherent way that I can see, but if you use PRBool, maybe the many PRBool members (12 that I can see) can be packed adjacently via PRPackedBool, to save a few words (9 words saved by packing 12 booleans into 3 words at 4 bytes per word). firstname.lastname@example.org /be
roy- do you need to produce another patch?
>roy- do you need to produce another patch? I am investigating on the GB18030 input. It seems Windows is sending WM_IME_COMPOSITION msgs on my XP-SC. So theoretically my patch should have no impact and I don't need to increase the buffer size to 5 However, there is a need for supporting WinMe where it sends WM_CHAR messages for these punctuation marks. To sum up, I need to remove the OS check out from the patch + if (nsToolkit::mW2KXP_CP936)
Just FYI: WinXP is supporting 2 type IMEs, the classic IME and new text frame IME. It might be better to consider the fix for both. Thanks!
>new text frame IME. It might be better to consider the fix for both. Thanks! ruixu: If you are talking about the Text Framework Service support, then we won't be able to do it with a simple patch. I agree with you that we need to support TSF (bug 88831) in near future. I believe, however though, it shouldn't interfere with this patch since TFS has its own APIs.
Created attachment 86335 [details] [diff] [review] Remove nmW2KXP_CP936 This patch is essentially the same as before except removing the mW2KXP_CP936 check
Created attachment 86341 [details] [diff] [review] Changing to PRPackedBool This should save some bytes out of every widget we create. :) I LXR'ed to see if we pass PRBool members by pointers; but they all used in assignment and _if_ comparison. brendan: do you care to super review? Thanks
On my XP (En XP + multi lang UI), using MSPY, I can't enter "?" and "!". I'm using 06/04 1.0.0 branch build.
For above problem， I can reproduce on native Simplified Chinese XP (06/05 branch build) with DOUBLE BYTE question mark and quotation.
Yes, I see the problem on my native XP. I see WM_IME_CHAR msg with proper CP936 code point and converted to correct unicode point. '!' 0xa3 0xa1 ---> 0xff01 FULL WITH EXCLAMATION MARK '?' 0xa3 0xbf ---> 0xff1f FULL WITH QUESTION MARK and then pass it to DOM NS_KEY_PRESS event. Both looks ok. Not sure why we can't support this. Investigating..
I filed a new bug 149397 for "?" and "!". Bug looks very similar to this bug; but the code execution path is different. I am going to check this patch into the trunk since patch really fixes the punctuation issue.
Comment on attachment 86341 [details] [diff] [review] Changing to PRPackedBool r=ftang
[adt2] since it make users cannot input commonly / daily used characters.
ftang: thanks for the extra review. brendan already gave /sr on the patch which is a minor modification from the previously approved patch. checked into the trunk
I tested with 2002061204 trunk on SC WinXP, only MS PinYin IME3.0 works fine, all other SC IMEs supported on SC WinXP doesn't work properly, still cannot enter puntuation marks, but entered some other characters when using them.
>MS PinYin IME3.0 works fine Good. PinYin is the one uses the Over-the-Spot processing (which I modified recently) However, other IMEs should behave like before. Before means NS6.x Were we able to enter those chars in NS6.x?
NS6.23 has the same issue as this original report.
Created attachment 87732 [details] [diff] [review] Process double byte char in WM_CHAR msg Those IMEs send 2xWM_CHAR messages to enter punctuation. (THEY SHOULD SEND WM_IME_CHAR, ACCORDING TO MS DOCUMENT) We had ::IsDBCSLeadByte() in place in nsWindow::OnChar; but we never dealt with such case. /shanjian: can you review?
Comment on attachment 87732 [details] [diff] [review] Process double byte char in WM_CHAR msg r=shanjian
should these bytes stored in member data of nsWindow instead of static ? static won't be thread safe, right?
I thought of the same; but I am not sure if we needed it in this case. Adding extra 3 bytes in all nsWindow objects seems too much for this and I can't think of user switching between different threads while entering some text. Plus I am pretty sure nsWindow is not thread-safe. (I remeber asking somebody in IRC; but I could be wrong.)
Created attachment 88168 [details] [diff] [review] Add new member data nsWindow::mLeadByte to maintain the lead-byte Implementing frank's recommendation after discussing with him. shanjian: can you review again?
roy, you can further simplify the code. mIMEWaitForTrailingByte can be removed if you add mLeadByte there.
Created attachment 88180 [details] [diff] [review] Removing mIMEWaitForTrailingByte thanks, shanjian.
Comment on attachment 88180 [details] [diff] [review] Removing mIMEWaitForTrailingByte r=shanjian
shaver: can you super review? (brendan is out of office)
Comment on attachment 88180 [details] [diff] [review] Removing mIMEWaitForTrailingByte email@example.com Can we add some sort of comment in OnChar() that says what this code does? The mIMEWaitForTrailingByte that was there before kind of gave a hint that it was IME related, but now that there is no mention of IME in mLeadByte perhaps a comment would be good. Also, there seems to be some sort of tabbing convention used between the type and member name in nsWindow.h, so you might want to fix that up.
have this been land into trunk ? once you do that, mark this bug as FIXED.
landed in the trunk yesterday
Verified on trunk 2002062008, it has been fixed on SC and JA Windows. It is still reproducible on English Windows, it is separated as a different issue in bug 153470.
please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+" keyword and add the "fixed1.0.1" keyword.
Adding adt1.0.1+ on behalf of the adt for checkin to the 1.0 branch. Please get drivers approval before checking in. When you check this into the branch, please change the mozilla1.0.1+ keyword to fixed1.0.1
checked into the 1.0 branch
Verified on branch 2002062408.