Last Comment Bug 610829 - Japanese IME: The textbox may move to a different window prior to commit
: Japanese IME: The textbox may move to a different window prior to commit
Status: RESOLVED FIXED
[4b7][inbound]
: inputmethod
Product: Core
Classification: Components
Component: Widget: Win32 (show other bugs)
: Trunk
: x86 Windows XP
: -- minor (vote)
: mozilla8
Assigned To: Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
:
Mentors:
Depends on:
Blocks: 569023
  Show dependency treegraph
 
Reported: 2010-11-09 15:53 PST by Naoki Hirata :nhirata (please use needinfo instead of cc)
Modified: 2011-07-08 05:47 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Screenshot (276.15 KB, image/png)
2010-11-09 15:53 PST, Naoki Hirata :nhirata (please use needinfo instead of cc)
no flags Details
screenshot (278.71 KB, image/png)
2010-11-09 15:54 PST, Naoki Hirata :nhirata (please use needinfo instead of cc)
no flags Details
Patch v1.0 (10.24 KB, patch)
2011-06-27 21:07 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
VYV03354: review+
Details | Diff | Review
Patch v1.1 (7.23 KB, patch)
2011-07-05 17:35 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured)
VYV03354: review+
Details | Diff | Review

Description Naoki Hirata :nhirata (please use needinfo instead of cc) 2010-11-09 15:53:10 PST
Created attachment 489329 [details]
Screenshot

1. use Japanese version of Windows XP
2. use Microsoft Japanese IME 
3. launch Firefox and set to Hiragana
4. bookmark the first page and tag it with "tag"
5. open a new browser
6. open the bookmarks
7. change the tag to "nihongo" and double tap the space 
8. click in the other browser's text field

Expected: the IME box does not change location
Actual: the IME box does switch over to the other browser
Comment 1 Naoki Hirata :nhirata (please use needinfo instead of cc) 2010-11-09 15:54:51 PST
Created attachment 489331 [details]
screenshot
Comment 2 Naoki Hirata :nhirata (please use needinfo instead of cc) 2010-11-09 15:57:25 PST
Mozilla/5.0 (Windows NT 5.1; rv:2.0b7) Gecko/20100101 Firefox/4.0b7
Comment 3 Alice0775 White 2010-11-09 16:46:39 PST
Confirmed WindowsXP+IME2003
*It does not happens ATOK2006
*It does not happens on Firefox3.6.13pre
Comment 4 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2010-11-09 19:35:42 PST
maybe, this is a regression of bug 569023.
Comment 5 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2011-06-19 03:27:31 PDT
(In reply to comment #3)
> *It does not happens ATOK2006

Alice-san, are you sure? Cannot you reproduce with ATOK too? I can reproduce this bug with ATOK 2010 and ATOK 2011 on Win7 with following STR:

1. Launch Firefox.
2. Open a new window.
3. Set focus to the textbox for search in one of the windows.
4. Turn on IME.
5. Input something and press space key and ensure the candidate window to be visible.
6. Click the other window's textbox for search.
Comment 6 Alice0775 White 2011-06-19 04:08:33 PDT
(In reply to comment #5)
> (In reply to comment #3)
> > *It does not happens ATOK2006
> 
> Alice-san, are you sure? Cannot you reproduce with ATOK too? I can reproduce
> this bug with ATOK 2010 and ATOK 2011 on Win7 with following STR:
> 
> 1. Launch Firefox.
> 2. Open a new window.
> 3. Set focus to the textbox for search in one of the windows.
> 4. Turn on IME.
> 5. Input something and press space key and ensure the candidate window to be
> visible.
> 6. Click the other window's textbox for search.

umm, the candidate window close immediately when I click the other window's textbox. I'm not sure, but it is  ATOK2006 and I use "明鏡国語辞典" optional dictionary.
Comment 7 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2011-06-19 08:09:12 PDT
Cannot you reopen the candidate window by pressing space key again?
Comment 8 Alice0775 White 2011-06-19 08:17:25 PDT
(In reply to comment #7)
> Cannot you reopen the candidate window by pressing space key again?
No I cannot. Autocomplete menu pops up when I press space key.
Comment 9 Alice0775 White 2011-06-19 15:57:07 PDT
(In reply to comment #4)
> maybe, this is a regression of bug 569023.

In my investigation with Microsoft IME, the regression window is as follows.
Works:
http://hg.mozilla.org/mozilla-central/rev/01fa971e62ee
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100827 Firefox/4.0b5pre ID:20100827190212
Fails:
http://hg.mozilla.org/mozilla-central/rev/0886ad6e6aaa
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b5pre) Gecko/20100827 Firefox/4.0b5pre ID:20100827190555
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=01fa971e62ee&tochange=0886ad6e6aaa

In localbuild:
build from 687b70fea4d0  :works
build from 7804b5cf4313 : fails
Comment 10 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2011-06-20 01:27:50 PDT
Thanks, it makes sense widget removable caused this. I'm creating patch but I'm fighting with some complicated issue...

I'll check the ATOK2006 behavior before review.
Comment 11 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2011-06-27 21:07:29 PDT
Created attachment 542370 [details] [diff] [review]
Patch v1.0

If an IM context has been disassociated and deactivated at committing or canceling composition, IME does nothing. Therefore, we should associate IM context with the window before doing that and disassociate IM from the window if we associated temporarily.

And now, we can use ImmAssociateContextEx() instead of ImmAssociateContext(). Therefore, we can get rid of mOldIMC from nsWindow.

Note that with ATOK 2011, you can see unnecessary candidate window on new focused editor. This should be a bug of ATOK 2011. I tried to avoid this issue by using NI_CLOSECANDIDATE but failed. I'll contact ATOK team later.
Comment 12 Masatoshi Kimura [:emk] 2011-06-28 09:11:37 PDT
Comment on attachment 542370 [details] [diff] [review]
Patch v1.0

> +  PRBool associated = aWindow->AssociateDefaultIMC(PR_TRUE);
> +  PR_LOG(gIMM32Log, PR_LOG_ALWAYS,
> +    ("IMM32: CommitComposition, associated=%s\n",
> +     associated ? "YES" : "NO"));
> +
>    nsIMEContext IMEContext(aWindow->GetWindowHandle());
>    if (IMEContext.IsValid()) {
>      ::ImmNotifyIME(IMEContext.get(), NI_COMPOSITIONSTR, CPS_COMPLETE, 0);
>      ::ImmNotifyIME(IMEContext.get(), NI_COMPOSITIONSTR, CPS_CANCEL, 0);
> +    if (associated) {
> +      aWindow->AssociateDefaultIMC(PR_FALSE);
> +    }
>    }
Is it Ok to skip disassociating when IMEContext.IsValid() returns false?

> +PRBool nsWindow::AssociateDefaultIMC(PRBool aAssociate)
This function do not share the code between associating and disassociating.
Could you split the function for those two cases?

r+ assuming the patch passes the try. Is it difficult to add a regression test?
Comment 13 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2011-06-28 16:44:29 PDT
(In reply to comment #12)
> Is it Ok to skip disassociating when IMEContext.IsValid() returns false?

Ah, it's Okay for now. But in logically, I shouldn't do so. I'll change it thanks.

> > +PRBool nsWindow::AssociateDefaultIMC(PRBool aAssociate)
> This function do not share the code between associating and disassociating.
> Could you split the function for those two cases?

I wrote so, but see the nsWindow::SetInputMode(). It makes the caller simple if we can change the behavior by param.

> r+ assuming the patch passes the try. Is it difficult to add a regression
> test?

Yes, we need to create IMM emulator layer for testing.
Comment 14 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2011-07-04 21:37:07 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/cceff8f75d6a

landed on inbound except the design issue of nsWindow::AssociateDefaultIMC() because no reply for comment 13 and it's just a coding style issue.

Kimura-san, if you don't like it and I should fix it, please file a new bug.
Comment 16 Masatoshi Kimura [:emk] 2011-07-04 23:28:31 PDT
(In reply to comment #14)
> landed on inbound except the design issue of nsWindow::AssociateDefaultIMC()
> because no reply for comment 13 and it's just a coding style issue.
That's fine. Sorry to forget a reply.
Comment 17 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2011-07-05 00:05:39 PDT
(In reply to comment #15)
> Backed out in
> http://hg.mozilla.org/integration/mozilla-inbound/rev/4aaf3b77a6f3 - see
> your assertion firing in
> http://tinderbox.mozilla.org/showlog.cgi?log=Mozilla-Inbound/1309840444.
> 1309844926.29590.gz&fulltext=1

Ah... probably the cause is that IMM isn't installed in English Windows.
Comment 18 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2011-07-05 17:35:19 PDT
Created attachment 544108 [details] [diff] [review]
Patch v1.1

I'd like to print the API failure. However, we shouldn't do it on non-IMM installed environment.

This patch adds nsIMM32Handler::IsIMEAvailable() for debug build. It returns the result of ::ImmIsIME() for current keyboard layout. And only when it returns TRUE, this patch checks the result of ::ImmAssociateContextEx(). Note that probably, it's not needed for disassociating because the context should be always NULL on non-IMM installed environment.
Comment 19 Masayuki Nakano [:masayuki] (Mozilla Japan) (working slowly due to injured) 2011-07-07 05:05:54 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/78a414587aa6
Comment 20 Marco Bonardo [::mak] 2011-07-08 05:47:52 PDT
http://hg.mozilla.org/mozilla-central/rev/78a414587aa6

Note You need to log in before you can comment on or make changes to this bug.