944397 - [Input method API] inputContext is not accessible if the app frame is loaded *after* user had focusing the input field

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Tim Guan-tin Chien [:timdream] (please needinfo)]

+++ This bug was initially created as a clone of Bug #944009 +++

Please read bug 944009 comment 9. Gary found that API cannot correctly expose inputContext to the newly created frame.

:yxl, do you have cycle to work on this? If not I will ask someone in Taipei for help (or I will try to do this in my spare time). Thanks!

This bug is 1.3+ because it blocks a 1.3+ bug.

Comment 1

[Yuan Xulei [:yxl] (Leave from May 18)]

Tim, Sorry. I don't have time to work on it until next Tuesday(Dec 3), but I can you to solve the bug.

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

http://dxr.mozilla.org/mozilla-central/source/dom/inputmethod/MozKeyboard.js#436

Maybe this message is lost if setInputMethodActive() is called when the frame is just created?

Comment 3

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Here is a better STR:

1. Apply the patch enabling keyboard oop
2. Focus on e.me search field and type some text
3. Remove the input focus
4. In adb shell, find the process id of "Built-in Keyboard" and kill it.
5. Focus on e.me search field again.

Expected:

1. Keyboard shows up briefly (it will be launched on-demand)

Actual:

1. Keyboard does not show up, even though we could verify it has been launched

Note:

We have ruled out possible keyboard management bug; keyboard frame at step 5 were indeed being set to active and visible. It simply does not receive the input context upon launch, nor receiving an inputcontextchange event.


Note 2:

If we tweak the STR to the following, IM API works as expected:

1. Apply the patch enabling keyboard oop
2. In adb shell, find the process id of "Built-in Keyboard" and kill it.
3. Focus on e.me search field and type some text

In this STR, keyboard app will only be launched on step 3. It will not receive the input context upon launch, BUT it WILL receive an inputcontextchange event almost after launch.


No idea what's the difference between the two STRs.

Comment 4

[Ivan Tsay (:ITsay)]

Hi Xulei,

(In reply to Yuan Xulei [:yxl] from comment #1)
> Tim, Sorry. I don't have time to work on it until next Tuesday(Dec 3), but I
> can you to solve the bug.

I wonder if you already have time to help on this bug. Thank you!

Comment 5

[Yuan Xulei [:yxl] (Leave from May 18)]

Sorry, not much time until now. But I'll start to look into this bug as well as other keyboard API bugs tomorrow.

Comment 6

[Yuan Xulei [:yxl] (Leave from May 18)]

By tracing Tim's first STR of Comment 3, I got the cause of this bug:

In step 5, when the keyboard app iframe is set to active and keyboard app content doesn't start to load, so the keyboard app iframe isn't be recognized as an input method iframe, and we fail to set it active. The related code is:
 http://mxr.mozilla.org/mozilla-central/source/dom/browser-element/BrowserElementChildPreload.js#914

The solve this bug, the iframe.setInputMethodActive should be called after the iframe initializes.

Comment 7

[Yuan Xulei [:yxl] (Leave from May 18)]

Attachment: gecko patch v1

We add a mozbrowser method setInputMethodActive to set an keyboard app iframe as active. When calling setInputMethodActive from BrowserElementParent, the 'set-input-method-active' message is sent to BrowserElementChild, and then BrowserElementChild calls navigator.mozInputMethod.setActive to activate the keyboard. Sometimes it is too early to call setInputMethodActive that BrowserElementChild can't get navigator.mozInputMethod and cause this bug.

The patch makes BrowserElementChild wait until DOMWindowCreated event fires to ensure navigator.mozInputMethod is available.

Comment 8

[Yuan Xulei [:yxl] (Leave from May 18)]

Gary, could you help to check if the above patch fixes your problem in Bug 944009 ([B2G][Keyboard] Keyboard does not always display when tapping on a text field)?

Comment 9

[[:fabrice] Fabrice Desré]

Comment on attachment 8344166 [details] [diff] [review]
gecko patch v1

Review of attachment 8344166 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm, but we need tests there before landing.

Comment 10

[Ivan Tsay (:ITsay)]

Plus this bug for v1.4 since it is the blocker on re-enable OOP for 3rd party keyboard.

Comment 11

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

(In reply to Yuan Xulei [:yxl] from comment #8)
> Gary, could you help to check if the above patch fixes your problem in Bug
> 944009 ([B2G][Keyboard] Keyboard does not always display when tapping on a
> text field)?

Hi Xulei,
   Nice work!!!!
   Keyboard works fine after OOM with your patch.

   Thanks a lots!!!

Comment 12

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Hi Xulei,
   I found keyboard will get |inputContext| but can not sent Key after OOM, it means keyboard app will be recalled again and show well but it can not type anything.

   Can you help to clarify this issue?

Comment 13

[William Hsu [:whsu]]

Regarding comment 12, if you need reproduction step, please needinfo "[e-mail]".
Thanks!

Comment 14

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Hi Xulei,
   Rudy find the root cause and filed a bug 950573.
   remove ni from you.

   Thanks Rudy and Xulei.

Comment 15

[Yuan Xulei [:yxl] (Leave from May 18)]

Attachment: gecko patch v2 with test.

The patch made an improvement. If there are more than requests sent to BrowserElementChild before DOMWindowCreated, these requests is queued. While with the previous patch, these requests are dropped except the last one.

I also added a test based on the patch of Bug 932145.

Comment 16

[Yuan Xulei [:yxl] (Leave from May 18)]

(In reply to Yuan Xulei [:yxl] from comment #15)
> The patch made an improvement. If there are more than requests sent to
> BrowserElementChild before DOMWindowCreated, these requests is queued. While
> with the previous patch, these requests are dropped except the last one.
Sorry about the grammar mistakes. Please see the following one instead.
-------------------------------
The patch makes an improvement. If there are more than one requests sent to
BrowserElementChild before DOMWindowCreated, these requests are queued. While
with the previous patch, these requests were dropped except the last one.

Comment 17

[Yuan Xulei [:yxl] (Leave from May 18)]

https://tbpl.mozilla.org/?tree=Try&rev=66e871666710

Comment 18

[Yuan Xulei [:yxl] (Leave from May 18)]

Attachment: gecko patch v2 with test

Add the missing file...

Comment 19

[Yuan Xulei [:yxl] (Leave from May 18)]

https://tbpl.mozilla.org/?tree=Try&rev=ecdd1f70fbe4

Comment 20

[Yuan Xulei [:yxl] (Leave from May 18)]

Comment on attachment 8350057 [details] [diff] [review]
gecko patch v2 with test

test passed :-)

Comment 21

[Kan-Ru Chen [:kanru] (UTC+9)]

https://hg.mozilla.org/integration/b2g-inbound/rev/be94be87a84c

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/be94be87a84c

Comment 23

[William Hsu [:whsu]]

I still can reproduce it by reproduction steps of comment 3.

By the way, I filed a similar bug( Bug 953027 ) a week ago.
Please help distinguish the two bugs then we can decide whether we need to file a new bug or mark as duplicate one.
Thanks!

* Build information
 - Gaia      5de94a2be6ab8d33434294d70c1de330f55d8f2d
 - Gecko     http://hg.mozilla.org/mozilla-central/rev/c8d5a871ae32
 - BuildID   20140101040201
 - Version   29.0a1

* Result:
  Can reproduce

Comment 24

[Jason Smith [:jsmith]]

William - Can you file a followup bug for what you found? The patch for this bug has already landed, so we need to open a followup for the issue found to ensure tree management can track what's landed vs. not.

Comment 25

[William Hsu [:whsu]]

Sure! Thanks for the reminder.
I think the bug relates to Bug 953027 and Bug 953044.
So, I suggest to follow remaining work on Bug 953027 and Bug 953044.
Thanks.

Happy New Year!  :)

Comment 26

[Ivan Tsay (:ITsay)]

Hi Xulei,

Should we handle both bug 953027 and 953044 to resolve keyboard OOM issue? William test the keyboard OOM issue and found it can still be reproduced. I would like to clarify the bugs that need to be followed up for resolving this issue. Thank you!

Comment 27

[Yuan Xulei [:yxl] (Leave from May 18)]

This bug wasn't completedly resolved. Bug 953044 is a followup for this bug. Once bug 953044 is resolved, keyboard OOM issue should be resolved.

Comment 28

[Ivan Tsay (:ITsay)]

(In reply to Yuan Xulei [:yxl] from comment #27)
> This bug wasn't completedly resolved. Bug 953044 is a followup for this bug.
> Once bug 953044 is resolved, keyboard OOM issue should be resolved.

Points taken. Thank you for the clarification