The default bug view has changed. See this FAQ.

Japanese IME: The textbox may move to a different window prior to commit

RESOLVED FIXED in mozilla8

Status

()

Core
Widget: Win32
--
minor
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: nhirata, Assigned: masayuki)

Tracking

({inputmethod})

Trunk
mozilla8
x86
Windows XP
inputmethod
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [4b7][inbound])

Attachments

(2 attachments, 2 obsolete attachments)

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
Created attachment 489331 [details]
screenshot
Attachment #489329 - Attachment is obsolete: true
Summary: Japanese IME The textbox will move to a different → Japanese IME: The textbox may move to a different window prior to commit
Mozilla/5.0 (Windows NT 5.1; rv:2.0b7) Gecko/20100101 Firefox/4.0b7

Comment 3

7 years ago
Confirmed WindowsXP+IME2003
*It does not happens ATOK2006
*It does not happens on Firefox3.6.13pre
Keywords: inputmethod

Updated

7 years ago
Whiteboard: [4b7]
(Assignee)

Updated

7 years ago
Severity: normal → minor
Component: Bookmarks & History → Widget: Win32
Product: Firefox → Core
QA Contact: bookmarks → win32
(Assignee)

Comment 4

7 years ago
maybe, this is a regression of bug 569023.
(Assignee)

Updated

6 years ago
Assignee: nobody → masayuki
Blocks: 569023
Status: NEW → ASSIGNED
(Assignee)

Comment 5

6 years ago
(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

6 years ago
(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.
(Assignee)

Comment 7

6 years ago
Cannot you reopen the candidate window by pressing space key again?

Comment 8

6 years ago
(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

6 years ago
(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
(Assignee)

Comment 10

6 years ago
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.
(Assignee)

Comment 11

6 years ago
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.
Attachment #542370 - Flags: review?(VYV03354)
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?
Attachment #542370 - Flags: review?(VYV03354) → review+
(Assignee)

Comment 13

6 years ago
(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.
(Assignee)

Comment 14

6 years ago
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.
Whiteboard: [4b7] → [4b7][inbound]
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
Whiteboard: [4b7][inbound] → [4b7]
(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.
(Assignee)

Comment 17

6 years ago
(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.
(Assignee)

Comment 18

6 years ago
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.
Attachment #542370 - Attachment is obsolete: true
Attachment #544108 - Flags: review?(VYV03354)
Attachment #544108 - Flags: review?(VYV03354) → review+
(Assignee)

Comment 19

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/78a414587aa6
Whiteboard: [4b7] → [4b7][inbound]
http://hg.mozilla.org/mozilla-central/rev/78a414587aa6
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.