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

VERIFIED FIXED

Status

defect
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: mnjul, Assigned: timdream)

Tracking

({regression})

unspecified
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

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

Details

Attachments

(1 attachment)

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.
blocking-b2g: --- → 2.2?
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.)
Blocks: 1094561
Flags: needinfo?(timdream)
Assignee: nobody → timdream
Status: NEW → ASSIGNED
Flags: needinfo?(timdream)
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 ...
Attachment #8526518 - Flags: review?(jlu)
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.
Attachment #8526518 - Flags: review?(jlu) → review+
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!
Attachment #8526518 - Flags: review+ → review?(jlu)
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".
Attachment #8526518 - Flags: review?(jlu) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 8

5 years ago
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"
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
2.2+ all fixed regressions -- reset if disagree.
blocking-b2g: 2.2? → 2.2+
You need to log in before you can comment on or make changes to this bug.