164204 - Need Level 3 IME Support on winxp for MS PinYin v3 ime

Status: VERIFIED FIXED

(Core :: Internationalization, defect, P1)

Description

[Jaime Rodriguez, Jr.]

Need to better support Level 3 IME.

ftang: pls add further information on this issue, based on conversations and
needs of the customer.

Comment 1

[Sam J. Fleet]

Can you provide some more explanation?  It sound like a request for enhancement
although I have no idea what IME is.....  Etc...

Comment 2

[Frank Tang]

Below are info received from 
Ben Morrell Gordon

Here is the bug:
Microsoft has confirmed that the MSPinYin 3.0 IME (WindowsXP) does not properly
respond to the ImmSetCandidate... APIs.

However, I have found that by default MSPinYin IME will set candidate window
position properly, by following cursor/caret pos.

The problem occurs when trying to explicitly set the position of the
caret/cursor in an edit widget after responding to the IMEs SetCursorPos flag
(in a WM_COMPOSITION msg).

The fix (in our case,) was to only honor SetCursorPos flag (in a WM_COMPOSITION
msg), when we were actually inserting text when recieving this message.

The problem appears to be related to an edit widgets management of the
caret/cursor in relation to when the IME try to determine the caret position.

Roy, please take a look at this

Comment 3

[Frank Tang]

roy- any progress?

Comment 4

[Jaime Rodriguez, Jr.]

Marking as topembed+ as this is needed by a major embeddor.

Comment 5

[Roy Yokoyama]

Attachment: simple app to exhibit the ime problem. (zip format)

We tried various combinations to see if we can move the candidate window
to a prefined position. (say x=100, y=200) and checked the position of 
the candidate window by calling ImmGetCandidateWindow() after setting it.

All the test cases we performed, it exhibited SUCCESS and returned the
predefined position despite the fact the Candidate window _is_ always on
the upper-left coner (except in case 2 where it rendered at the center of
the screen)

Test cases involves:
1) WM_IME_NOTIFY, WM_IME_CONTROL and IMC_SETCANDIDATEPOS
2) removing WM_IME_COMPOSITION, WM_IME_ENDCOMPOSITION
3) create Caret, remove Caret
4) MoveWindow(ImmGetDefaultIMEWnd(hWnd));
5) check the position on WM_PAINT

Need help!

Comment 6

[Jaime Rodriguez, Jr.]

roy: from who, and what type of help do you need?

Comment 7

[Roy Yokoyama]

>roy: from who, and what type of help do you need?
from mozilla community and if one can make the attached sample
works with MS PinYin :)

Comment 8

[Roy Yokoyama]

Attachment: remove USE_OVERTHESPOT_IME() and replace it with PINYIN_IME_ON_XP()

gPinYinIMECaretCreated is defined to keep track of Caret Creation
and to avoid the resource leak.

Caret is created on nsWindow::HandleStartComposition()
and destroyed on nsWindow::HandleEndComposition()

shanjian: can you review?

Comment 9

[Roy Yokoyama]

Attachment: oops, wrong attachment

Comment 10

[Shanjian Li]

Comment on attachment 100460 [details] [diff] [review]
oops, wrong attachment

The code looks fine to me. r=shanjian

Comment 11

[Frank Tang]

Comment on attachment 100460 [details] [diff] [review]
oops, wrong attachment

mark r= for shanjian he forgot to do that.,

Comment 12

[Johnny Stenback (:jst)]

Comment on attachment 100460 [details] [diff] [review]
oops, wrong attachment

sr=jst

Comment 13

[Roy Yokoyama]

code checked into the trunk. 
Thanks shanjian and jst

Comment 14

[Jaime Rodriguez, Jr.]

This is needed by a major embedding customer on 1.0 branch asap. 

Benefit: Provides support for PinYin IME Inline support on Windows XP. This
would benefit all customers of the 1.0 branch [embedding customer(s), Mozilla
and Netscape users].

Risk: Minimal -- Would only effect IME on Windows XP. Not other parts of the
code are touched.

Pls provide adt and Drivers' approval, so that we can have this checked in tonight.

Comment 15

[Jaime Rodriguez, Jr.]

Comment on attachment 100460 [details] [diff] [review]
oops, wrong attachment

a=chofmann (verbal approval by phone) for checkin into the 1.0 branch. pls
check this into the 1.0 branch asap, then replace "mozilla1.0.2+" with 
"fixed1.0.2". thanks!

Comment 16

[Frank Tang]

be more presices about the risk
1. it won't impact linux or mac
2. it won't impact win 95,98,,me
3. it will only impact asian input method. so it won't impact any users who do
not use Asian input method
4. it won't impact Japanese, Korean or Traditional Chinese input method 
5. it only change the behavior of ONE particular input method in Chinese (there
are multiple chinese input method and this patch only affect ONE of them)

Comment 17

[Frank Tang]

I think we should take this one. if dan or another adt member agree, then we
should + it. 

Comment 18

[Frank Tang]

all the code in that pach are in the block which related to ime operation. For
normal keyboard input, it won't hit those code at all. 

Comment 19

[Frank Tang]

here are some question I asked roy over the aim:
yungfongta (8:47:13 PM):
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/winui/winui/windowsuserinterface/resources/carets/caretreference/caretfunctions/destroycaret.asp
yungfongta (8:47:38 PM):

DestroyCaret destroys the caret only if a window in the current task owns the
caret. If a window that is not in the current task owns the caret, DestroyCaret
does nothing and returns FALSE.

The system provides one caret per queue. A window should create a caret only
when it has the keyboard focus or is active. The window should destroy the caret
before losing the keyboard focus or becoming inactive.

For an example, see Destroying a Caret

yungfongta (8:47:49 PM): A window should create a caret only when it has the
keyboard focus or is active. The window should destroy the caret before losing
the keyboard focus or becoming inactive.
yungfongta (8:47:56 PM): does your patch handle that ?
yungfongta (8:48:12 PM): destroy the caret before losing the keyboard focus or
becoming inactive.
ROYYOKOYAMA (8:48:53 PM): yes, it was the same question shanjian asked me before
review it. we recei
ROYYOKOYAMA (8:49:17 PM): we receive EndComposition() when we become out of focus
yungfongta (8:52:23 PM): ok, it make sense. I guess that also cover the case
when we switch keyboard layout
ROYYOKOYAMA (8:52:49 PM): yes, it should send EndComposition() too


according to this conversion, I believe the DestroyCaret call have no problem 

Comment 20

[bobj]

Some comments explaining that these changes are workarounds for bugs in the
MS Pinyin IME on XP would help others understand this code later.

Comment 21

[bobj]

> Some comments explaining that these changes are workarounds for bugs in the
> MS Pinyin IME on XP would help others understand this code later.
more clarity:
Some comments explaining that these changes are "workarounds for bugs inline
style support in the MS Pinyin IME on XP" would help others understand this
code later.

Comment 22

[Frank Tang]

The first place it call SetCaretPos is in HandleTextEvent and the 2nd place it
call SetCaret is in HandleCompositionStart . And HandleCompositionStart is
guarantee be called before any HandleTextEvent. That is why the first block it
does not try to create the caret. It don't need to since the 2nd block will do
it for it before it been called. 
The 2nd block do not need to worry about gPinYinIMECaretCreated is true already
because the only time HandleCompositionStart been called is after
HandleCompositionEnd been called, and in HandleCompositionEnd  we always destroy
the caret and turn gPinYinIMECaretCreated  to false.

Comment 23

[Daniel Veditz [:dveditz]]

If this is checked into the trunk shouldn't it be resolved FIXED?

Looks isolated enough to me, adt+

Comment 24

[Roy Yokoyama]

marking as fixed

Comment 25

[Frank Tang]

land the fix into trunk after approval. 

Comment 26

[Frank Tang]

sorry, it should be
land the fix into branch for roy after approval. 

Comment 27

[Kyle Yuan]

Is this a WinXP only problem? I use MS PinYin v3 IME on Windows 2000, the 
candidate window is always located in the top-left corner of the screen.

Comment 28

[Jaime Rodriguez, Jr.]

ruixu: can you pls verify this as fixed on the today's trunk builds? thanks!

Comment 29

[Roy Yokoyama]

>I use MS PinYin v3 IME on Windows 2000
rui: can you try on Win2k as well?  I was under impression that it was only for XP.

Comment 30

[Jaime Rodriguez, Jr.]

ruixu: doh! forgot ... can you pls verify this as fixed on the today's 1.0
branch builds, as well (i.e. First Priority)? thanks!

Comment 31

[Rui Xu]

It is not reproducible with 2002-09-08 branch build on SC WinXP, EN WinXP, JA 
WinXP, JA Win2K, SC WinNT4 and SC Win98SE, and no regression was found.
The candidate window is in correct position as well.

Comment 32

[Rui Xu]

Correction: build number in Comment #31 should be 2002-09-25-08 branch.

Comment 33

[Rui Xu]

Hi Kyle,
I tested MS PinYin IME v3 on Simplified Chinese Win2000, and don't see the problem as mentioned in Comment #27. The default MS PinYin IME installed with Win2000 is v2, and I updated it to v3 with Simplified Chinese Office XP.

Could you please let me know which language version is your Windows 2000? How did you update to MS PinYin IME v3? Is it a standalone version or coming from Office XP? If it's a standalone version, where to get it? If it's from Office XP, which language version is your Office XP? Thanks.

Comment 34

[Kyle Yuan]

Attachment: screen shot

I've updated and built my local tree today, and it still doesn't work well with
my MS PinYin 3.0. I think my MS PinYin IME does come with my OfficeXP + Chinese
Language Package. It's version is 3.0 2516, MS Word's version is 10.2627.2625.

And my language bar doesn't work well with mozilla either. I attached a zip
file that is consist of 3 screen shot gif files which can show how they look
like on my Windows2k.

Comment 35

[bobj]

Commericial branch WFM on US W2K in US region:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.2) Gecko/20020925
Netscape/7.0 

I tried MS Pinyin in Composer, URL bar and in an HTML form, and candidate
window appeared where the cursor was in the editable area.

Comment 36

[Rui Xu]

Verified fixed with trunk 2002-09-26-08.

The problem for the candidate window position of MS PinYin IME v3 in Comment 
#27 is confirmed, and filed bug 171065.