1151186 - Japanese IME input is not working Google spreadsheet

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[PRiMENON]

Build identifier: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0

Steps to reproduce:

0. create clean profile.
1. open Google Drive and login google account.
2. create new spreadsheets.
3. turn on Japanese IME
4. input あああ
5. active window change firefox to other application.
6. re-active firefox window
7. input あああ

Actual results:
aaa

Expected results:
あああ

Comment 1

[Alice0775 White]

I can reproduce all version of Firefox on Windows7 MSIME2010 and Ubuntu14.04 Mozc Fcitx.
And reproducible regardless of e10s or TSF.

Google Chrome41 and IE11 works fine.

Masayuki san, Is Google's failure?

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

FYI: Except when you turn intl.tsf.force_enable true, TSF is disabled in e10s mode. So, this should be e10s's bug (I cannot this bug without e10s).

I'm not sure if it's google's failure, but perhaps, ours.

Comment 3

[Makoto Kato [:m_kato]]

comment #0 is on TSF mode.

If using IMM32 mode, you cannot input any character even if turning off IME after step 5.  Then, after IME blur (focus awesome bar), you can input character by IME.

Also, If using IMM32 mode, behavior is very very strange

- Step
1. focus A1 cell
2. input あああ
3. focus B1 cell
4. input いいい

- Result
Candidate window is top-level of screen and input characters (by step 4) isn't appeared in cell.  After hit [Enter] for committing string, you can input character by IME.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Oops, I retry to test the STR, I reproduce this both TSF/IMM without e10s.

Looks like that the editable cell is made by setting contenteditable="true". However, our focus management fails to set proper IME state when the cell getting focus.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

When I double-click on a cell immediately after open a spreadsheet, IME is kept disabled. However, the log of IMEStateManager looks like working fine. I'll investigate what occurs on focusing cell.

I guess that root cause could be same as the reported bug.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Oh, I found odd log...

> 0[1011140]: ISM: IMEStateManager::OnFocusInEditor(aPresContext=0x22f73c00, aContent=0x29849150, aEditor=0x2949a4f0), sPresContext=0x22f73c00, sContent=0x29849150, sActiveIMEContentObserver=0x0
> 0[1011140]: ISM: IMEStateManager::CreateIMEContentObserver(aEditor=0x2949a4f0), sPresContext=0x22f73c00, sContent=0x29849150, sActiveIMEContentObserver=0x0, sActiveIMEContentObserver->IsManaging(sPresContext, sContent)=false
> 0[1011140]: ISM:   IMEStateManager::CreateIMEContentObserver() is creating an IMEContentObserver instance...
> 0[1011140]: ISM: IMEStateManager::OnChangeFocus(aPresContext=0x22f73c00, aContent=0x0, aCause=CAUSE_UNKNOWN)
> 0[1011140]: ISM: IMEStateManager::OnChangeFocusInternal(aPresContext=0x22f73c00, aContent=0x0, aAction={ mCause=CAUSE_UNKNOWN, mFocusChange=FOCUS_NOT_CHANGED }), sPresContext=0x22f73c00, sContent=0x29849150, sActiveIMEContentObserver=0x25181f80
> 0[1011140]: ISM: IMEStateManager::NotifyIME(aMessage=REQUEST_TO_COMMIT_COMPOSITION, aWidget=0xebdf000), composition=0x0, composition->IsSynthesizedForTests()=false
> 0[1011140]: ISM: IMEStateManager::DestroyIMEContentObserver(), sActiveIMEContentObserver=0x25181f80
> 0[1011140]: ISM:   IMEStateManager::DestroyIMEContentObserver(), destroying the active IMEContentObserver...
> 0[1011140]: ISM: IMEStateManager::GetNewIMEState(aPresContext=0x22f73c00, aContent=0x0), sInstalledMenuKeyboardListener=false
> 0[1011140]: ISM:   IMEStateManager::GetNewIMEState() returns DISABLED because no content has focus
> 0[1011140]: ISM: IMEStateManager::SetIMEState(aState={ mEnabled=DISABLED, mOpen=DONT_CHANGE_OPEN_STATE }, aContent=0x0, aWidget=0xebdf000, aAction={ mCause=CAUSE_UNKNOWN, mFocusChange=LOST_FOCUS })
> 0[1011140]: ISM:   IMEStateManager::SetIMEState(), calling nsIWidget::SetInputContext(context={ mIMEState={ mEnabled=DISABLED, mOpen=DONT_CHANGE_OPEN_STATE }, mHTMLInputType="", mHTMLInputInputmode="", mActionHint="" }, aAction={ mCause=CAUSE_UNKNOWN_CHROME, mAction=LOST_FOCUS })
> 0[1011140]: ISM: IMEEnabledStateChangedEvent::Run(), notifies observers of "ime-enabled-state-changed"

When I set focus to a cell in RDP and check the log from another computer, I see this log.

Strangely, it's disabled due to nobody has focus after normal (expected) focus handling.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Attachment: part.1 Don't reset IME enabled state after focus is changed by JS during setting focus

There are two bugs. One of them is fixed by this patch.

If a focus event handler of document sets focus to an editor, nsFocusManager::Focus() calls IMEStateManager::OnChangeFocus() with null as focused content. Then, IMEStateManager disables IME *after* the editor finishes enabling IME.

So, if focus is moved to another element during running nsFocusManager::Focus(), it shouldn't call IMEStateManager::OnChangeFocus().

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Attachment: part.2 nsHTMLEditor should accept composition events even while there is no active editing host but it has composition

Only with the part.1, we meet MOZ_ASSERT() at activating window. It's here:
http://mxr.mozilla.org/mozilla-central/source/dom/events/TextComposition.cpp?rev=bd14f3a87117#278

TextComposition::mIsComposing is true only while focused editor has focus. However, nsHTMLEditor fails to receive composition events during nsFocusManager handling activating a window (It's not problem in normal cases, but focus event handler of document can change focused element and it may cause requesting to commit composition to IME).

But fortunately, there can be only one composition on a top level window. Therefore, if nsHTMLEditor has composition (mComposition isn't nullptr), nsHTMLEditor::IsAcceptableInputEvent() should always return true for composition events if mComposition isn't nullptr. Then, we can guarantee that all composition events is always handled by an editor even if it has lost focus.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Attachment: part.1 Don't reset IME enabled state after focus is changed by JS during setting focus

Oops, sorry. I posted different bug's patch. Please see the previous patch's comment for the detail.

Comment 10

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Attachment: part.2 nsHTMLEditor should accept composition events even while there is no active editing host but it has composition

Sorry for the spam. I posted different bug's patch. Please see the previous patch's comment for the detail.

Comment 11

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Ehsan:

Ah, and please ignore the part of IMEStateManager, I just fixes a missing case of logging code found at debugging this bug.

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Is this a regression?

Comment 13

[Alice0775 White]

Attachment: testcase of the patch part1

Regression window using testcase of the patch part1:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=90d3e6d2cbb9&tochange=4430cae50dad

triggered by: Bug 178324

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1cde9e7658e5
https://hg.mozilla.org/integration/mozilla-inbound/rev/67dbfa43caa3

Comment 15

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

I don't think that this is so critical because this occurs only after switching from another application and there is composition string before that. I think that this isn't so usual case. Additionally, the case to reproduce this bug is very tricky. Typically, web developers don't move focus from focus event handler of document.

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

Actually, since 2002, this hasn't been reported for about 12 years!

Comment 17

[Ryan VanderMeulen [:RyanVM][PTO June 24-28]]

https://hg.mozilla.org/mozilla-central/rev/1cde9e7658e5
https://hg.mozilla.org/mozilla-central/rev/67dbfa43caa3