Cannot enter some puntuation marks with some SC IMEs on JA and SC Windows

VERIFIED FIXED in mozilla1.0.1

Status

()

Core
Internationalization
--
major
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: Rui Xu, Assigned: Roy Yokoyama)

Tracking

({intl})

Trunk
mozilla1.0.1
x86
Windows XP
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [adt2])

Attachments

(2 attachments, 7 obsolete attachments)

(Reporter)

Description

16 years ago
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...
(Reporter)

Comment 1

16 years ago
It is reproducible on both browser and composer.
Keywords: intl, nsbeta1
(Assignee)

Updated

16 years ago
Status: NEW → ASSIGNED

Comment 2

16 years ago
what will happen if you try to enter it?

Comment 3

16 years ago
nsbeta1+
roy, please work closely with rui
Blocks: 141008
Keywords: nsbeta1 → nsbeta1+
(Assignee)

Comment 4

16 years ago
I have sat down with Ruixu last week.

>what will happen if you try to enter it?
Nothing got entered. <blank>
(Assignee)

Comment 5

16 years ago
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.
Target Milestone: --- → mozilla1.0.1
(Assignee)

Comment 6

16 years ago
Created attachment 85529 [details] [diff] [review]
Use ::IsDBCSLeadByte(charToConvert[0]) 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?

Comment 7

16 years ago
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.
(Assignee)

Comment 8

16 years ago
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?
Attachment #85529 - Attachment is obsolete: true
(Assignee)

Comment 9

16 years ago
Created attachment 85632 [details] [diff] [review]
Oops, I need to follow the naming convention

shanjian: can you review?
Attachment #85625 - Attachment is obsolete: true

Comment 10

16 years ago
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.
Attachment #85632 - Flags: review+
(Reporter)

Comment 11

16 years ago
> 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.
(Assignee)

Comment 12

16 years ago
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

Comment 13

16 years ago
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. 
(Assignee)

Comment 14

16 years ago
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.
(Assignee)

Comment 15

16 years ago
references to GB18030

http://www.microsoft.com/downloads/release.asp?
http://warp/u/ftang/utf8test/gb18030.cgi
http://www.i18nwithvb.com/surrogate_ime/
http://www.i18nwithvb.com/surrogate_ime/code_charts/
(Assignee)

Comment 16

16 years ago
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).

sr=brendan@mozilla.org

/be
Attachment #85632 - Flags: superreview+

Comment 18

16 years ago
roy- do you need to produce another patch?
(Assignee)

Comment 19

16 years ago
>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) 
(Reporter)

Comment 20

16 years ago
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!
(Assignee)

Comment 21

16 years ago
>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.
(Assignee)

Comment 22

16 years ago
Created attachment 86335 [details] [diff] [review]
Remove  nmW2KXP_CP936

This patch is essentially the same as before except 
removing the mW2KXP_CP936 check
Attachment #85797 - Attachment is obsolete: true
(Assignee)

Comment 23

16 years ago
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
Attachment #86335 - Attachment is obsolete: true

Comment 24

16 years ago
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.

Comment 25

16 years ago
For above problem&#65292; I can reproduce on native Simplified Chinese XP (06/05
branch build) with DOUBLE BYTE question mark and quotation. 
(Assignee)

Comment 26

16 years ago
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..
(Assignee)

Updated

16 years ago
Attachment #85797 - Attachment is obsolete: false
(Assignee)

Comment 27

16 years ago
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 28

16 years ago
Comment on attachment 86341 [details] [diff] [review]
Changing to PRPackedBool

r=ftang
Attachment #86341 - Flags: review+

Comment 29

16 years ago
[adt2] since it make users cannot input commonly / daily used characters. 
Whiteboard: [adt2]
(Assignee)

Comment 30

16 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Reporter)

Comment 31

16 years ago
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. 
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 32

16 years ago
>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?
(Reporter)

Comment 33

16 years ago
NS6.23 has the same issue as this original report.
(Assignee)

Comment 34

16 years ago
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?
Attachment #85632 - Attachment is obsolete: true
Attachment #86341 - Attachment is obsolete: true

Comment 35

16 years ago
Comment on attachment 87732 [details] [diff] [review]
Process double byte char in WM_CHAR msg

r=shanjian
Attachment #87732 - Flags: review+

Comment 36

16 years ago
should these bytes stored in member data of nsWindow instead of static ? static
won't be thread safe, right?
(Assignee)

Comment 37

16 years ago
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.)
(Assignee)

Comment 38

16 years ago
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?
Attachment #87732 - Attachment is obsolete: true

Comment 39

16 years ago
roy, you can further simplify the code. mIMEWaitForTrailingByte can be removed
if you add mLeadByte there. 

(Assignee)

Comment 40

16 years ago
Created attachment 88180 [details] [diff] [review]
Removing mIMEWaitForTrailingByte

thanks, shanjian.
Attachment #88168 - Attachment is obsolete: true

Comment 41

16 years ago
Comment on attachment 88180 [details] [diff] [review]
Removing mIMEWaitForTrailingByte

r=shanjian
Attachment #88180 - Flags: review+
(Assignee)

Comment 42

16 years ago
shaver: can you super review? (brendan is out of office)
Whiteboard: [adt2] → [adt2] [Need super review]

Comment 43

16 years ago
Comment on attachment 88180 [details] [diff] [review]
Removing mIMEWaitForTrailingByte

sr=kin@netscape.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.
Attachment #88180 - Flags: superreview+

Comment 44

16 years ago
have this been land into trunk ? once you do that, mark this bug as FIXED.
(Assignee)

Comment 45

16 years ago
landed in the trunk yesterday
Status: REOPENED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED
(Reporter)

Comment 46

16 years ago
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.
Status: RESOLVED → VERIFIED
Summary: Cannot enter some puntuation marks under some SC IMEs → Cannot enter some puntuation marks with some SC IMEs on JA and SC Windows

Updated

16 years ago
Keywords: adt1.0.1, mozilla1.0.1
Whiteboard: [adt2] [Need super review] → [adt2]

Updated

16 years ago
Attachment #88180 - Flags: approval+

Comment 47

16 years ago
please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.
Keywords: mozilla1.0.1 → mozilla1.0.1+

Comment 48

16 years ago
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
Keywords: adt1.0.1 → adt1.0.1+
(Assignee)

Comment 49

16 years ago
checked into the 1.0 branch
Keywords: mozilla1.0.1+ → fixed1.0.1

Updated

16 years ago
Blocks: 146292
No longer blocks: 141008
(Reporter)

Comment 50

16 years ago
Verified on branch 2002062408.
Keywords: fixed1.0.1 → verified1.0.1
You need to log in before you can comment on or make changes to this bug.