Closed Bug 1061439 Opened 6 years ago Closed 6 years ago
Prevent IMEngine from touching layouts when inputcontext is gone
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.
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.
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
(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!
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...
Summary: Keyboard 2.1 often misses first keystroke → latin.js failed to handle sendKey() rejection with a specific Rocketbar STR
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.
Summary: latin.js failed to handle sendKey() rejection with a specific Rocketbar STR → Prevent IMEngine from touching layouts when inputcontext is gone
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.
Attachment #8483190 - Flags: review?(rlu) → review+
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
Attachment #8483190 - Flags: approval-gaia-v2.1?(fabrice)
+1 for approval-gaia-v2.1, it's not just rocketbar (which I don't use), also occasionally in browser, HERE maps, etc.
Attachment #8483190 - Flags: approval-gaia-v2.1?(fabrice) → approval-gaia-v2.1+
(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.
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
You need to log in before you can comment on or make changes to this bug.