1061439 - Prevent IMEngine from touching layouts when inputcontext is gone

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Keyboard, defect)

Description

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

Apparently the patch in the cloned bug did not fix the issue.

Let me investigate and see that's missing in the fix.

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

Since I updated yesterday the keyboard misses my first keystroke every now and then Just switch to a textfield and press a key as fast as possible. Bubble appears, but key doesn't get inserted.

I can't really put my finger on a reproduction scenario, but the last time it happened for me in ConnectA2.

Comment 1

[Kevin Grandon :kgrandon]

Here is a STR which reproduces the issue for me 100% of the time: 

1 - Open rocketbar, enter URL and press enter.
2 - Page should now be in a browser frame.
3 - Tap on the rocketbar to focus.
4 - Press a character in the keyboard.

Expected STR:
Input clears and new character is shown.

Actual STR:
Input does not change. No 'input' event is delivered to the system app.

Comment 2

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

So according to the original bug report from Jan, the regressed build is the one around Aug 14. Here are the Gaia commit hashes from all the pvt builds around the time:

2014-08-13-04-02-02 9f35fca9d818b26c06aa6b7e5c0bef25886f8f20
2014-08-13-16-02-02 a2219a55145e730e56e09527b40152d68a43b0d9
2014-08-14-00-53-02 5e074831f9ddacdf6f622a6dffaecb626f740be8
2014-08-14-04-02-02 5e074831f9ddacdf6f622a6dffaecb626f740be8
2014-08-14-16-02-06 c7f8938522a18454809fb6ea0fd3eddef10a73ea
2014-08-15-04-02-04 9979fccc6321be72cd17370f3a20c65bc376af33
2014-08-15-16-02-05 fe933e0c958f80b241a9b580cad980ff1338b038

$  git log --oneline fe933e0c958f80b241a9b580cad980ff1338b038...9f35fca9d818b26c06aa6b7e5c0bef25886f8f20 -- apps/keyboard/
e57ea93 Merge pull request #22742 from yor-mozilla-com/gaia-header-keyboard
23b85ff Merge pull request #22819 from mnjul/bug_1024298_full_kb_panel_for_numbe
494dc6f Merge pull request #22401 from comoyo/bug1046798
8763293 Bug 1024298 - [Keyboard UX update] Show the full symbol panel for type="
e22f6d8 Bug 1015295 - [Keyboard] Update to use gaia-header
70697f9 Merge pull request #22572 from comoyo/bug992647
4d6a16f Bug 992647 - Add keyboards and wordlists for Irish, Scottish Gaelic and 
5cec5fe Merge pull request #22611 from bubuhuang/bug-1033186
517253f Merge pull request #22700 from RudyLu/keyboard/Bug1022609
bc467ce Bug 1033186 - Bosnian keyboard layout
5b08663 Merge pull request #22696 from timdream/keyboard-const
9599a7e Bug 1048095 - Remove "const" in keyboard app
2772c96 Bug 1022609 - Define 2 different symbol panels that includes ',' or not.
20446dc Bug 1046798 - Yield uppercase suggestions if input is uppercase

Comment 3

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

(In reply to Kevin Grandon :kgrandon from comment #1)
> Here is a STR which reproduces the issue for me 100% of the time: 
> 
> 1 - Open rocketbar, enter URL and press enter.
> 2 - Page should now be in a browser frame.
> 3 - Tap on the rocketbar to focus.
> 4 - Press a character in the keyboard.
> 
> Expected STR:
> Input clears and new character is shown.
> 
> Actual STR:
> Input does not change. No 'input' event is delivered to the system app.

Thank you. I can reproduce this!

Comment 4

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

I can now confirm this is a distinct bug unrelated to bug 1054184 but related to how latin.js works with rejected sendKey() request. It might still be a regression.

I have a patch ready that transforms PerformanceTimer into a KeyboardConsole object and I find out this based on the log. I will annotate latin.js to fix this...

Comment 5

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

So this is my WIP that works w/o any unit tests.

https://github.com/timdream/gaia/tree/keyboard-rocketbar-miss

The root cause is because we have something quirky here: 

1) when the user hits search, the rocket bar input field is destroyed and we receives a inputcontextchange event.
2) the sendKey() promise is properly rejected, and the rejection is properly handled.
3) latin.js handles the (now resolved) promise by resetting the layout page and uppercase etc.
4) however, since inputcontext is removed, when latin.js call these methods on the keyboard, we get a JS error from RenderingManager because we can't redraw the layout w/o modified layout JSON structure.

So my first commit update the keyboard app to include an console module to find out where the problem is.
The second commit patches latin.js, allowing the JS error in (4) to be visible in the console, and recover from it.
The third commit actually fix the problem by blocking all calls from IMEngine from Glue when the inputcontext is not available.

I will finish the unit tests tomorrow and submit for review.

Comment 6

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

Attachment: Github: https://github.com/mozilla-b2g/gaia/pull/23655

Comment 7

[Rudy Lu [:rudyl] (inactive after 2015/7/3)]

Comment on attachment 8483190 [details] [review]
Github: https://github.com/mozilla-b2g/gaia/pull/23655

Looks good to me, r+.

Thanks for the patch and the face-to-face discussion.

Comment 8

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

master: https://github.com/mozilla-b2g/gaia/commit/f6ab17186b4617a6669b15f4495f847dbd0dec42

Comment 9

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

Comment on attachment 8483190 [details] [review]
Github: https://github.com/mozilla-b2g/gaia/pull/23655

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): One of the key handling bug in 2.1 and combine with enabling of rocket bar
[User impact] if declined: The first key will be dropped for a common use case of rockbar (see comment 1)
[Testing completed]: tested manually
[Risk to taking this patch] (and alternatives if risky): only the 2 & 3rd commit actually affects the code flow, the first commit simply annotate the code with console. Not uplifting the first commit risk of diverging v2.1 and v.2.2 greatly.
[String changes made]: None

Comment 10

[Jan Jongboom [:janjongboom] (inactive)]

+1 for approval-gaia-v2.1, it's not just rocketbar (which I don't use), also occasionally in browser, HERE maps, etc.

Comment 11

[Ryan VanderMeulen (PTO, back July 27)]

v2.1: https://github.com/mozilla-b2g/gaia/commit/7c417d2d01d4254f3107f09ac83ee7dcf318eac3

Comment 12

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

(In reply to Jan Jongboom [:janjongboom] (Telenor) from comment #10)
> +1 for approval-gaia-v2.1, it's not just rocketbar (which I don't use), also
> occasionally in browser, HERE maps, etc.

One thing I didn't mention in comment 5 is that this bug affect whatever input that come after rocket bar, so it's very possible this has always been the same bug (including your original STR when your filed bug 1054184.)

Anyway, if there are still a reproducible step for missing keys, please file a bug I will fix it.

Comment 13

[Elie]

Attachment: VIDEO0045.mp4

This issue has been successfully verified on Flame 2.1 new versions:
Gaia-Rev        1b231b87aad384842dfc79614b2a9ca68a4b4ff3
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/95fbd7635152
Build-ID        20141119001205
Version         34.0
Device-Name     flame
FW-Release      4.4.2

See video:VIDEO0045