913783 - [Keyboard][FTE] Enable language-associated keyboard when language is selected (for 3rd-party keyboard support)

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::First Time Experience, defect, P1)

Description

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

When selecting the system language during FTE, enable the language-associated built-in keyboard layout and if it is non-latin, enable the English keyboard layout as well (default latin layout).

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

Comment 1

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Attachment: Pull Request #12010 - [FTE] Enable language-associated kb

The new keyboard settings structure was integrated in the FTE language selection logic, thus ensuring the language-associated keyboard layout is enabled.

Comment 2

[Fernando Campo (:fcampo)]

Just a small comment about the duplicated code and tests. Rest looks fine, so I'll probably give the r+ when tests are working

Comment 3

[Fernando Campo (:fcampo)]

Comment on attachment 801255 [details]
Pull Request #12010 - [FTE] Enable language-associated kb

Just minusing it for you to request review again when tests are working and nits done, so I don't lose it in my queue :)

Comment 4

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Thanks for checking it, Fernando! I will update the PR once the patch for bug 913782 lands, since there are a few dependencies there.

Comment 7

[Corey Frang [:gnarf]]

I'm working on bug 912010 - and proposing changing the format of this setting.  The current one requires that all keyboards are listed.  I think we should instead set:

{"app://keyboard.gaiamobile.org": {"es": true, "number": true}}

as a plain object in two different keys - the keyboard.enabled-layouts and keyboard.default-layouts - 912010 takes care of reading these formats.

Comment 8

[Corey Frang [:gnarf]]

I'd be happy to take this bug and finish it as well.

Comment 10

[Mihai Cirlanaru [:mihai][:mcirlanaru]]

Hey Rudy, I can still work on this if the structure for keyboard_layouts.json is in place -- at that point I will only need to update the patch that I already submitted

Comment 11

[bhavana bajaj [:bajaj]]

Needinfo on Rudy to help with an eta here and help set the target milestone .

Comment 12

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

Attachment: pull request 13480

Hi Fernado,

This change (the last commit) mainly remove the related code and depends on KeyboardHelper.changeDefaultLayouts() to do the trick.

The related tests were also removed, will get the unit tests ready into keyboard_helper.

Please help review this patch.
Thanks.

Comment 13

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

Comment on attachment 828832 [details] [review]
pull request 13480

Pull request update since Bug 913784 has landed.

Etienne,

Could you please help on reviewing this?
Thanks.

Comment 14

[Fernando Campo (:fcampo)]

Comment on attachment 828832 [details] [review]
pull request 13480

Looks good, but I would like to have a test in FTU to check that a language change ends up in a keyboard change too. I understand that those tests are made on system/keyboard_helper_test, but as FTU deals also with it, I think is necessary to have them there too.

Anyway, you got the r+, as all the tests are passing, you only need to copy the test on ftu.

Comment 15

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

Patch updated to add the tests as you requested.

Merged to Gaia master,
https://github.com/mozilla-b2g/gaia/commit/cc9fec48957c9d217221d403aac4b5f35a826dd6

:fcampo,
Thanks for the review.

Comment 16

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

Uplifted cc9fec48957c9d217221d403aac4b5f35a826dd6 to:
v1.2: cac92434fe84216f49887582af6322d2b6f8356c