Closed Bug 913783 Opened 9 years ago Closed 9 years ago

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


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



(blocking-b2g:koi+, b2g-v1.2 fixed)

1.2 C4(Nov8)
blocking-b2g koi+
Tracking Status
b2g-v1.2 --- fixed


(Reporter: mihai, Assigned: rudyl)



(Whiteboard: [FT:System-Platform], [Sprint: 2], [3rd-party-keyboard])


(2 files)

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 +++
See Also: → 828237
Depends on: 913782
The new keyboard settings structure was integrated in the FTE language selection logic, thus ensuring the language-associated keyboard layout is enabled.
Attachment #801255 - Flags: review?(fernando.campo)
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 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 :)
Attachment #801255 - Flags: review?(fernando.campo) → review-
Thanks for checking it, Fernando! I will update the PR once the patch for bug 913782 lands, since there are a few dependencies there.
Depends on: 920431
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://": {"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.
I'd be happy to take this bug and finish it as well.
Assignee: mihai → rlu
Whiteboard: [FT:System-Platform], [Sprint: 2] → [FT:System-Platform], [Sprint: 2], [3rd-party-keyboard]
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
Needinfo on Rudy to help with an eta here and help set the target milestone .
Flags: needinfo?(rlu)
Flags: needinfo?(rlu)
Target Milestone: --- → 1.2 C4(Nov8)
Attached file 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.
Attachment #828832 - Flags: review?(fer.campo.garcia+bugzilla)
Comment on attachment 828832 [details] [review]
pull request 13480

Pull request update since Bug 913784 has landed.


Could you please help on reviewing this?
Attachment #828832 - Flags: review?(etienne)
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.
Attachment #828832 - Flags: review?(fer.campo.garcia+bugzilla) → review+
Patch updated to add the tests as you requested.

Merged to Gaia master,

Thanks for the review.
Closed: 9 years ago
Resolution: --- → FIXED
Attachment #828832 - Flags: review?(etienne)
Uplifted cc9fec48957c9d217221d403aac4b5f35a826dd6 to:
v1.2: cac92434fe84216f49887582af6322d2b6f8356c
You need to log in before you can comment on or make changes to this bug.