Gecko shouldn't draw composition string at ChangJie (Traditional Chinese IME)

RESOLVED FIXED in mozilla1.9.2a1

Status

()

Core
Widget: Win32
P2
normal
RESOLVED FIXED
16 years ago
8 years ago

People

(Reporter: 石庭豐 (Seak, Teng-Fong), Assigned: masayuki)

Tracking

({inputmethod})

Trunk
mozilla1.9.2a1
x86
Windows 2000
inputmethod
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 15 obsolete attachments)

4.18 KB, image/gif
Details
4.78 KB, image/gif
Details
26.74 KB, patch
Details | Diff | Splinter Review
(Reporter)

Description

16 years ago
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.6) Gecko/20011120
BuildID:    20011120

When Global IME is in use in Mozilla, its "appearance" is different from when
it's used in other applications.  The first image (IME&Mozilla.gif) shows when I
input traditional Chinese using Global IME (provided in Win2k) in ChangJie
method together with Mozilla.  The second image (IME&Word.gif) shows how it
looks like in other applications like Word or ICQ.

The difference (if it isn't clear) is that, when used with other applications, a
"floating character-combination window" (I don't know how to call that!) is used
where "radicals" are shown inside before combining into a real character,
whereas in Mozilla, the "radicals" are put directly in the text.  Maybe there's
a difference when using IMM (input method manager)?  Don't worry, there isn't
any big problem with this behaviour (at least I haven't come across any), except
it's a matter of cosmetics and convenience, so it isn't a big issue, but I'd
still like to see floating combination window be used if possible :-)

BTW, I suppose Global IME 5.0 would behave the same.

Reproducible: Always
(Reporter)

Comment 1

16 years ago
Created attachment 60305 [details]
When used with Mozilla
(Reporter)

Comment 2

16 years ago
Created attachment 60306 [details]
When used with other applications
(Reporter)

Comment 3

16 years ago
Created attachment 60307 [details]
When used with Mozilla
reassign to the right component
Assignee: ducarroz → yokoyama
Component: Composition → Internationalization
Product: MailNews → Browser
QA Contact: sheelar → teruko
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 5

16 years ago
Created attachment 60324 [details]
When used with Mozilla
(Reporter)

Comment 6

16 years ago
I don't know why every attachment failed!

Comment 7

16 years ago
seak; how are you attaching it?  did you select
the correct Content Type? 
Status: NEW → ASSIGNED

Comment 8

16 years ago
I cannot see any attachments.

Updated

16 years ago
Priority: -- → P4
Target Milestone: --- → mozilla0.9.9
(Reporter)

Comment 9

16 years ago
Sure!  I selected image/gif.  I'm going to try once again but this time I'll try
"automatic".
(Reporter)

Comment 10

16 years ago
Created attachment 60625 [details]
When used with Mozilla

Comment 11

16 years ago
Created attachment 60637 [details] [diff] [review]
IME and Mozilla

Got an email from Fong, let's see if this works.
Attachment #60305 - Attachment is obsolete: true
Attachment #60306 - Attachment is obsolete: true
Attachment #60307 - Attachment is obsolete: true
Attachment #60324 - Attachment is obsolete: true
Attachment #60625 - Attachment is obsolete: true

Comment 12

16 years ago
Created attachment 60638 [details]
Mozilla IME

Ahhh, my bad.  try again.
Attachment #60637 - Attachment is obsolete: true

Comment 13

16 years ago
Created attachment 60639 [details]
Word IME

Comment 14

16 years ago
yea, that is because we support on-the-spot style instead of off the spot style. 
should we have an option for that ?
future it. 
Target Milestone: mozilla0.9.9 → Future
(Reporter)

Comment 15

16 years ago
Yeah, please.
On the other hand, have I mentioned the inconvience with this on-the-spot style?
when I'm almost at the end of a line and when I input radicals, some of the
radicals get printed at the beginning of the next line.  If this happens in the
middle of a sentence, that's quite annoying.

By the way, I came across the following problem, but I don't know if it's
related to this bug:
after a word is inputted, the cursor is freezed on the current position, ie,
arrow keys don't respond.  In this situation, I have to press ESC once and
everything is then OK.  I don't have this in off-the-spot styled situation.
(Reporter)

Comment 16

16 years ago
For the second problem in my last comment, I've filed a separate bug as bug 143390

Comment 17

16 years ago
Seak: adding an option to have over-the-spot ON/OFF is easy to do.  
      Where do we want to put the checkbox in Pref dlgbox?

Peter: can you cc someone for the UI recommendation?
Target Milestone: Future → mozilla1.2beta
(Reporter)

Comment 18

16 years ago
Since there's no "General" node in the preferences tree (category), can I ask
for creating a new node as "Advanced -> IME" ?

Comment 19

16 years ago
cc lorikaplan for UE input

Comment 20

16 years ago
lorikaplan? any thoughts?

Comment 21

15 years ago
bulk milestone change
Target Milestone: mozilla1.2beta → mozilla1.3alpha
(Reporter)

Comment 22

13 years ago
(In reply to comment #17)
> Seak: adding an option to have over-the-spot ON/OFF is easy to do.  
>       Where do we want to put the checkbox in Pref dlgbox?

Roy, you've talked about adding this option.  Is the option available now?  I'm
using FF & TB now, hope the option wouldn't be just implemented in Mozilla only.

OTOH, I've found this URL about IME in WinXP.  It's not very technical, but I
hope it can be useful to you:
http://www.microsoft.com/technet/prodtechnol/winxppro/evaluate/muiovw.mspx
We are drawing the composition string of IME ourselves. If we use IME owning window for composition string, then, we cannot support some our features. E.g., -moz-transform: rotate();, non-solid color elements.

And TSF (Text Service Framework, Windows new framework for IME and etc.) supporting applications must draw the composition string themselves. So, we should not change the current behavior.

marking to WONTFIX
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → WONTFIX
(Reporter)

Comment 24

9 years ago
After seven years of discussion, and it's closed as "won't fix".  Great ....

As a simple user, I really don't care if the codes are drawn in-line or out-line, in case it works.  But the problem is your method presents bugs.  What could I say more?
Ah, I misunderstood this report. I understood the IME owning window is used only on some applications which don't draw the composition string themselves. E.g., wordpad, notepad and win32 native text input controls. However, the IME owing window is displayed on IE too. IE drawing composition string itself. So, for checking the reason, I reopen this bug. (But I'm not sure this bug should be fixed.)
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Target Milestone: mozilla1.3alpha → ---
We do no longer support Global IME (dropped by bug 330276). Does this bug still occur when using IMM32 IME or TSF TIP?
This bug can be reproduced with ChangJie (traditional Chinese) only on Win2k and WinXP. I cannot reproduce this bug on Vista.
hmm.... The property value of ChanJie on WinXP is 0xC00E. I.e., IME_PROP_CANDLIST_START_FROM_1 and IME_PROP_UNICODE. So, it doesn't return IME_PROP_SPECIAL_UI...
::ImmGetCompositionWindow returns S_OK and the COMPOSITIONFORM::dwStyle is zero (CFS_DEFAULT). Looks like we should not draw composition string ourselves then. I'll check other IME's behaviors.
Status: REOPENED → NEW
Summary: Wish: use "floating character-combination window" in Global IME → Don't draw composition string by Gecko at ChangJie
Assignee: tetsuroy → nobody
Component: Internationalization → Widget: Win32
QA Contact: teruko → win32
Summary: Don't draw composition string by Gecko at ChangJie → Don't draw composition string by Gecko at ChangJie (Traditional Chinese IME)
Summary: Don't draw composition string by Gecko at ChangJie (Traditional Chinese IME) → Gecko shouldn't draw composition string at ChangJie (Traditional Chinese IME)
(In reply to comment #29)
> ::ImmGetCompositionWindow returns S_OK and the COMPOSITIONFORM::dwStyle is zero
> (CFS_DEFAULT).

oops, this is wrong. ::ImmGetCompositionWindow returns BOOL value, not HRESULT.
http://www.microsoft.com/globaldev/handson/user/ime_paper.mspx

I found this document, both Chinese IMEs are using "Reading Window". But Japanese and Korean are using inline style. Maybe, we should not draw composition string ourselves if current keyboard layout is Simplified or Traditional Chinese.
(Reporter)

Comment 32

9 years ago
Could we/you just NOT do anything at all concerning IME?  I mean, when I use Visual C++ or wxDev-C++ to make a very simple program with some input field, it's not necessary to add code to make IME work.  The O/S is able to do it all alone.
(In reply to comment #32)
> Could we/you just NOT do anything at all concerning IME?  I mean, when I use
> Visual C++ or wxDev-C++ to make a very simple program with some input field,
> it's not necessary to add code to make IME work.  The O/S is able to do it all
> alone.

Yes, the "NOT do anything" is a way for to fix this bug. However, it should not be good for Japanese and Korean IMEs. Therefore, we need to check the keyabord layout whether it is for Chinese or not.

I'm creating a patch, but the patch breaks MSPinYin behavior on Vista...
Depends on: 88831
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Created attachment 357423 [details] [diff] [review]
Patch v0.8 (work in progress)
Created attachment 357425 [details] [diff] [review]
Patch v0.8 (work in progress)

oops, the previous file was failed to merge...
Attachment #357423 - Attachment is obsolete: true
The patch also fixes Intelligent ABC's candidate window position problem on Vista.
(Reporter)

Comment 37

9 years ago
(In reply to comment #33)
> Yes, the "NOT do anything" is a way for to fix this bug. However, it should not
> be good for Japanese and Korean IMEs.

Not good?  How is it not good?  I'd like to do some test for you if you could tell me which IME's you're using and what behaviour you're expecting.
(In reply to comment #37)
> (In reply to comment #33)
> > Yes, the "NOT do anything" is a way for to fix this bug. However, it should not
> > be good for Japanese and Korean IMEs.
> 
> Not good?  How is it not good?  I'd like to do some test for you if you could
> tell me which IME's you're using and what behaviour you're expecting.

The composition string of Japanese IMEs and Korean IMEs should be displayed inline. For painting it perfectly (same font, same font style and also adjusting to correct position), we need to paint it ourselves.
Created attachment 357646 [details] [diff] [review]
Patch v1.0

O.K. This patch works fine for me excepting the font size/style issue. The issue should be separated to another bug. Because it is not important issue and it needs more works in XP level.

This patch provides a way which users can switch IME composition string drawing in prefs. The name is "intl.ime.cpXXX.imm32.use_native_composition_window", so, we can specify whether the all IMEs of a code page should be drawn by us or IME. The reason why it is per code page is I don't have good idea to specify per IME. And most users may use only one IME per code page. So, I think "per code page" is enough for most users.

If the pref is true, the composition string is drawn by IME. Then, we set the font of composition window to specified by "intl.ime.cpXXX.imm32.native_composition_font". I think that there are two ways to choose the font other than the using prefs. One is using first font of the current frame. Other is using system default font. However, both ways don't guarantee to be able to draw all composing characters by the font. So, the pref ways is best choice I think.

This patch always create a native hidden caret to our caret position. By this behavior, some Chinese IMEs which honor the native caret works fine for me. This change removes hacky code which checks registry key value of current keyboard layout.

And this patch uses position of target selection first character for candidate window position. We are always use caret position currently, but it is different behavior with MS-IME. So, this is improves the native IME behavior emulation.

I'll post test builds by tryserver.
Attachment #357425 - Attachment is obsolete: true
Attachment #357646 - Flags: review?(VYV03354)
Of course, the patch should not be landed to Fx3.1, it's too late.
Comment on attachment 357646 [details] [diff] [review]
Patch v1.0

You should use
mIMEProperty & IME_PROP_SPECIAL_UI || !(mIMEProperty & IME_PROP_AT_CARET)
instead of hacky guessing from the codepage.
Created attachment 357714 [details] [diff] [review]
Patch v2.0

Great! The condition may be same as IE.
Attachment #357646 - Attachment is obsolete: true
Attachment #357714 - Flags: review?(VYV03354)
Attachment #357646 - Flags: review?(VYV03354)
Created attachment 357724 [details] [diff] [review]
Patch v2.1

I found a bug. ImmGetCompositionFont always fails with Sogou (Simplified Chinese).
Attachment #357714 - Attachment is obsolete: true
Attachment #357724 - Flags: review?(VYV03354)
Attachment #357714 - Flags: review?(VYV03354)
Created attachment 357729 [details] [diff] [review]
Patch v2.2

Oh, and also Sogou returns wrong cursor position which is real cursor position in the composition string, but Sogou returns empty string when we get composition string at IME composition window using mode.

Then, nsWindow::GetTextRangeList puts an assertion by the illegal cursor position. And we don't need to call it at this time.

This patch doesn't change any actual behaviors from previous patch.
Attachment #357724 - Attachment is obsolete: true
Attachment #357729 - Flags: review?(VYV03354)
Attachment #357724 - Flags: review?(VYV03354)
Comment on attachment 357724 [details] [diff] [review]
Patch v2.1

In the first place, you don't have to set the font of the composition window because it will not be displayed at the caret. The IME itself will know the most appropreate font. At leaset Notepad and IE didn't set the font on Win2k.
test build (Patch v2.0)
https://build.mozilla.org/tryserver-builds/2009-01-19_12:10-masayuki@d-toybox.com-attachment357714 [review]/
(In reply to comment #45)
> (From update of attachment 357724 [details] [diff] [review])
> In the first place, you don't have to set the font of the composition window
> because it will not be displayed at the caret. The IME itself will know the
> most appropreate font. At leaset Notepad and IE didn't set the font on Win2k.

No, MSPinYin of Vista doesn't know the good font. It looks like it uses system default font. However, by comment 41, MSPinYin doesn't use composition window now. I'm checking other IMEs...
(In reply to comment #47)
> No, MSPinYin of Vista doesn't know the good font. It looks like it uses system
> default font. However, by comment 41, MSPinYin doesn't use composition window
> now.
I thought about IMEs which should draw composition window themselves like ChangJie because it's clear that we don't have to set the font of the native compossition window when we draw the composition string ourselves.
> I'm checking other IMEs...
Of course I'll withdraw a comment if you find a bad IME.
Created attachment 357733 [details] [diff] [review]
Patch v2.3

O.K. I don't find such bad IME.
Attachment #357729 - Attachment is obsolete: true
Attachment #357729 - Flags: review?(VYV03354)
Attachment #357733 - Flags: review?(VYV03354)
test build (patch v2.3)
https://build.mozilla.org/tryserver-builds/2009-01-19_14:55-masayuki@d-toybox.com-attachment357733 [review]/
Comment on attachment 357733 [details] [diff] [review]
Patch v2.3

After the patch applied, query selection/character/caret event will be called much more times than now. I have a little concern about performance. Could you reduce event count or explain why it is not a problem?
> -  return result;
> +  return ShouldDrawCompositionStringOurselves() ? result : PR_FALSE;
You can return PR_TRUE to suppress extra WM_IME_CHAR messages when you obtained a committed string from ImmGetCompositionString even if Gecko do not draw the composition string itself.
> +  static PRInt32    sIMECommittedCharCount;
Then this will not be required anymore.
(In reply to comment #51)
> (From update of attachment 357733 [details] [diff] [review])
> After the patch applied, query selection/character/caret event will be called
> much more times than now. I have a little concern about performance. Could you
> reduce event count or explain why it is not a problem?

Basically, I don't worry the performance. If it has performance problem, the cause is a performance problem of generating "flat text content". If so, we should redesign nsQueryContentEventHandler (E.g., cache the content until the contents are changed).

# probably, roc doesn't worry it too.

> > -  return result;
> > +  return ShouldDrawCompositionStringOurselves() ? result : PR_FALSE;
> You can return PR_TRUE to suppress extra WM_IME_CHAR messages when you obtained
> a committed string from ImmGetCompositionString even if Gecko do not draw the
> composition string itself.

I though so too. However, if the param of nsWindow::OnIMEComposition is not just commit state or composing state, i.e., the param has both states, the approach fails. I think that the current way is safer.

If the approach is safe, nsWindow::OnIMEComposition can be simpler (we can separate the code to a commit handler and a composing handler. I'm not sure whether there are such IMEs. But if I changed nsWindow::OnIMEComposition logic to simpler code, it seems that it is pretty risky thing.
(In reply to comment #52)
> Basically, I don't worry the performance. If it has performance problem, the
> cause is a performance problem of generating "flat text content". If so, we
> should redesign nsQueryContentEventHandler (E.g., cache the content until the
> contents are changed).
I see.
> I though so too. However, if the param of nsWindow::OnIMEComposition is not
> just commit state or composing state, i.e., the param has both states, the
> approach fails. I think that the current way is safer.
Well, what about simply removing the WM_IME_CHAR handling? It had not been used in effect. If someone send WM_IME_CHAR during count down of sIMECommittedCharCount, count will mismatch. And I suspect WM_IME_CHAR handler was a cause of "double commited string" bugs.
> If the approach is safe, nsWindow::OnIMEComposition can be simpler (we can
> separate the code to a commit handler and a composing handler. I'm not sure
> whether there are such IMEs. But if I changed nsWindow::OnIMEComposition logic
> to simpler code, it seems that it is pretty risky thing.
I think such a big change is out of scope of this bug.
(In reply to comment #53)
> Well, what about simply removing the WM_IME_CHAR handling? It had not been used
> in effect. If someone send WM_IME_CHAR during count down of
> sIMECommittedCharCount, count will mismatch. And I suspect WM_IME_CHAR handler
> was a cause of "double commited string" bugs.

Ah, good idea.

> > If the approach is safe, nsWindow::OnIMEComposition can be simpler (we can
> > separate the code to a commit handler and a composing handler. I'm not sure
> > whether there are such IMEs. But if I changed nsWindow::OnIMEComposition logic
> > to simpler code, it seems that it is pretty risky thing.
> I think such a big change is out of scope of this bug.

Yes. I just said an example of the risk of the way.
Created attachment 357794 [details] [diff] [review]
Patch v2.4
Attachment #357733 - Attachment is obsolete: true
Attachment #357794 - Flags: review?(VYV03354)
Attachment #357733 - Flags: review?(VYV03354)
Comment on attachment 357794 [details] [diff] [review]
Patch v2.4

Ok, ChangJie now displays a reading window on Win2k.
Attachment #357794 - Flags: review?(VYV03354) → review+
Attachment #357794 - Flags: review?(emaijala)
Comment on attachment 357794 [details] [diff] [review]
Patch v2.4

Thank you, Kimura-san.
Ere, would you review the patch?
Comment on attachment 357794 [details] [diff] [review]
Patch v2.4

Roc:

Would you sr+ for this?

# Can I land this patch without Ere's review? We don't have any replay from Ere... And the most part of the patch was reviewed enough by Kimura-san.
Attachment #357794 - Flags: superreview?(roc)
Comment on attachment 357794 [details] [diff] [review]
Patch v2.4

pending the reviews for TSF patch landing.
Attachment #357794 - Flags: superreview?(roc)
Attachment #357794 - Flags: review?(emaijala)

Comment 61

9 years ago
I usually get to reviews faster than other bugmail, so flagging a request is a good idea. 

I'd trust Kimura-san on this as I'm not an IME expert anyway. Just a couple of nits about comments:

compositon, compostion should be composition everywhere.

comitted => committed

// clear to the default window proc so it will draw the candidcate window for
=> // directly to the default window proc so it will draw the candidate window for
(if that's what tries to say)
Created attachment 363540 [details] [diff] [review]
Patch v2.4.1

updated for latest trunk. and fixed the wrong comments which were checked by Ere.
Attachment #357794 - Attachment is obsolete: true
Attachment #363540 - Flags: review?(emaijala)
Priority: P4 → P2
Severity: trivial → normal

Comment 63

9 years ago
Comment on attachment 363540 [details] [diff] [review]
Patch v2.4.1

It's ok, but please really fix "comitted" to "committed" before committing the patch.
Attachment #363540 - Flags: review?(emaijala) → review+
Created attachment 363913 [details] [diff] [review]
Patch v2.4.2

Thank you, Ere!
# I'll find a spell checker for my editor :-(
Attachment #363540 - Attachment is obsolete: true
Attachment #363913 - Flags: superreview?(roc)
roc, would you sr the patch?
Attachment #363913 - Flags: superreview?(roc) → superreview+
Comment on attachment 363913 [details] [diff] [review]
Patch v2.4.2

+  if (ShouldDrawCompositionStringOurselves())
+    aISC &= ~ISC_SHOWUICOMPOSITIONWINDOW;

{} around the assignment statement
http://hg.mozilla.org/mozilla-central/rev/f687f8bd6f7c

checked-in.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Keywords: inputmethod
You need to log in before you can comment on or make changes to this bug.