Closed
Bug 913783
Opened 11 years ago
Closed 11 years ago
[Keyboard][FTE] Enable language-associated keyboard when language is selected (for 3rd-party keyboard support)
Categories
(Firefox OS Graveyard :: Gaia::First Time Experience, defect, P1)
Firefox OS Graveyard
Gaia::First Time Experience
Tracking
(blocking-b2g:koi+, b2g-v1.2 fixed)
Tracking | Status | |
---|---|---|
b2g-v1.2 | --- | fixed |
People
(Reporter: mihai, Assigned: rudyl)
References
Details
(Whiteboard: [FT:System-Platform], [Sprint: 2], [3rd-party-keyboard])
Attachments
(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 +++
Reporter | ||
Comment 1•11 years ago
|
||
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)
Comment 2•11 years ago
|
||
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•11 years ago
|
||
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-
Reporter | ||
Comment 4•11 years ago
|
||
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•11 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.
Assignee | ||
Updated•11 years ago
|
Assignee: mihai → rlu
Status: NEW → ASSIGNED
Whiteboard: [FT:System-Platform], [Sprint: 2] → [FT:System-Platform], [Sprint: 2], [3rd-party-keyboard]
Reporter | ||
Comment 10•11 years ago
|
||
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•11 years ago
|
||
Needinfo on Rudy to help with an eta here and help set the target milestone .
Flags: needinfo?(rlu)
Assignee | ||
Updated•11 years ago
|
Flags: needinfo?(rlu)
Target Milestone: --- → 1.2 C4(Nov8)
Assignee | ||
Comment 12•11 years ago
|
||
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)
Assignee | ||
Comment 13•11 years ago
|
||
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 14•11 years ago
|
||
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+
Assignee | ||
Comment 15•11 years ago
|
||
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
Closed: 11 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•11 years ago
|
Attachment #828832 -
Flags: review?(etienne)
Comment 16•11 years ago
|
||
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.
Description
•