90759 - [PATCH]Global IME-Half width and full width space do not work in composer

Status: VERIFIED FIXED

(Core :: Internationalization, defect, P1)

Description

[Teruko Kobayashi]

Global IME-Half width and full Fidth space do not work in composer.

Steps of reproduce
1. Install Global IME (Japanese) to Win98 US
2. Launch Netscape
3. Start Composer
4. Change Input method to Japanese
5. Select each mode, full width hiragana, full width katakana, full width 
alphabet, half width alphabet, and half width katakana, 
6. Hit space bar

Space is not added.

Tested 7-13-0.9.2 Win32 on Win98 US with Japanese global IME.

Comment 1

[Yuying Long]

Is this bug combine bug 83515 (still open) and bug 42574 (fixed)?

I can reproduce bug 83515 but can not reproduce bug 42574 on Win2k-CN though.

Comment 2

[Teruko Kobayashi]

This bug is little different from 83515 and 42574. Space does not work if IME 
is on no matter any places of Composer.  I tried to regress Japanese IME 2000 
for Win2k, I cannot reproduce it.  I know hankaku space does not work which is 
83515.  

This problem is Global IME. 

Comment 3

[Teruko Kobayashi]

I can reproduce this on Winnt 4.0 US with Japanese Global IME.

Comment 4

[Roy Yokoyama]

accepting and setting the milestone to 0.9.3 to see if I can fix this in time.

Comment 5

[Roy Yokoyama]

Moving milestone to 0.9.4 

Comment 6

[Frank Tang]

I can reproduce this on my NT4J with Native Japanese IME. On NT4J, what will
happen is the space WILL be added but the caret does not move, after I type the
next characters, you will see there are a space insert before the next character. 

Seems important bug. 

Comment 7

[Joe Francis]

i'll try to reproduce this on my mac...

Comment 8

[Roy Yokoyama]

Little observation:
1) Turning OFF IME and insert SPACEs -> single-byte spaces are inserted
2) Turn IME ON and insert SPACEs -> db-byte spaces are inserted.
3) Turn IME ON and insert SPACEs -> show only one single-byte space is inserted.

Comment 9

[Asa Dotzler [:asa]]

0.9.4 is out the door.

Comment 10

[Roy Yokoyama]

Sorry here is the update:
1) Turning OFF IME and insert SPACEs -> single-byte spaces are inserted
2) Turn IME ON and insert DB-BYTE SPACEs -> db-byte spaces are inserted.
3) Turn IME ON and insert 1 byte SPACEs -> shows only one single-byte space is
inserted.

Teruko: can you reproduce this on Linux/Mac? 

Comment 11

[msanz]

roy, aren't your three scenaros the expected result?

Comment 12

[Roy Yokoyama]

msanz:  First two are expected results; but the third one isn't.
I inserted bunch of 1-byte "SPACEs" with IME ON.  
It shows "only one" single-byte space.  Caret doesn't move either.

Comment 13

[msanz]

I see, it should add two spaces... and caret should  move.

Comment 14

[Jaime Rodriguez, Jr.]

Adding correctness Status Whiteboard, correct/expected behavior does not occur.

Comment 15

[selmer (gone)]

PDT-.  No comments indicating anyone has worked on producing a patch for this
bug.  If someone produces a very simple, very short patch in a *very* short
period of time and can get reviews and adequate testing, then I *might* reconsider.

Comment 16

[Roy Yokoyama]

as per our discussion.

Comment 17

[Joe Francis]

Roy, Frank, Kin, others...
After griping that this was going to be very hard and might require a rewrite of 
editor ime handling to fix, I now am more optimistic.  Though it's a fairly big 
deal, I think I can recycle the code that handles this issue on the non-IME 
codepath and get it working on the ime codepath.

Moving milestone to 096 for now because of the amount of time I'll need.  I 
suspect we will have to go through a couple of passes here with some good testing 
in between.

Comment 18

[Joe Francis]

*** Bug 83515 has been marked as a duplicate of this bug. ***

Comment 19

[Joe Francis]

Attachment: patch for editor/libeditor/html/nsHTMLDataTransfer.cpp

Comment 20

[Joe Francis]

ok, so my 10 estimate was way off.  This ended up being really easy.  Woohoo!

Roy, can you please test this out for me?  The IME flavors I have access to all
use full-width spaces.  I need testing on half-width (ie, regular ascii) spaces.
 Thanks!

Comment 21

[Yuying Long]

Add nsbeta1

Comment 22

[Roy Yokoyama]

jfrancis: sorry, the patch didn't make any difference.

Comment 23

[Joe Francis]

fix checked in on trunk

Comment 24

[Roy Yokoyama]

ylong: my test didn't fix the problem; however I may be incorrect.
       can you verify? thanks

Comment 25

[Yuying Long]

Yes, the fix doesn't work for my WinXP-Ja with 10-22-09 trunk build.

Re-open for now...

Comment 26

[Joe Francis]

i dont have any way to know what is going wrong here.  I will have to debug this
with Roy to see why the patch isn't working.

Comment 27

[Joe Francis]

097

Comment 28

[Joe Francis]

098

Comment 29

[Kathleen :Brade]

What is the current state of this bug?  
Are both half-width and full-width spaces not working?
How important is this bug?  Should it still be EDITORBASE?

Comment 30

[Yuying Long]

I checked it on today's trunk build, it's a little difference than the original
report:
When I start a NEW compose page, and change IME to half-width katakana(win98,
nt, 2k, XP), press space key a few times, it stoped at the first one or two
steps, I can not add space properly.
And for win98 and nt, it happens on full width space too.

However, when I first start type something first, then would not have the
problem with add half-width and full width space.

Comment 31

[Joe Francis]

pushing off 098 to 099

Comment 32

[Syd Logan]

plussing

Comment 33

[Joe Francis]

099 to 1.0; set pri to 1 for these pushed off EB+ bugs

Comment 34

[Joe Francis]

I have an nt box with a debug build.  I have Microsoft Global IME 5.0 for 
Japanese installed.  Is this a setup that should exhibit the bug?  I cannot get 
the bug to happen on it, but I don't know if I am properly selecting the modes, 
or even the difference between entering full-width and half-width spaces.  

If this setup should show the bug, can an IME-savvy engineer come give me a hand 
reproducng it?  If I can reproduce it then I have a shot at debugging it...

Comment 35

[Rui Xu]

>> I have an nt box with a debug build.  I have Microsoft Global IME 5.0 for 
>> Japanese installed.  Is this a setup that should exhibit the bug?  
On my US NT4+SP6 with Microsoft JA Global IME 5.02 installed, I can still repro 
this bug with 20020301 trunk build. I beleive it can be reproduced on your 
machine.
I couldn't find you today, I will come by your cube tomorrow again.
BTW, here is a related bug 128562.

Comment 36

[kinmoz]

Hey Joe, so Rui Xu and I sat down at your computer and reproduced the problem.
The good news is that we are entering the spaces into the content tree. The bad
news is that we aren't doing the normal alternating nbsp and space character
deal when it comes to IME ... when the space bar stops moving the caret, we are
adding raw spaces into the content.

In the IME modes we used, typing a space triggers
nsTextEditorTextListener::HandleText() followed immeidately by a
nsTextEditorCompositionListener::HandleEndComposition() don't know if that makes
a difference to the WS handling code.

Comment 37

[Joe Francis]

Attachment: patch to nsEditor.h,cpp and nsWSRunobject.cpp

This took some time to figure out, and once I did I had to think for a while
about the simplest and safest way to fix it.  Here it is.  The short story is
that sometimes the whitespace handling code in editor will adjust whitespace
that is near to (but the exact) text that is inserted.	Under the hood these
adjustments were going through the same text insertion routines that ime does. 
These routines were detecting that ime was in progress and treated these other
text insertions as if they were from IME.  That led to editors internal ime
state becoming invalid.

The easiest fix was to ad a flag to the low level text insertion routine to
force it to treat a given insertion as non-IME.  The whitespace adjusting code
then uses this flag.   Seems to fix this bug.  This will also fix some
unreported bugs involving normal typing that happens to be just before an nbsp,
etc.

Comment 38

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

Comment on attachment 77008 [details] [diff] [review]
patch to nsEditor.h,cpp and nsWSRunobject.cpp

yep ; r=glazman
(although I cannot test ; lack of win98 and stuff)

Comment 39

[kinmoz]

Comment on attachment 77008 [details] [diff] [review]
patch to nsEditor.h,cpp and nsWSRunobject.cpp

sr=[e-mail]

Can we change "noIME" to "suppressIME"? People like me are slow to process
double negatives (!noIME). :-)

Comment 40

[Kevin McCluskey (gone)]

adt1.0.0

Comment 41

[Asa Dotzler [:asa]]

Comment on attachment 77008 [details] [diff] [review]
patch to nsEditor.h,cpp and nsWSRunobject.cpp

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 42

[Frank Tang]

yokoyama, can you test jfrancis's latest patch in your local machine ?

Comment 43

[Jaime Rodriguez, Jr.]

adt1.0.0+ (on ADT's behalf) approval for checkin to 1.0, pending Roy's test
turns out ok.

Comment 44

[Roy Yokoyama]

Verified the patch works.
Thanks, Joe.

Comment 45

[Joe Francis]

fix landed on trunk

Comment 46

[Yuying Long]

Verified fixed on 04-08 trunk build / WinXP-CN while 04-02 was broken.

Comment 47

[Joe Francis]

This landed before the branch was cut.  Hence: fixed1.0.0

Comment 48

[Yuying Long]

verified1.0.0: vefified before 1.0 branch - comment #46.