If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED in Firefox OS v1.2

Status

Firefox OS
Gaia::First Time Experience
P1
normal
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mihai, Assigned: rudyl)

Tracking

unspecified
1.2 C4(Nov8)
Dependency tree / graph

Firefox Tracking Flags

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

Details

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

Attachments

(2 attachments)

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 +++
(Reporter)

Updated

4 years ago
See Also: → bug 828237
(Reporter)

Updated

4 years ago
Depends on: 913782
Created attachment 801255 [details]
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.
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.
(Reporter)

Updated

4 years ago
Depends on: 920431
(Reporter)

Updated

4 years ago
Duplicate of this bug: 915396
(Reporter)

Updated

4 years ago
Duplicate of this bug: 925283

Comment 7

4 years ago
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

4 years ago
I'd be happy to take this bug and finish it as well.
Duplicate of this bug: 929390
Assignee: mihai → rlu
Status: NEW → ASSIGNED
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)
Created attachment 828832 [details] [review]
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.
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.

Etienne,

Could you please help on reviewing this?
Thanks.
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,
https://github.com/mozilla-b2g/gaia/commit/cc9fec48957c9d217221d403aac4b5f35a826dd6

:fcampo,
Thanks for the review.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Attachment #828832 - Flags: review?(etienne)
Uplifted cc9fec48957c9d217221d403aac4b5f35a826dd6 to:
v1.2: cac92434fe84216f49887582af6322d2b6f8356c
status-b2g-v1.2: --- → fixed
You need to log in before you can comment on or make changes to this bug.