Closed Bug 944397 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.4+, firefox28 wontfix, firefox29 fixed, b2g-v1.4 fixed)

RESOLVED FIXED
blocking-b2g 1.4+
Tracking Status
firefox28 --- wontfix
firefox29 --- fixed
b2g-v1.4 --- fixed

People

(Reporter: timdream, Assigned: xyuan)

References

Details

Attachments

(1 file, 2 obsolete files)

+++ 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.
Flags: needinfo?(xyuan)
Tim, Sorry. I don't have time to work on it until next Tuesday(Dec 3), but I can you to solve the bug.
Flags: needinfo?(xyuan)
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?
Summary: [Input method API] inputContext is not accessible if the app frame is loaded *after* user had focusing the app → [Input method API] inputContext is not accessible if the app frame is loaded *after* user had focusing the input field
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.
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!
Flags: needinfo?(xyuan)
Sorry, not much time until now. But I'll start to look into this bug as well as other keyboard API bugs tomorrow.
Flags: needinfo?(xyuan)
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.
Attached patch gecko patch v1 (obsolete) — Splinter Review
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.
Attachment #8344166 - Flags: review?(fabrice)
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)?
Assignee: nobody → xyuan
Status: NEW → ASSIGNED
Flags: needinfo?(gchen)
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.
Attachment #8344166 - Flags: review?(fabrice) → review+
Plus this bug for v1.4 since it is the blocker on re-enable OOP for 3rd party keyboard.
blocking-b2g: --- → 1.4+
Depends on: 932145
(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!!!
Flags: needinfo?(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?
Flags: needinfo?(xyuan)
Regarding comment 12, if you need reproduction step, please needinfo "whsu@mozilla.com".
Thanks!
Hi Xulei,
   Rudy find the root cause and filed a bug 950573.
   remove ni from you.

   Thanks Rudy and Xulei.
Flags: needinfo?(xyuan)
Attached patch gecko patch v2 with test. (obsolete) — Splinter Review
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.
Attachment #8344166 - Attachment is obsolete: true
Attachment #8348506 - Flags: review?(fabrice)
(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.
Attachment #8348506 - Flags: review?(fabrice) → review+
Add the missing file...
Attachment #8348506 - Attachment is obsolete: true
Comment on attachment 8350057 [details] [diff] [review]
gecko patch v2 with test

test passed :-)
Attachment #8350057 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/be94be87a84c
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Depends on: 953044
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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.
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Flags: needinfo?(whsu)
Resolution: --- → FIXED
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!  :)
Flags: needinfo?(whsu)
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!
Flags: needinfo?(xyuan)
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.
Flags: needinfo?(xyuan)
(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
You need to log in before you can comment on or make changes to this bug.