1013570 - [B2G][Keyboard]Double or triple tapping on the spacebar can result in 2-3 periods, as well as deleted characters

Status: RESOLVED FIXED

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

Description

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

Attachment: Logact Open C 1.4

Description:
Double or triple tapping on the spacebar can result in 2-3 periods, as well as deleting characters

Repro Steps:
1) Update a Open_C to BuildID: 20140513000208
2) Open the Calendar app> Tap on '+'
3) Tap inside one of the information fields such as the 'Notes' field
4) Type any number, or letter
5) Quickly tap the spacebar 3 times

Actual:
Sometimes 2-3 periods will be typed. Sometimes the number is sometimes deleted and replaced with periods. Sometimes there will be a space between the number or letter and the period

Expected:
The user sees 1 period directly after a number or letter that is typed.

Open C 1.4
1.4 Environmental Variables:
Device: Open_C 1.4
BuildID: 20140516000201
Gaia: 32fca83da31b9a0f9a5a88f96c913a25accdc14b
Gecko: a1e455367fa6
Version: 30.0
Firmware Version: P821A10V1.0.0B06_LOG_DL

Keywords:Double, Triple, Quickly, Fast, Rapidly, Multiple, Deleted, Removed

Notes: This issue occurs more frequently when using a number. Also the issue can repro after words. This issue occurs on all apps with a Keyboard

Repro frequency: 80% (this includes seeing any of the 3 'Actual' results)
See attached: Logcat, Video - http://youtu.be/gsmiX7taDMc

Comment 1

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

This Issue DOES occur on the Buri 1.4 and is easier to reproduce


1.4 Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140519000201
Gaia: defd0650fb9d30c6515d50a89e72d8fb74ce7e62
Gecko: d95594048b36
Version: 30.0
Firmware Version: v1.2-device.cfg

Comment 2

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

This issue does NOT occur on Buri 1.3

Buri 1.3: 

1.3 Environmental Variables:
Device: Buri 1.3 MOZ
BuildID: 20140514024003
Gaia: 96e3fa769a436a2182e6d54088fb41386eb2b5b5
Gecko: 685cf1d0dedb
Version: 28.0
Firmware Version: v1.2-device.cfg

Comment 3

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

This issue DOES occur on Flame 1.4

1.4 Environmental Variables:
Device: Flame 1.4 MOZ
BuildID: 20140520000201
Gaia: 17b102ee8d9a724b62285547cc5f1c5d30bfb01c
Gecko: 95be84248033
Version: 30.0
Firmware Version: v10f-3

Comment 4

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

Central Nightly/RIL Regression Window:

Last Working Environmental Variables:
Device: Buri MOZ
BuildID: 20140102040201
Gaia: 67a82f88da231969efa4d22df9fb946abf2cf4df
Gecko: 540d85f60c57
Version: 29.0a1
Firmware Version: v1.2-device.cfg

First Broken Environmental Variables:
Device: Buri MOZ
BuildID: 20140103040201
Gaia: 83cc63f728489a24256731adf558354bb2012a59
Gecko: 49d2fce9a86c
Version: 29.0a1
Firmware Version: v1.2-device.cfg

Last Working Gaia / First Broken Gecko: Issue Does NOT reproduce
Gaia: 67a82f88da231969efa4d22df9fb946abf2cf4df
Gecko: 49d2fce9a86c

Last Working Gecko / First Broken Gaia: Issue DOES reproduce
Gaia: 83cc63f728489a24256731adf558354bb2012a59
Gecko: 540d85f60c57

Gaia Pushlog:
https://github.com/mozilla-b2g/gaia/compare/67a82f88da231969efa4d22df9fb946abf2cf4df...83cc63f728489a24256731adf558354bb2012a59

Note:
A narrower window is not possible for this date.

Comment 5

[howie [:howie]]

Hi Tim, may you or Rudy take this? Thanks.

Comment 6

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

(In reply to Pi Wei Cheng from comment #4)
> Gaia Pushlog:
> https://github.com/mozilla-b2g/gaia/compare/
> 67a82f88da231969efa4d22df9fb946abf2cf4df...
> 83cc63f728489a24256731adf558354bb2012a59
> 
> Note:
> A narrower window is not possible for this date.

Look like bug 952078. Jan, do you have cycles to address this?

Comment 7

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

I'll take a look this week.

Comment 8

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

Attachment: WIP - pull request 19856

[Solution]
 1. Send the next key after the promise returned by the previous sendKey() invocation is resolved.
 2. In order to also solve bug 987809, we should send the correct keyCode based on the right capitalization state (also after the previous promise is resolved).
    For this change, I think keyboard.js should pass to the input method with a keyObject which contains both the lowercase and uppercase keycodes.

This should be easier to give feedback based on, 
https://github.com/mozilla-b2g/gaia/pull/19856/files?w=1

Tim, could you please help feedback on the above approach?
Thank you.

Comment 9

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

Comment on attachment 8432204 [details] [review]
WIP - pull request 19856

With a offline discussion, I am convinced this is a valid 1.4/2.0 solution to the problem. I would like to see proper fix (a queue) implemented in keyboard app itself and not the IME.

I have made some comments on Github to see if we could get a better clarity of this addition.

Comment 10

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

Comment on attachment 8432204 [details] [review]
WIP - pull request 19856

Patch updated to address the review comments.

Tim, could you please help review this again?
Thank you.

Comment 11

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

Comment on attachment 8432204 [details] [review]
WIP - pull request 19856

Discussed offline.

Comment 12

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

Comment on attachment 8432204 [details] [review]
WIP - pull request 19856

Hi Tim,

Patch updated for:
 1. Separate the 2 then() functions to make the code easier to read, and add some comments for the handling sequence.

 2. I found we still need to return the promise in click(), or in the tests, we could not know when the clicks have been processed.

 3. It seems that in handleKey() function, we already handle the case when inputContext.sendKey() is rejected, however, for safety, I also add a empty function as the onRejected paremeter to the 2nd then().

Please help review it again and thanks for your patience.

Comment 13

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

Landed to Gaia master,
https://github.com/mozilla-b2g/gaia/commit/f0a8ab7d0b6000cc2c2cd2a201964bafa959858b

Tim, thanks for the review.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Needs a branch-specific patch for v1.4 uplift.

Comment 15

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

Attachment: Patch for v1.4

I have a patch for v1.4 branch, however the Travis CI always failed recently.
Working on bug 1023741 and bug1023694 to get it green again.

Keep the ni so that this will be in my queue.

Comment 16

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

Uplifted to v1.4,
https://github.com/mozilla-b2g/gaia/commit/8d528bba5fc29b674bdef21f5133967996614c37

The travis CI is green,
https://travis-ci.org/mozilla-b2g/gaia/builds/27371939

Comment 17

[Sue]

Attachment: video of issue verify

This issue has been verified successfully on Flame 2.1
See attachment: verify_video.MP4
Reproducing rate: 0/5
Flame 2.0 versions:
Gaia-Rev        99e4594c66aa3738d58b0cb44bd885a87a063b6e
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/f91abc6127d9
Build-ID        20141125000201
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141125.035023
FW-Date         Tue Nov 25 03:50:34 EST 2014
Bootloader      L1TC00011880

Flame 2.1 versions:
Gaia-Rev        1bdd49770e2cb7a7321e6202c9bf036ab5d8f200
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/db893274d9a6
Build-ID        20141125001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141125.040617
FW-Date         Tue Nov 25 04:06:28 EST 2014
Bootloader      L1TC00011880