UNIX IMEs sometimes invoke extra windows for operation, for example, candidate window which grabs input focus. As I described the problem in http://bugzilla.mozilla.org/show_bug.cgi?id=29065 IME wants to draw pre-edit text (composed text) while the candidate window is invoked. It seems that Editor does not accept NS_TEXT_EVENT for updating pre-edit text by the recent changes in Editor and Layout (/event?). When I filed a bug 29065, Editor could accept NS_TEXT_EVENT, so I could see the pre-edit text was updated event when candidate window is invoked. However, it does not work on the current build. How to reproduce: 1) Start Mozilla 2) Visit www.yahoo.com 3) Click on text field for Search 4) Turn conversion on by Shift+SPACE 5) Type a i 6) Invoke candidate window by SPACE, SPACE Main window of Mozilla will loose the input focus 7) Select a candidate candidate window will be closed and the selected text will be drawn on the text field, but it won't. The first candidate word is still displayed. 8) type return to commit the word The correct word is committed, which means the word drawn in 7) is not correct. I'd like to ask Editor and Layout folks to evalute this problem. Problem of kinput2 (filed as 47568) seems to happen due to this.

reassign to kin. add yokoyama@netscape.com to the cc list kin- do you know what happen about this ?

setting to m19, Kin please take a look and see what is up, we will evaluate after Kin has preformed a preliminary debug

Accepting bug. I'm going to need some help setting IME up on my Linux box.

Kin - Is somebody helping to setup IME on your Linux? Also, since this is i18n related problem, I suggest to assign teruko@netscape.com or ji@netscape.com for QA concact.

future

Adding intl and nsbeta1 keywords. Adding mcarlson to cc-list.

I see slightly different behavior. export LANG=ja_JP.eucJP jserver kinput2 & ./mozilla 1) click in the URL bar. 2) shift-space to engage IME 3) type "watashi" 4) type Ctrl-w twice to bring up candidate window If I use the keyboard to navigate the candidate window (space to advance, return to select) the new character(s) is displayed. If I use the mouse to select a candidate the new character(s) is not displayed until I click in the URL bar.

Katakai-san, is your window focus model: pointer or click? Would someone at sun check if the display is wrong when: 1) just using the keyboard to select the kanji or 2) using the pointer to select the kanji

tajima@eng.sun.com is blocked by this bug. Any ideas on unblocking? /be

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

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

changing to nsbeta1-. this one does not meet beta stopper guidelines.

Beppe - I don't we should future this one, it has multiple dupes and is a blocker. Pls reconsider for M0.9.2. Adding nsCatFood and RTM keyword.

This problem is still serious when users use enlightenment and CDE window manager because there is no way to configure IME candidate window not to grab input focus. The problem is that event with NS_TEXT_EVENT does not go to the *last focused* content. I'm now debugging this problem and need you help. It seems that IME works well on HTML editor but does not work on any input field of XUL docs and HTML form. I found the following mCurrentEventContent = mDocument->GetRootContent(); in http://lxr.mozilla.org/mozilla/source/layout/html/base/src/nsPresShell.cpp#5399 does not return correct content while it's losing input focus. On HTML editor, it returns correct value which is same wiht HTML editor itself. However, on HTML form, the GetRootContent() returns the parent(?) content of HTML form field, not the exact content of HTML form field. Is there any way to get the *last focused* content (e.g. input form) here? Or if it is OK to return the parent content here but can we pass the event to the input form finally?

This sounds like this is a Focus/Event problem, not an editor problem. Cc'ing the focus gurus ... saari@netscape.com and blizzard@mozilla.org Anyone want to take this off my plate?

Editor *will* get a blur event if the window is deactivated. Can the editor accept these events when blurred? Have you verified that it gets them, or does the event never make it that far?

I understand editor can accept NS_TEXT_EVENT events while losing focus because this problem does not happen on HTMLEditor (Composer). This problem happens on any text field of GUI and HTML forms. When input field has an input focus, manager->GetFocusedContent(&mCurrentEventContent); can return the correct and exact content of nsHTMLInputElement. If not, mCurrentEventContent = mDocument->GetRootContent(); can not return the exact content of the *last focused* nsHTMLInputElement. Is it possible to get the *last focused* content here?

I've verified this problem does not happen on M16 but M17 has this problem.

Should we change Summary to "layout does not throw NS_TEXT_EVENT event to nsHTMLInputElement while losing focus" ?

I found one questionable point on source codes. (But, I don't know this is related to this bug.) Try to search by NS_TEXT_EVENT. http://lxr.mozilla.org/seamonkey/search?string=NS_TEXT_EVENT There are many codes like "event->message == NS_TEXT_EVENT". However, according to the struct nsEvent, NS_TEXT_EVENT must be in member "eventStructType", not "message". http://lxr.mozilla.org/seamonkey/source/widget/public/nsGUIEvent.h#88 Are these codes mistakes?

Did anyone see my comment above ?

Thank you, Kamio-san, widget side always sets both message and eventStructType to NS_TEXT_EVENT. So I don't think this causes problem. /widget/src/gtk/nsWindow.cpp, line 3700 -- textEvent.message = textEvent.eventStructType = NS_TEXT_EVENT;

I've attached the codes for workaround. The problem is that if losing input focus, editor does not accept any NS_TEXT_EVENT so we're seeing two problems, - composed characters are not updated when we choose a candidate on candidate window. when the candidate window grabs input focus. - commit string on candidate window does not work because commit event comes before mozilla grabs input focus again from candidate window There is no way to solve problem #1 by IME side, but for problem #2, we can store committed string as pending string then commit it by the input focus is back. I understand this is not really good way to solive this, but it can be workaround for reference. #2 is very serious issue.

patch for widget side is not working well on composer, http://bugzilla.mozilla.org/attachment.cgi?id=76393&action=view because focus is not back to "html editor pane" when getting focus again... Actually there is not way for widget side to fix this problem... I tried to modify PresShell::HandleEvent() of nsPresShell.cpp. manager->GetFocusedContent(&mCurrentEventContent); Is the focus in Mozilla, mCurrentEventContent is returned. This is correct. If not, trying to get mCurrentEventContent by, mDocument->GetRootContent(&mCurrentEventContent); this works on only composer. On HTML form and any XUL UI, it does not work. When input focus is not on Mozilla, this code is used but which will not send NS_TEXT_EVENT to proper contents. How about the patch? If the input focus exactly is not on Mozilla, get the last focused-content by using GetFocusedElement(). If not, continue to use mDocument->GetRootContent(). Kin, what do you think? logically correct? Toshi, can you try on your environment?

so, this is not editor bug, I think. Is it better to change component to layout or event handling?

Right, the bug is not an editor or layout problem, it's an event handling/dispatching problem. I'm going to defer to the focus/event handling gurus on this one, since I don't know the full extent of what could be affeted by your patches. saari, aaronl, or joki ... any comments?

Could anyone review the patch and give comments please?

Since we know this isn't an editor problem, I'm going to hand this one over to saari so it shows up on his radar.

saari@netscape.com: Can you or someone take a look of this patch for us please? We want someone in this area to check out the patch because we need this ASAP for Sun's release. Thanks, Margaret

Chris Saari is on vacation until Tuesday.

Can anyone review the patch? I've verified the patch is working fine with the current tree.

We need this review please. Can someone give us a pointer to a reviewer for this bug?

CC:'ing some editor folks...

Hi Roland. Thanks for cc'ing to others. Any chance that you can help getting this in? We need it badly. Margaret

I am concerned with the last change; it sounds like that will break something on Macintosh (maybe keybindings?).

Thank you very much for comment, I don't have build environment for Mac, so I can not verify this. OK, this problem happens on only UNIX platform, no problem for other platoform because candidate window does not grabs input focus on other platform. We'd better to keep existing codes for other platform. I've added #if defined(XP_UNIX) for safety. Also, for safer way and to avoid performance regression for normal key event, I've put the codes for getting mCurrentEventContent into if (NS_IS_IME_EVENT(aEvent)) { } Could you review again please?

Comment on attachment 97720 [details] [diff] [review] use defined(XP_UNIX) I am not sure whether |XP_UNIX| is correct since we have Unix-like OSes without X11 (MacOSX, Photon port, BeOS etc. etc) ... ... what about using |MOZ_X11| instead ?

Thanks, Roland. Yes, I believe that MOZ_X11 is correct for this case.

Comment on attachment 97757 [details] [diff] [review] Updated patch which is only used on X11-based ports (GTK+, Qt, Xlib) I'm not at all familiar with this code, but it looks very safe -- it won't effect non-X11 platforms and it won't change anything on any event that isn't NS_IS_IME_EVENT, so I think taking it is safe. I have three stylistic issues: >+ nsCOMPtr<nsIFocusController> focusController; >+ nsCOMPtr<nsIScriptGlobalObject> ourGlobal; >+ mDocument->GetScriptGlobalObject(getter_AddRefs(ourGlobal)); >+ nsCOMPtr<nsPIDOMWindow> ourWindow = do_QueryInterface(ourGlobal); >+ if (ourWindow) { >+ ourWindow->GetRootFocusController(getter_AddRefs(focusController)); You don't really need the focus controller until you're inside the ourWindow clause, so it would be better to declare it there. >+ if(focusedElement) { You should probably put a space after the if for consistency with the rest of the function. The third issue is that I find it hard to read with the X11 clause in one place and then later the !X11 clause. Everything in between them is comments, and they all apply to the non-X11 clause, so can you change that first #endif to an #else and remove the #ifndef? I'll attach a revision of the patch in case that wasn't clear, with my three changes, but I'll mark my r=akkana on this one.

Thank you very much, Akkana. I've corrected the position of focusController definition and space issue. Your changes for #else are included. Could you r= please?

Oh, I'm sorry the patch has r= already. Now asking sr=... to reviewers.

Comment on attachment 97919 [details] [diff] [review] new patch with Akkana 's comment + // if not, search pre-focused element "search for" +#endif /* !MOZ_X11 */ That '!' should not be there (since your ifdef is now just MOZ_X11). + focusedElement->QueryInterface(NS_GET_IID(nsIContent), + (void **)&mCurrentEventContent); CallQueryInterface(focusedElement, &mCurrentEventContent); Also, there's lots of commented-out code around here that just makes the code hard to understand, imo. Could we just remove that code please? (cced saari, who commented this out ages ago). Looks good other than those nits.

Thank you very much for review and comments, Boris. I've corrected the patch, except removing codes in comment line. Saari, could you give suggestion asap? We're very close to code freeze of OEM branch.

Comment on attachment 98409 [details] [diff] [review] update patch sr=bzbarsky. Just land this if saari does not get back to you by the time you need to land it... If possible, do that on branch only so we can clean the code up a bit on the trunk.

Sure, Thank you very much. Trunk tree is frozen for Mozilla 1.2alpha, so I'll not ask this to Trunk but ask to OEMbrach. Adding branchOEM keyword to request checkin to OEMbranch.

Hi Masaki: Please check this in to the OEM branch. Jim has already given you the permission for checkin to the OEM branch under the assumption that you will check this into the trunk when the code is cleaned up and the trunk is reopen. Thanks, Margaret

Hi Boris: Is saari around? We have not heard from him for quite a while. Shall we check in to the trunk (once it is open) and file another bug (assigned to saari) for code clean up. We just want to make sure that we are in sync with the trunk to avoid any discrepancies with our future release. Thanks, Margaret

If saari is not answering, I say we rip out those comments (and make that clear in the checkin comment). CVS history will have them if really needed.

> If saari is not answering, I say we rip out those comments (and make that clear > in the checkin comment). CVS history will have them if really needed. I concur with Boris, in case you want a second opinion/reviewer for that part.

Tree is now open for 1.2Beta. I'll check in the same patch to Trunk and OEMbranch. For Trunk, after the check-in, I'll remove the commented out codes with proper CVS comments by myself for clean up if I do not get answer from Sarri or ask Sarri to do that.

checked in the patch to Trunk. Will track code cleanup as bug 167866. Thanks everyone for your help!

checked into OEM branch. Thanks again.

Seems it still not fix for me on 09-12 trunk and 09-13 branch builds / linux RH7.2-JA. So, this fix only for OEM? I wonder why?

It was checked in to the trunk. Not branch though. I am assuming that you are talking about the 1.2 alpha branch?

No, I double checked this on 09-13 both trunk and branch - I don't see there is any difference: If I invoke Japanese IME, and start type something to bring up the candidate window, and press space key to switch the candidate chracters, only first two characters can be displayed in non-commit character position, all others can be highlighted but can not showed up in non-commit character. When I hit return key to commit, then I will get the highlighted character(s) be commit not the one in non-commit position. Is this bug supposedly fix this problem?

Hi Yuying, thank you for testing, The first problem - only first candidate can be displayed on preedit area this is correct behavior of kinput2. Please try the same operation on normal gtk application such as gedit. You can see the same behavior on it. The 2nd problem - candidate string can not be displayed at commit this is a bug of kinput2 I understand. Please see bug 63456. I have marked the bug as WONT FIX. kinput2 does not call needed method at commit. Type something after your commit, the preedit string can be changed to proper characters you selected at candidate window. Does your testing team have ATOK X input server? I'll try to verify the fix on Trun with ATOK X and let you know the result.

verified the fix on OEM branch and Trunk on both Linux and Solaris.