931977 - [Keyboard] Ensure first keyboard shown is properly initialized

Status: VERIFIED FIXED

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

Description

[James Lal [:lightsofapollo]]

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

Re-enable test_everythingme_launch_packaged_app.py (looks like timing based failures on TBPL)

Comment 1

[Corey Frang [:gnarf]]

Attachment: pr on github

Rudy, This seems to fix the problem in the test itself, however there is still something strange happening that you figured out in 912010, maybe we don't need to change it at all.

Comment 2

[Corey Frang [:gnarf]]

Rudy summarizes what breaks in the test really well here: https://bugzilla.mozilla.org/show_bug.cgi?id=912010#c58

Comment 3

[Zac C (:zac)]

This test used to be very reliable until today :(

Comment 4

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 823517 [details]
pr on github

This is a workaround until a more suitable fix is found. Could we add a comment preceding the line that's changed with a TODO and referencing the appropriate bug? Also, please include the bug number and reviewers in the commit message.

Comment 5

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

Comment on attachment 823517 [details]
pr on github

I don't think I am qualified to review the gaia-ui-tests code.

However, if possible I think we should dig into the root cause in keyboard app about the init process.
As my comment, Bug912010#c58, we should check why latin inputmethod was not activated correctly in this specific case.
 
Thanks.

Comment 6

[Corey Frang [:gnarf]]

Attachment: Pull Request on Github

I think tracked down why the inputMethod wasn't being initialized Rudy, does this make sense to you?

Comment 7

[David Flanagan [:djf]]

Corey's patch looks good to me, though I'd prefer to put the setKeyboardName() call in initKeyboard().

Comment 8

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

Not blocking but a fix is very welcome.

Comment 9

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

Find me to a+ this patch to v1.2 if that's what landed in master.

Comment 10

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

Comment on attachment 824142 [details] [review]
Pull Request on Github

r=me.

Corey,

Good work, thanks for your help to locate the point and fix it.


--
A note to myself,

It seems that in this specific case, mozvisibilitychange event would happen very late so that setKeyboardName() would not be invoked.                
https://bugzilla.mozilla.org/show_bug.cgi?id=845685#c10

Comment 11

[Corey Frang [:gnarf]]

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

Can we get approval to uplift this onto 1.2 tim?

Comment 12

[Corey Frang [:gnarf]]

Comment on attachment 824142 [details] [review]
Pull Request on Github

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): 912010
[User impact] if declined: It is possible that the first keyboard shown will not properly initialize its input method.  This means things like autocorrect, auto capitalization, etc might not work on the first keyboard displayed.
[Testing completed]: device and integration tests, everything still behaves properly
[Risk to taking this patch] (and alternatives if risky): low
[String changes made]: none

Comment 13

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

Comment on attachment 824142 [details] [review]
Pull Request on Github

My pleasure ;)

Comment 14

[John Ford [:jhford] CET/CEST Berlin Time]

I was not able to uplift this bug to v1.2.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1.2, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1.2
  git cherry-pick -x  b6221a5ecdeb1252c42f6a7b5628c90c1888346b
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 15

[John Ford [:jhford] CET/CEST Berlin Time]

This was uplifted in 90d2ede..cce2ba1

Comment 16

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

v1.2 reverted: 9d335a436d494c3703e98e195c6d94da4be9a15d

https://tbpl-dev.allizom.org/?rev=fc1043ab2d67&tree=Mozilla-B2g26-v1.2

Comment 17

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

v1.2 reland: 79368fd140ce7d965b533304fccc49148f5d3072

Comment 18

[William Hsu [:whsu]]

It seems to be a race condition.
I cannot manually verify it.
But the test result of TBPL works as normal after the patch was landed.
Marked as "Verified"