1135293 - [Windows Management][Keyboard] Opening an app that calls the keyboard, and then tapping home will result in an active keyboard on homescreen

Status: VERIFIED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

Attachment: Keybaord Issue Logcat

Description:
The Music widget can get destroyed if the user is playing a song and then takes a video from the lockscreen and then returns to the lockscreen after taking the video


Repro Steps:
1) Update a Flame to 20150220010206
2) Open Messages app
3) Tap on the compose message icon
4) quickly tap the home button


Actual:
Keyboard is active on the homescreen, and if the user taps and holds a key it will stay highlighted on the keyboard


Expected:
The keyboard is not active on the homescreen

Environmental Variables:
Device: Flame 3.0 (319mb)(Kitkat)(Full Flash)
Build ID: 20150220010206
Gaia: e4f7c67378e33e83f88d38ddb4a6c2cabf1423c3
Gecko: 1b4c5daa7b7a
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Repro frequency: 10/10
See attached: Logcat, Video - http://youtu.be/R8G4h0QPhM4

Comment 1

[Derek Harris [:DerekH] - Dont need info. Last day 6/5/15]

Attachment: Keys Stay Highlighted Picture

This bug does NOT occur on Flame 2.2

The keyboard does not stay active on the homescreen.

Environmental Variables:
Device: Flame 2.2 (319mb)(Kitkat)(Full Flash)
Build ID: 20150220002501
Gaia: ce79d35b92261e7cbfeaefebf87859ebeb0979b4
Gecko: b864abe1c6b3
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 37.0a2 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Comment 2

[Peter Bylenga [:PBylenga]]

[Blocking Requested - why for this release]:
Functional regression of a core feature.

Requesting a window.

Comment 3

[Pi Wei Cheng [:piwei] (inactive)]

This issue also occurs on 2.2. Keyboard could stay on Homescreen after invoking it in Messages app. It's more of a timing issue and repro rate is about 1 out of ~6.

Device: Flame 2.2 (shallow flash, 319MB mem)
BuildID: 20150222140402
Gaia: 389542b71c89253c0d176d3b0bfb54e275c19bf1
Gecko: 9fd3441c8983
Version: 37.0a2 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

-----

This issue does not occur on Flame 2.1, but behavior on 2.1 is a little different. When going back to Messages app from Homescreen, the focus is NOT automatically put to a field that would invoke the keyboard, therefore the possibility of encountering this issue decreases to 0 out of ~30 attempts. I'm leaning towards this is not a regression because the feature to resume focus to a field that would invoke the keyboard is not in 2.1.

Device: Flame 2.1 (shallow flash, 319MB mem)
BuildID: 20150222140631
Gaia: 88c44f2243a5ca1683587aca9faf29023974b96c
Gecko: efd5205ec813
Version: 34.0 (2.1) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 4

[howie [:howie]]

Triage: blocking on 3.0. ni Alive to have a look.

Comment 5

[(inactive after 6/18) Alive Kuo [:alive]]

Unfortunately I cannot reproduce. qawanted

Comment 6

[Pi Wei Cheng [:piwei] (inactive)]

It is dependent on timing. The repro rate is like 1 out of 20 attempts for me, and 1 out of 3 for the reporter on today's 3.0.

Revised STR:
1) Create a new message
2) Tap Home with keyboard already extended on new message page
3) Go back to Messages, and before keyboard is extended, tap Home
4) Repeat step 3 until bug occurs

Device: Flame 3.0 Master (full flash 319MB mem)
BuildID: 20150304010324
Gaia: 3fc0ac309f5fb0c1fe82c12223b955a4efce27e6
Gecko: c5b90c003be8
Gonk: e7c90613521145db090dd24147afd5ceb5703190
Version: 39.0a1 (3.0 Master)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:39.0) Gecko/39.0 Firefox/39.0

Comment 7

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I think it has to do with having the sms focused on a field and then hitting home when the keyboard is working on coming up.  It seems after I hit the wifi/rocketbar bug, the performance of the keyboard dismissing slowed down further.  

Having said that I don't get it to stay there.  Maybe I'm not tapping fast enough?

Comment 8

[(inactive after 6/18) Alive Kuo [:alive]]

Attachment: Logging patch

Hi, here is a logging patch, could you provide me the log once you reproduce? Thanks in advance.

Comment 9

[Pi Wei Cheng [:piwei] (inactive)]

Attachment: logcat with additional debugs enabled

Attaching the logcat with the patch applied to the phone.

I think I reproduced the bug within 10 attempts this time so the log shouldn't be too long.

Comment 10

[(inactive after 6/18) Alive Kuo [:alive]]

Thanks PiWei.

From the log I found that HomescreenWindow.focus() is correctly called each time going back to homescreen; however, KeyboardManager only gets blur event for 5 times.

George:
Could you help to investigate what's happening here? It might need to debug form.js in gecko to trace what's blocking the focus/blur to send to gaia. You could refer to Tim if you have problems.

Comment 11

[(Inactive after June) George Duan [:gduan] [:喬智]]

As Alive said, there's a missing mozChromeEvent of inputmethod-contextchange which should blur and make keyboard hide. 

I flash device to 20150220010206 and modify its memory to 319, but cannot reproduce even once.

Comment 12

[(Inactive after June) George Duan [:gduan] [:喬智]]

I track forms.js and found that
in https://dxr.mozilla.org/mozilla-central/source/dom/inputmethod/forms.js#306, we send unhandleFocus if focusedElement is element or not, however focusedElement might return null if the wrapper is dead (see https://dxr.mozilla.org/mozilla-central/source/dom/inputmethod/forms.js#235 ). 

So, I try below repro steps (a little different with comment 6)
1) Create a new message
2) Tap Home with keyboard already extended on new message page
3) Go back to Messages, and before keyboard is extended, tap Home *** and adb kill message app ***

Then, I successfully reproduce this bug easily. 

Hi Xulei,
If my assumption is correct, I think blur event should still be sent to gaia even if the wrapper is killed. Could you advise?

Comment 13

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

(In reply to George Duan [:gduan] [:喬智] from comment #12)
..
> 
> Hi Xulei,
> If my assumption is correct, I think blur event should still be sent to gaia
> even if the wrapper is killed. Could you advise?

Yes, if a normal app is killed or hide before keyboard hides, we should send a blur event to notify the keyboard. We had some code to guard against this:
https://github.com/mozilla/gecko-dev/blob/3bfd6947732ef41b734fa2be5b3e975a430575f5/dom/inputmethod/Keyboard.jsm#L113

But it doesn't seem to cover this bug's case.

Comment 14

[(Inactive after June) George Duan [:gduan] [:喬智]]

Hi Xulei,
this.formMM might be not equal to mm and blur event would not be sent out with comment 6's steps?
see https://github.com/mozilla/gecko-dev/blob/3bfd6947732ef41b734fa2be5b3e975a430575f5/dom/inputmethod/Keyboard.jsm#L71
I think gecko should send out inputmethod-contextchange blur event to gaia when sms crash and homescreen app focused.


Hi Alive,
I think the root cause is, sms app crash when we press home and the keyboard is just about to show up,
I don't have any good solution now, perhaps we can check 
(this._visible && layoutManager.keyboardEnabled) in _handle_mozbrowsererror of app_window.js and call hideInputWindowImmediately? Any advice?

Comment 15

[(inactive after 6/18) Alive Kuo [:alive]]

Thanks for hard investigation.

I need Tim's opinion about comment 14 in gecko part to decide if we need to workaround that in gaia; but I hope we could fix it in gecko.

Comment 16

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

Discussed offline -- George is going to try to fix the problem in comment 14. Changing the component to reflect the part of code to fix.

Comment 17

[(Inactive after June) George Duan [:gduan] [:喬智]]

I try to track the code again and realize that, when oop-frameloader-crashed happens,
https://github.com/mozilla/gecko-dev/blob/3bfd6947732ef41b734fa2be5b3e975a430575f5/dom/inputmethod/Keyboard.jsm#L113

it will send Keyboard:FocusChange to MozKeyboard.js and setInputContent to null,
https://github.com/mozilla/gecko-dev/blob/3bfd6947732ef41b734fa2be5b3e975a430575f5/dom/inputmethod/MozKeyboard.js#L239

and it will send a inputcontextchange to keyboard app (I put an console.log in state_manager.js in inputcontextchange and it does receive that message when step3 of comment 12, and navigator.mozInputMethod.inputcontext would be null but keyboard remains)
https://github.com/mozilla/gecko-dev/blob/3bfd6947732ef41b734fa2be5b3e975a430575f5/dom/inputmethod/MozKeyboard.js#L312

If it's expected, what we should do is to hide keyboard in keyboard app when inputcontextchange event happens and navigator.mozInputMethod.inputcontext is null, or call handleFocusChange with blur type msg in Keyboard.jsm because as far as I know, we hide keyboard in system app.

Hi Xulei,
could you advise? Please correct me if I misunderstood.

also cc rudy.

Comment 18

[John Lu [:mnjul][Please NI or I'll miss]]

From Gaia's perspective, we want to aleays hide keyboards with System app [*], as a lot of input management issues are tied-in with window management. Please see if that's possible to incorporate at Gecko side for dealing with this bug.

[*] The exception is we tell gecko to hide keyboard when user long-presses space key. But that's an excepTion.

Comment 19

[John Lu [:mnjul][Please NI or I'll miss]]

(Wow, the keyboard app I use on my phone generates typos...)

Also re-NI Xulei for comment 17 since George seems to have mis-cancelled it there.

Comment 20

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: 1013585.patch

Hi Xulei,
could you check my patch? 
I think we should send inputmethod-contextchange to system app. I've tested it and it works fine.

Comment 21

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8579100 [details] [diff] [review]
1013585.patch

# HG changeset patch
# User George Duan <[e-mail]>
# Parent 2ab2f85583c47c920c29cf2548a4a272bec9814d
Bug 1130028 - send inputmethod-contextchange to systemapp to hide keyboard when frame crash . r=yxl
diff --git a/dom/inputmethod/Keyboard.jsm b/dom/inputmethod/Keyboard.jsm
--- a/dom/inputmethod/Keyboard.jsm
+++ b/dom/inputmethod/Keyboard.jsm
@@ -110,16 +110,21 @@ this.Keyboard = {
     let frameLoader = subject.QueryInterface(Ci.nsIFrameLoader);
     let mm = frameLoader.messageManager;
 
     if (topic == 'oop-frameloader-crashed') {
       if (this.formMM == mm) {
         // The application has been closed unexpectingly. Let's tell the
         // keyboard app that the focus has been lost.
         this.sendToKeyboard('Keyboard:FocusChange', { 'type': 'blur' });
+        // Notify system app to hide keyboard.
+        SystemAppProxy.dispatchEvent({
+          type: 'inputmethod-contextchange',
+          inputType: 'blur'
+        });
       }
     } else {
       // Ignore notifications that aren't from a BrowserOrApp
       if (!frameLoader.ownerIsBrowserOrAppFrame) {
         return;
       }
       this.initFormsFrameScript(mm);
     }

Comment 22

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: 1135293.patch

Sorry, should be this one for review.

Comment 23

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

Comment on attachment 8579105 [details] [diff] [review]
1135293.patch

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

Nice catch!
Sorry for late review as I took a sick leave yesterday.

Comment 24

[(Inactive after June) George Duan [:gduan] [:喬智]]

Thanks!

Comment 25

[Ryan VanderMeulen [:RyanVM]]

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

Please double-check the bug number in your commit messages in the future. The attached patch had the wrong one.

Comment 26

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

https://hg.mozilla.org/mozilla-central/rev/60a191c34578

Comment 27

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

George:  On the latest Nightly Flame 3.0 build I'm still able to bring the keyboard following the steps in comment 0, but it will dismiss itself shortly after getting to the homescreen.  It takes about a second to dismiss but I can still type stuff with it for the duration it is up.  Is this the intended results of your patch or should it never be able to pop up at all?

Environmental Variables:
Device: Flame 3.0 KK (319MB) (Full Flash)
BuildID: 20150401010204
Gaia: 03164bd160809747e6a198e0dba1b7c3ee7789f5
Gecko: 18a8ea7c2c62
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Comment 28

[(Inactive after June) George Duan [:gduan] [:喬智]]

No, that's not an intended result of my patch.
My patch is to hide keyboard through gecko while background app is crashed, so you won't see the keyboard keep remaining on the homescreen while switching apps in some situations.

From my understanding to comment 27, gecko takes time to switching focus to sms app, if you press home during that switching, we do focus on homescreen but it requires time in gecko to blur previous app and send inputmethod-contextchange with blur type to gaia. Gaia has no idea when to close keyboard til we receive the event, that's why you might see the keyboard for around 1 sec.

Hi Xulei, could you correct me if I am wrong ?
I think that would be a different issue with this bug.

Comment 29

[John Lu [:mnjul][Please NI or I'll miss]]

(In reply to George Duan [:gduan] [:喬智] from comment #28)
> From my understanding to comment 27, gecko takes time to switching focus to
> sms app, if you press home during that switching, we do focus on homescreen
> but it requires time in gecko to blur previous app and send
> inputmethod-contextchange with blur type to gaia. Gaia has no idea when to
> close keyboard til we receive the event, that's why you might see the
> keyboard for around 1 sec.

I'm pretty sure it's correct. My original assumed/perceived fix for the bug is that Gaia correctly receives the blur event. And unfortunately we can't really control when that event will fire; under the current architecture/framework, what we can do our best to guarantee is it will eventually fires at some point, without the user further focusing/blurring any other input field when the bug manifests itself, but that can be some hundreds milliseconds or even a full second later, as in comment 27.

Still let's wait for Xulei for final comment. We may also ping Tim also to comment on my comment :)

Comment 30

[Pi Wei Cheng [:piwei] (inactive)]

This bug was originally about having the keyboard staying up/stuck on homescreen following str (showcased in https://www.youtube.com/watch?v=R8G4h0QPhM4 ). I will file a separate issue for what was observed on comment 27 (which is easily reproducible today).

This issue is verified as fixed on master and 2.5. Keyboard does not get stuck on homescreen after going to messages app. Repro rate is 0 out of 30 on each branch/device.

Device: Aries 2.6
BuildID: 20151207143802
Gaia: 24ed003a53a81f63367e265fa7117cbe7d23d4c8
Gecko: 59bc3c7a83de7ffb611203912a7da6ad84535a5a
Gonk: a19052e4389c3ae2d8fc3e7a74a475401baacc56
Version: 45.0a1 (2.6) 
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:45.0) Gecko/45.0 Firefox/45.0

Device: Flame 2.6
BuildID: 20151207030216
Gaia: 24ed003a53a81f63367e265fa7117cbe7d23d4c8
Gecko: 528ea05671e9bd9ccb33d1558a20691a72c85f98
Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a
Version: 45.0a1 (2.6) 
Firmware Version: v18Dv4
User Agent: Mozilla/5.0 (Mobile; rv:45.0) Gecko/45.0 Firefox/45.0

Device: Flame 2.5
BuildID: 20151207121611
Gaia: 2d54c29f429bed790b5d8284633812dc2b782518
Gecko: c491dedc389de5c4686543b990c92d4f47715ee8
Gonk: 205ac4204bbbb2098a8046444acba551ba5dc75a
Version: 44.0a2 (2.5) 
Firmware Version: v18Dv4
User Agent: Mozilla/5.0 (Mobile; rv:44.0) Gecko/44.0 Firefox/44.0