1184342 - Downloaded keyboards associated to a locale are not enabled by default when switching locale

Status: RESOLVED FIXED

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

Description

[Théo Chevalier]

STR:

1. Get a build with missing built-in keyboards, such as 'es' (Spanish) keyboard. So, es keyboard must not be built in.

2. Switch to en-US locale

3. Go to Settings, Keyboards, Built in keyboard, then download es keyboard from the CDN.

4. Switch to Spanish locale

5. Open Messages, type in.

Actual results:

Only en-US keyboard is enabled.

Expected results:

en-US and es keyboards should be enabled.


---------------


Issue does not occur with built-in layouts. If for instance you have a build with French keyboard built-in, when you switch locales, the French layout will be enabled as expected.
Note, the issue is not that layouts are not downloaded when switching locales (that would be Bug 1179973 and kind of a feature request). This bug is only about behavior for already downloaded layouts.

Layouts and locales are associated as described in this file: https://github.com/mozilla-b2g/gaia/blob/master/build/config/keyboard-layouts.json

Tested with latest master build on Flame.

Comment 1

[Théo Chevalier]

[Blocking Requested - why for this release]:

Downlodable keyboards are available on 2.5 only, so requesting to block this branch. This is a regression from previous releases where keyboards were automatically enabled when switching locales.

Comment 2

[Delphine Lebédel]

l10n triage: blocking on this as this is a regression in user experience (see comment 1), and is a broken new 2.5 functionality. Not code regression but definitely needs fix (please remove keyword if not appropriate).

Comment 3

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

This is not in the original scope.

Delphine, is the current behavior looks broken to you? 

We already give user the opportunity to enable the layout when it is first downloaded. While the behavior here is indeed inconsistent, I don't think this blocks the feature from shipping.

Comment 4

[Théo Chevalier]

I think I missed the part with the prompt (quickly hit 'Not now' then did the locale switch way later), so I agree this shouldn't be a blocker anymore (just checked with Delphine).
Thanks for the clarification, Tim :)

Comment 5

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

Unassign myself because I don't know the priority of this issue.

To fix this issue, the Gaia build script (specifically, keyboard-layout.js) would need to take account the layouts to be downloaded, and insert them into the generated default keyboard_layout.json accordingly.

keyboard_layout.json is taken account by KeyboardHelper when language switches. It should then attempt enable the layouts listed in the JSON, but it should not enable layouts that has not been made available.

Comment 6

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

New idea: let's make keyboard-layout.json static and include *all* layouts (dynamic or not) for all locales, and instead have KeyboardHelper to check the actual downloaded layouts and only turn those on. That would fix the behavior here and also kill a build step.

Comment 7

[GitHub Autolander Bot]

Attachment: [gaia] timdream:input-mgmt-move-locale-layout-map > mozilla-b2g:master

Comment 8

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

Needs to update accordingly

https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/Market_customizations_guide#Keyboard_settings

Comment 9

[Ray Lin[:ralin]]

Comment on attachment 8708688 [details] [review]
[gaia] timdream:input-mgmt-move-locale-layout-map > mozilla-b2g:master

Hi, Alberto

It took me a while to study keyboard in system/build part, sorry about that. After understanding the bug and look back to Tim's patch, I think this patch is good. 

Could you help to review this patch? Thanks a lot.

Comment 10

[Alberto Pastor [:albertopq]]

Hey Ray, unit and integration tests seems to be failing in this PR. Could you please verify if that's because of the patch? I'll take a look to the code in the meanwhile. Thanks!

Comment 11

[Ray Lin[:ralin]]

Alberto,

This is another PR with the same commits pulled from Tim's branch: https://github.com/mozilla-b2g/gaia/pull/33970
It looks fine with these commits (failed tests are caused by other apps). Thanks!!

Comment 12

[Alberto Pastor [:albertopq]]

Comment on attachment 8708688 [details] [review]
[gaia] timdream:input-mgmt-move-locale-layout-map > mozilla-b2g:master

The code looks good to me. I don't like landing patches while something is still failing in treeherder (even if is not caused by this), but I've been trying to fix the Gij11 and Gij18 issues without any luck (just sent an email about it) so let's merge this if is urgent. Thanks!

Comment 13

[Ray Lin[:ralin]]

https://github.com/mozilla-b2g/gaia/commit/6227ab91a3548070815d17a6836eef33fdbc8606