Closed Bug 1054184 Opened 10 years ago Closed 10 years ago

Keyboard 2.1 often misses first keystroke

Categories

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

x86
macOS
defect
Not set
normal

Tracking

(blocking-b2g:2.1+, b2g-v2.1 verified, b2g-v2.2 verified)

VERIFIED FIXED
2.1 S4 (12sep)
blocking-b2g 2.1+
Tracking Status
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: janjongboom, Assigned: timdream)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
I've seen this happen in pretty much any input field. I think it also happens with the backspace key as well.
[Blocking Requested - why for this release]: Makes the keyboard very frustrating to use. We should figure this out in 2.1.
blocking-b2g: --- → 2.1?
Keywords: regression
Before getting a window here - we're going to need clearer STR here. Once we have that, then we can move forward with doing branch checks & getting a window.
(In reply to Jason Smith [:jsmith] from comment #3)
> Before getting a window here - we're going to need clearer STR here. Once we
> have that, then we can move forward with doing branch checks & getting a
> window.

Just dogfood trunk for a few minutes and the STR is easy. Simply try typing into various inputs and the first key will be often be missed.
STR:
(1) make sure the browser app isn't running.
(2) open the browser
(3) start typing something to the location bar 
... the first keystroke isn't handled.
QA Contact: ddixon
Branch Check

I used the STR listed in Comment 5. 

Repro Rate: 1/12 

Issue DOES occur in Flame 2.1, 2.0, 1.4 and Buri 2.1

(Flame devices tested with 319 MB memory)

Device: Flame Master
Build ID: 20140818002014
Gaia: aa8aace12d65956dd9525da5dac66e0d3b28597f
Gecko: 0aaa2d3d15cc
Version: 34.0a1 (Master)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.0
Build ID: 20140818021116
Gaia: 640ce38ca03f1e26a4524ff4215b8b3f7731e2f0
Gecko: 692c93509dc9
Version: 32.0 (2.0)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Device: Flame 1.4
Build ID: 20140815080714
Gaia: 93b469e3643f6caed8e190c72b612c60f6a74a1e
Gecko: 4492ce0cec4e
Version: 30.0 (1.4)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Device: Buri 2.0
Build ID: 20140811110610
Gaia: 1144cdc3a544f81c9bf71598aba1cb67d6c95a29
Gecko: c4768df8f483
Version: 32.0 (2.0)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
-----------------------------------------------------------------------------------
-----------------------------------------------------------------------------------
DOES NOT occur in Flame v123 base image. 

Device: Flame 1.3
Build ID: 20140627162151
Gaia: 5c43d012a9351e8aaeacda0b87b0886b7f67d33d
Gecko: e181a36ebafaa24e5390db9f597313406edfc794
Version: 28.0 (1.3)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:28.0) Gecko/28.0 Firefox/28.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
according to those results this is not a regression - removing regression related keywords.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
Hmm, this is *definitely* way worse than 2.0 and 1.4. I am fairly certain we did not perform the branch checks correctly.
Can someone who is seeing this bug on 2.1 include a video of the bug? That could help make sure that we're all analyzing the same bug here.
This is definitely a recent regression. I'd say started to happen during the last two weeks or so.
And based on comment 6, this is a regression. Doesn't happen in 1.3 at all (and is apparently worst in 2.1).
In order to do a window here, we'll need to get a video clarifying the bug here so that our testers know what to do a window against. Could you provide a video showing what you are seeing?
Flags: needinfo?(bugs)
Just an anecdotal +1 on this definitely being a regression from 2.0 to 2.1. I'm dogfooding 2.0 as my main phone with no problems but see this regularly while developing on master. It is intermittent and difficult to capture on video.
Alright. Switching this to steps-wanted - let's try to reproduce this with input fields in other apps (e.g. contacts). If reproduced, then branch check. Accordingly to the above comments, we should have the right STR if this happens on 2.1, but not 2.0.
QA Whiteboard: [QAnalyst-Triage+]
Flags: needinfo?(bugs)
Tested with 2.1 Flame device with 512 MB memory. 

STR: 

Browser App: 

1. Search "Google" in Browser.
2. Tap in Browser search field to highlight the web address. 
3. Type a new web address. 

 Or

1. Go to a graphically intense website such as "ign.com" 
2. Tap in web address bar to highlight current URL
3. Type in a new URL

Contacts App: 

1. Add Contact
2. Tap in any field. 
3. Quickly switch to a different field and tap on the keyboard

Messages App: 

1. Create new message. 
2. Tap "Message" to activate cursor
3. Immediately tap on any key after activating cursor

Actual Results: 
First keystroke does not register. 

Repro Rates: 
Browser App= 1/10
Contacts App= 9/10
Messages App= 5/10
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: steps-wanted
Triage: maybe-regression.

Let me see what I can do.
Assignee: nobody → timdream
Status: NEW → ASSIGNED
blocking-b2g: 2.1? → 2.1+
Jan, after discussing with Rudy a bit about this, I think this might be a 2.1 regression due to the fact stop keeping the previous inputContext object when it was removed by inputcontextchange event. 

If the user changes focus between two inputs, there is always a short time between two inputcontextchange event where mozInputMethod.inputcontext will be null but the keyboard remain visible. That might be the timing when your key strokes are missing.

Anyway, could you try to reproduce this locally and |adb logcat| for any keyboard JS errors? My educated guess is that we should see "TypeError: inputContext is null" since we will be trying to run inputContext.sendKey() at the time from the default input method engine*.

* Since 2.1, we switch away input method engine (while keep the rendering) when the inputContext is lost. That might be another issue contributing to this regression.
Flags: needinfo?(janjongboom)
PS the regression might be any of the bug under bug 1023729.
Yeah, let me see when it happens and logcat.
Flags: needinfo?(janjongboom)
(In reply to Jan Jongboom [:janjongboom] (Telenor) from comment #18)
> Yeah, let me see when it happens and logcat.

\o/ that means this can be fixed!
Duane - I don't see any testing done on 2.0 with your steps from comment 14.  Please review comment 13 and complete the task.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell) → needinfo?(ddixon)
Keywords: qawanted
I see no output in logcat when the keypress is dropped. Keon running 2.1 (nightly) build 20140821221403.
Branch Check: 

Following checks performed with 512 MB memory on a Flame device. 

-----------------------------------------------------
Flame 2.1 

Repro? 
Contacts App: Yes
Messages App: Yes
Browser App: Yes

Device: Flame Master
BuildID: 20140822010750
Gaia: afcdd36f13e75adcdebe57d842a277fd587faf28
Gecko: 0b9dd32d1e16
Version: 34.0a1 (Master) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
-----------------------------------------------------
2.0 Flame

Repro? 
Contacts App: Yes
Messages App: Yes
Browser App: No

Device: Flame 2.0
BuildID: 20140822100753
Gaia: 06edd086387c2150017b549e6318a61cd7e4fd02
Gecko: d946233724d5
Version: 32.0 (2.0) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
-----------------------------------------------------
Flame 1.4 

Repro? 
Contacts App:  Yes
Messages App: Yes 
Browser App: No

Device: Flame 1.4
BuildID: 20140821084113
Gaia: f63cdae1a06c808443ed14de3cd13b61311426e0
Gecko: 1cacad481f49
Version: 30.0 (1.4) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ddixon) → needinfo?(jmitchell)
Keywords: qawanted
QA-Wanted triage:  Not adding regression window tag - Based on the results the aspect of this bug that is a regression between 2.0 and 2.1 is its appearance in the browser app - comment 14 indicates a repro rate of 1/10 for 2.1 Browser repro's. This rate is too low to do a reliable window against.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
See Also: → 1057898
(In reply to Ralph Giles (:rillian) from comment #21)
> I see no output in logcat when the keypress is dropped. Keon running 2.1
> (nightly) build 20140821221403.

Yeah, my guess was wrong here. I did some investigation on the Contact app case in comment 14 and here is what I found:

It turned out I called |activeTargetsManager.clearAllTargets()| too early when the layout is being switched away. That function invalidates all user interactions on the layout being switch away, even for the occasions of switching to the same layout. When this happens there will be no logcat since this is considered "normal".

I have a fix that can take the the keyboard back to what it behaved back in v2.0, by adding a small delay. Will push later today or tomorrow.

That said, I filed bug 1057898 to the input method API, since there are two events when user tap from one input to another. There is always be a small window where the key won't get send out even if we want to. logcat will output when this happens. I think that's what Duane is seeing in comment 22 but I really can't fix it in this bug as a blocker (and I shouldn't since it's not the regression covered by this bug). Therefore, I am marking v1.4 and v2.0 as wontfix even though STR reproduces on these branches if one tries hard enough.
Comment on attachment 8478105 [details] [review]
mozilla-b2g:master PR#23258

Here is that patch that should fix the issue. A timeout is introduced so we don't clear active targets right away when the inputcontext is gone. Nor we deactivate the current IMEngine right away. After that,

-- If the user happen to lift her/his finger before we get the new context of the next input field (in the 6ms window!) she/he will hit by bug 1057898 (not covered by this bug)
-- When the new inputcontext event is received, the deactivation timer will be effectively cancelled.
-- The current (old) layout will continue to be functional with the new inputcontext until the new layout is rendered
-- There will be a small window before the rendering is rendered, that the user is effectively sending a key on the old layout to the new IMEngine. That's harmless to our use case here but it might be worthy fixing in the future.

I have change the test to use MockPromise I wrote in bug 1056842 so it's pretty clear on what the StateManager is doing step-by-step, including the case when deactivation/activation is interrupted by the opposite action. (Although I am not happy with the verboseness of the test script.)

As demonstrated in the test, this should be good enough (in terms of managing concurrency) for now. This however will not work if user purposefully activate, and deactivate, *and* activate the keyboard again, but I highly doubt this is a real world scenario and adding that in the code will make it a lot messier since
promise doesn't offer that out of box. We can always do that if we need to, in
the next patch *after* FL.
Attachment #8478105 - Flags: review?(rlu)
Attachment #8478105 - Flags: feedback?(janjongboom)
Correction -- my previous claim on 6ms is incorrect, it's actually around 20ms on a Flame. Maybe more on low-end device.
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
Comment on attachment 8478105 [details] [review]
mozilla-b2g:master PR#23258

This change looks fine except for some typos, r=me.
Thank you.
Attachment #8478105 - Flags: review?(rlu) → review+
Since CI already returns green on the previous run I am not going to wait for it after just correcting some typo.

https://tbpl.mozilla.org/?rev=061ad0191b4996302ff347f1f59a53ff35654aa2&tree=Gaia-Try

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

For the remaining possibilities that make the STR reproducible on all branches, see bug 1057898.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Attachment #8478105 - Flags: feedback?(janjongboom)
Should this be in the latest nightlies? Since I don't really see any difference, still missing the first keystroke all the time.
(In reply to Olli Pettay [:smaug] from comment #33)
> Should this be in the latest nightlies? Since I don't really see any
> difference, still missing the first keystroke all the time.

:smaug, I am not entirely sure you are still hitting this bug or bug 1057898 because the symptom is the same. If it's bug 1057898 you will see a console warning 

https://github.com/mozilla-b2g/gaia/blob/master/apps/keyboard/js/keyboard/input_method_manager.js#L169-L170

If you see anything else in the log while still missing keys, please file a new bug and attach the log.

Thanks!
Flags: needinfo?(bugs)
I don't see anything like https://github.com/mozilla-b2g/gaia/blob/master/apps/keyboard/js/keyboard/input_method_manager.js#L169-L170
In fact I don't see anything relevant in the log. Do I need to set some pref perhaps to see
more logging?
Flags: needinfo?(bugs)
(In reply to Olli Pettay [:smaug] from comment #35)
> I don't see anything like
> https://github.com/mozilla-b2g/gaia/blob/master/apps/keyboard/js/keyboard/
> input_method_manager.js#L169-L170
> In fact I don't see anything relevant in the log. Do I need to set some pref
> perhaps to see
> more logging?

Sigh. We don't have anything that could produce more log in Keyboard app.

Let me try to reproduce tomorrow. Where in Gaia is the bug most reproducible to you?
In the "Search/Location bar" when initiated using the location bar of the new browser (the one which doesn't look like Firefox on Android)
I still can't reproduce this bug after updated to latest OTA. 

Jan, Kevin, would you be able to try it? I was going to needinfo you two after bug 1057898 is fixed but apparently that too complex and can't make it to v2.1.
Flags: needinfo?(kgrandon)
Flags: needinfo?(janjongboom)
I have encountered this twice today (WAY less than before). It's not a timing issue as I wait 2 seconds before keyboard pops up before typing character, and it happened to me when going from blur -> focus both times.
Status: RESOLVED → REOPENED
Flags: needinfo?(janjongboom)
Resolution: FIXED → ---
Thanks, but let's not reopen a bug w/o backing out the patch.

(In reply to Jan Jongboom [:janjongboom] (Telenor) from comment #39)
> I have encountered this twice today (WAY less than before). It's not a
> timing issue as I wait 2 seconds before keyboard pops up before typing
> character

This should be a new bug. Do you have any log with you on this?


> and it happened to me when going from blur -> focus both times.

I think this is bug 1057898.
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Yes, this still happens constantly on the latest master. Are you guys dogfooding with FirefoxOS as your primary device? It's driving me nuts, so I'm not sure how you haven't seen it.

I'll try to dig in this week and debug a bit.
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #40)
> > and it happened to me when going from blur -> focus both times.
> 
> I think this is bug 1057898.

Can you tell me where to stick some logging so I can confirm if it's bug 1057898 or not?
Flags: needinfo?(kgrandon) → needinfo?(timdream)
I am reopening because I don't think this is bug 1057898.

The reason I don't think it's bug 1057898 is because I added logging inside of state_manager.js and I only received a single inputcontextchange when this issue reproduced.

The received event order is:

First: visibilitychange
Followed by, inputcontextchange.

I have not observed any events or logging when the keystroke is missed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Kevin Grandon :kgrandon from comment #42)
> (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from
> comment #40)
> > > and it happened to me when going from blur -> focus both times.
> > 
> > I think this is bug 1057898.
> 
> Can you tell me where to stick some logging so I can confirm if it's bug
> 1057898 or not?

Exactly what I said in comment 34.

I've filed bug 1061439 and marked as 2.1+ so I can keep working on this.
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Flags: needinfo?(timdream)
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S4 (12sep)
This issue does NOT occur on the Flame 2.2 Master (319mb) and the Flame 2.1 KK (319mb)

Flame 2.2 Master KK (319mb) (Full Flash)

Device: Flame 2.2 Master
BuildID: 20141011040204
Gaia: 95f580a1522ffd0f09302372b78200dab9b6f322
Gecko: 3f6a51950eb5
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Flame 2.1 KK (319mb) (Full Flash)

Device: Flame 2.1
BuildID: 20141011000201
Gaia: f5d4ff60ffed8961f7d0380ada9d0facfdfd56b1
Gecko: d813d79d3eae
Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Result: Keyboard reponse time functions apropriately.
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+][lead-review+] → [QAnalyst-Triage?][lead-review+]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?][lead-review+] → [QAnalyst-Triage+][lead-review+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.