17419 - IME bug-Clicking Tool bar then cannot type japanese

Status: VERIFIED FIXED

(Core :: Internationalization, defect, P3)

Description

[Teruko Kobayashi]

When you type Japanese characters in the Composer and click on Bold, Italic, or
Underline in the Tool bar, the Kana-input mode window will move to the beginning
of the first line.

Steps of reproduce
1. start kinput2
2. launch Apprunner
3. open Composer by selecting menu Tasks|Composer
4. Shift+Space to turn on kana input mode
5. type Japanese characters
   After you hit the return to commit the characters, the Kana-input mode
window, [あ], will move to the current carret.
6. select Japanese characters and click on Bold, Italic, or Underline button
   in the tool bar
Notice that the kana-input mode window will move back to the beginning of the
first line.

Tested 10-27-08 Linux build.

Comment 1

[Frank Tang]

I think this is a dup of 15496. Basically, the IME status window and over-the-spot uncommitted area is in wrong position.

Comment 2

[Hidetoshi Tajima]

I could reproduce the problem and have started investigation.

Comment 3

[Teruko Kobayashi]

This still happens in 12-01-09 Linux build.

Comment 4

[leger]

Can workaounnd.  Approved for pdt- per bobj.

Comment 5

[Frank Tang]

reassign this to ftang

Comment 6

[Frank Tang]

I can no longer reproduce this problem. Probably got fix last time. Mark it fix.

Comment 7

[Teruko Kobayashi]

This still reproduciable in 2000011208 Linux build, so I reopen this.

Comment 8

[Frank Tang]

The reproduce procedure teruko stated is not complete.

You need to do the following to reproduce this (after try w/ teruko several

times together...)

Click in the edit area

Type Japanese and commit

Click [B]

Click in the edit area.

The "click" is the key to reproduce it.



I cannot figure out what happen yet.

It seems are are sending the correct X,Y and XIC

Comment 9

[Frank Tang]

It looks funny now, but I don't think this is a BETA stoper. Move it to M16

Comment 10

[bobj]

So, there is no problem with recovering?  The user just clicks the mouse or
arrows over to the position s/he wants to type?  Which characters get the
style (Bold, Italic or Underline)?

Comment 11

[leger]

Putting dogfood in the keyword field.

Comment 12

[Frank Tang]

I don't think this is a dogfood bug.

Comment 13

[bobj]

Not dogfood.  Updated summary and keywords to reflect this.

Comment 14

[Frank Tang]

This may related to IC-per-shell issue.

Comment 15

[Masaki Katakai]

In the current implementation, IC is created per widget.
So [B] button has own IC and the IC is activated by clicking
the button. It causes this problem.

We plan to check in a big patch for ic-per-shell and
on-the-spot after M14. I've verified the patch can
solve this problem.

Comment 16

[Hidetoshi Tajima]

The timing to unset XIC focus of a text widget is wrong. We
should unset XIC focus of the text widget only when another
text widget should gets the XIC focus. Currently, we do unset
XIC focus when a button widget gets X Input focus, which is wrong, since the
button widget does not have an XIC. This
results we are making a dead period when no XIC is focused,
and no request to update spotlocation is processed during
that period.
(I'm sorry if my comments confuses you. To understand it, you
will need to know XIC focus and X input focus are different,
and how XIC focus is managed by XIM/Xlibrary).

Comment 17

[Hidetoshi Tajima]

Composer's HTML editor and Toolbar is a single
container component, regarding to input focus
management.

When a B-button on the Toolbar is pressed, input
focus is set back to the contentWindow component
by editor/ui/composer/content/EditorCommands.js
    contentWindow.focus();

This makes input focus given to contentWindow, but
not to HTML editor window, and it has resulted that
the HTML editor has lost XIM's XIC focus.

 contentWindow = W0
 |-----------------|
 |  toolbar pane   | Clicking editor pane sets focus onto editor
 |-----------------| pane(W1), but clicking toolbar sets
 |  editor pane    | focus back to parent(W0), which cause
 |         W1      | the problem.
 |-----------------|


Hence, this bug 17419 might be fixed if Composer can 
set focus to W1, but not to W0 when the toolbar is 
clicked.

 I'd like to forward this analysis to composer owners
and ask if this change can be accepted or not. If
they don't, we may put some *hack* into
widget/src/gtk/nsWidget like below:

  If a widget(A) getting input focus does not have
  XIC, and if that widget(A) is the parent of the
  widget(B) which previously had XIC focus, then we
  will set XIC focus back to the widget(B).

  In case of Composer, the widget (A) it
  contentWindow(W0), and the widget (B) is editor
  pane(W1) in the picture above.

Comment 18

[Frank Tang]

This is currenlty not only move Kana input window, but lost the Japanese input.
Tajima said he have a temp fix that will rememver the child window which have
XIC and restore it. We chat with cmansk and mjudget. We all think this is the
right thing to do before beta by adding this small temp fix since we we believe
sarri won't fix the focus issue by them.

Put beta1 and remove the [PDT-] now. If we do not have the temp fix then
Japanese user won't be able to type japanese after they hit any editor toolbar.

Comment 19

[leger]

Putting on PDT+ radar for beta1.

Comment 20

[Jim Roskind]

Please update the status whiteboard with a planned landing date.  IF you can't
provide a solution in the very near term, please kick this back to bobj or
ftang, and we'll try to find a resource to work on this bug. This is seemingly
quite critical, and the beta train will soon be leaving the station.
Thanks,
Jim

Comment 21

[Hidetoshi Tajima]

Attachment: save the XIC-ed child widget for non-XIC-ed parent

Comment 22

[Hidetoshi Tajima]

Frank, proposed patch is in the attachment. Please
review it with folks you think to be involved in.
Note that this contains the change at editor/base/nsEditor.cpp, which is for
calling ResetInputState() when the composer is first brought up. This patch will
give the fix for 29715,
and also fix yet another focus problem I wrote to
you yesterday.

Comment 23

[Hidetoshi Tajima]

Okay, it is my fault. What I tried to fix for 17419
is including this phenomena, and the proposed patch
for 17419 will fix it. Try out!

Comment 24

[Frank Tang]

tajima, neither editor folks nor me understand why you put the 
ForceCompositionEnd(); in that method. Should it be called in some event code 
instead ?

Comment 25

[Hidetoshi Tajima]

Frank, as comments in the patch says, what I really
like to call is ResetInputState() which is currently 
only called in ForceCompositionEnd(). We create XIC
instance for an nsWidget at the 1st ResetInputState() call for the widget, and,
on Mozilla composer, the
1st ResetInputState() call is made when you click the
editor window. The patch will make the call happen a
bit earlier when the composer 1st gets mapped on the
screen.

Comment 26

[Masaki Katakai]

Yes, we need ResetInputState() call for XIC initialization.

But, ResetInputState() may not be proper method for this purpose.

How about adding InputFieldInit() method to nsIKBStateControl?

We have PasswordFieldInit() method for initialization of

password field, but don't have for general input field,

HTML and Text editor that need input method.

We can use it for XIC initialization.



      /*

       * Force Input Method Editor to commit the uncommited input

       */

      NS_IMETHOD ResetInputState()=0;



      /*

       * This method is called in the init stage of a password field

       */

      NS_IMETHOD PasswordFieldInit()=0;



+     /*

+      * This method is called in the init stage of an input field

+      */

+     NS_IMETHOD InputFieldInit()=0;



We can call the method PasswordFieldInit() or InputFieldInit()

for initialization purpose in PostCreate() of editor, and can

call ResetInputState() for reset purpose. I think it's proper

changes for nsEditor to fix this problem and 16940.

What do you think?

Comment 27

[Hidetoshi Tajima]

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

Comment 28

[Hidetoshi Tajima]

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

Comment 29

[Hidetoshi Tajima]

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

Comment 30

[bobj]

Frank, We need your comments to the last 2 comments.  Are we in agreement of
what we must do for the Beta1 temp fix?  Pls comment tonight after your class.

Comment 31

[Masaki Katakai]

Attachment: patch that uses nsIKBStateControl::InputFieldInit()

Comment 32

[Masaki Katakai]

I created a new patch which uses InputFieldInit() new 
nsIKBStateControl.

At initialization of nsEditor,

  nsIKBStateControl::InputFieldInit()

  or

  nsIKBStateControl::PasswordFieldInit()

is called. InputFieldInit() method of widget is called when
the widget needs IME. PasswordFieldInit() method is called
when the widget is password field, so we can find the widget
doesn't need IME.

Then if nsEditor needs to reset IME state, call

  nsIKBStateControl::ResetInputState()

ResetInputState() should be used only for the purpose that resetting
the IME state. So I believe that adding new method InputFieldInit()
to nsIKBStateControl is the right way for fixing this problem.

Please review. And please ask editor people, owners of mac and
windows IME. widget/src/mac/nsWindow.cpp and
widget/src/windows/nsWindow.cpp need to be update also.

Thanks.

Comment 33

[Frank Tang]

I don't think we should take the new fix for beta1 now. The change require 
changing xp interface now. I think we could take the old patch. 

Comment 34

[Jim Roskind]

Time for beta is running out on this.  Please consider landing the patch that
frank suggested was acceptable RSN.
Please update the status whiteboard (which shows a lading date of last friday)
to have a correct date.
Thanks,
Jim

Comment 35

[Hidetoshi Tajima]

We Sun team are okay to go with the 1st patch
(3/02/00's one) for beta, but we'll make sure to
discuss this later to move to the 2nd patch after
beta. Frank, for check-in approval whom should I talk
to? Will you please help get it?

Comment 36

[bobj]

Please have ftang review and run the pre-checkin tests.  If all looks good,
then a=bobj.

Comment 37

[Frank Tang]

r=ftang for the 1st patch. check in when the tree is open after you run & pass 
the pre-check in test.

Comment 38

[bobj]

Fix checked in 3/9 17:30.  r=ftang a=bobj

Comment 39

[msanz]

reassigning to Xianglan for verification.

Comment 40

[ji]

Verified with 03-13-03-M15-nb1b build. It is fixed.