1102141 - Cannot deactivate English keyboard layout in Settings (despite other keyboard layouts being enabled)

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System::Input Mgmt, defect)

Description

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

We can no longer deselect the English keyboard layout in Settings even though other keyboard layouts are enabled.

STR:
1. Go to Settings -> Keyboards -> Select Keyboards
2. In Built-in Keyboard, select Portuguese, French and English (in addition to "Number").
3. Deselect English. Note, you should NOT be warned about needing to enable English as default keyboard.
4. Go back

Expected:
Only Portuguese and French keyboards have been enabled (in addition to "Number")

Actual:
Portuguese, French, and English keyboards have been enabled (in addition to "Number")

Environment:
Gaia-Rev        e64428c5b2dce5db90b75a5055077a04f4bd4819
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/aa72ddfe9f93
Build-ID        20141119160202

Checked and this issue does NOT repro on 2.1, so let's directly flag regressionwindow-wanted.

P.S. My system language is English (US), which was selected in FTU and was not changed since that.

Comment 1

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

Alright, did a quick bisect and part 3 of bug 1094561 was it. Tim, could you check?

(FYI, I had to revert both bug 1094559 and bug 1094561 to confirm it. Reverting only bug 1094559 would not fix this bug. Also my information ten minutes ago regarding bug 1094561 not being offensive was wrong -- didn't realize it had three parts.)

Comment 2

[Mozilla Shepherd (This account is used for automation and doesn't receive bugmail)]

Attachment: mozilla-b2g:master PR#26350

Comment 3

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

Comment on attachment 8526518 [details] [review]
mozilla-b2g:master PR#26350

The root cause of this bug is because KeyboardHelper.checkDefaults() expects getApps() to be in sync. In the regressed bug I changed it to be always async which make the method fail.

https://github.com/mozilla-b2g/gaia/blob/18b075b0b7d083ac4f27dc4f837f6e6bde85c2bc/shared/js/keyboard_helper.js#L572-L590

The alternative solution to this patch would be fix this method too, but I don't want to scope creep ...

Comment 4

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

Comment on attachment 8526518 [details] [review]
mozilla-b2g:master PR#26350

Let's add a test to guard this behavior? Also it kinda worries me that the patch didn't break Gu at all.

Comment 5

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

Comment on attachment 8526518 [details] [review]
mozilla-b2g:master PR#26350

You are making a great point -- KeyboardHelper.getApps() was never covered in the tests!

Comment 6

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

Comment on attachment 8526518 [details] [review]
mozilla-b2g:master PR#26350

Looks awesome! Just that the second test case name should be "inputAppList ready" instead of "inputAppList not ready".

Comment 7

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

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

Comment 8

[sarsenyev]

Verified the issue is fixed on 2.2 Flame

When English is diactivated is no longer appears as selected language

"Flame 2.2
Device: Flame 2.2 (319mb)(Kitkat Base)(Shallow Flash)
Build ID: 20141121040204
Gaia: 25388c6bce932657ebf93adedf31881bfaf88c15
Gecko: 3366c0fcf9c2
Version: 36.0a1 (2.2)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0"

Comment 9

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

2.2+ all fixed regressions -- reset if disagree.