1128396 - Include all layouts with no dictionary or downloadable dictionary

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Keyboard, defect)

Description

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

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

Bug 1029951 will be landed disabled -- that is, by setting the preload layouts === loaded layouts, the UI will not be shown and the patch will have no effect.

When we feel ready, we could enable that by doing

GAIA_KEYBOARD_LAYOUTS=noPreloadDictRequired,en,pt-BR,es,de,fr,fr-CA,pl,ko,zh-Hans-Pinyin,en-Dvorak
GAIA_KEYBOARD_PRELOAD_DICT_LAYOUTS=en,pt-BR,es,de,fr,fr-CA,pl,ko,zh-Hans-Pinyin,en-Dvorak

To load all layouts that doesn't require a preloaded dictionary *and* preload dictionaries we have already preloaded.

There are still some UIs to be patched before we could enable the feature on master. We would use dependency to track that.

Comment 1

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

Actually, |noPreloadDictRequired| does not only cover Latin-IMEngine based layouts, but all layouts w/o a dictionary or doesn't need a preloaded dictionary.

Updating summary.

Comment 2

[Matěj Cepl]

What's stopping this bug from being RESOLVED?

Comment 3

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

(In reply to Matěj Cepl from comment #2)
> What's stopping this bug from being RESOLVED?

Actually enable it in on master...? I am sorry I was busy at something else.

(In reply to Tim Guan-tin Chien [:timdream] (slow response; please ni? to queue) from comment #0)
> GAIA_KEYBOARD_LAYOUTS=noPreloadDictRequired,en,pt-BR,es,de,fr,fr-CA,pl,ko,zh-Hans-Pinyin,en-Dvorak
> GAIA_KEYBOARD_PRELOAD_DICT_LAYOUTS=en,pt-BR,es,de,fr,fr-CA,pl,ko,zh-Hans-Pinyin,en-Dvorak

This might not be the right way to re-purpose the GAIA_KEYBOARD_LAYOUTS value. If someone enables a layout in her local.mk today, she would have to change GAIA_KEYBOARD_PRELOAD_DICT_LAYOUTS too after this patch lands.

I should think of some way that could carry over the meaning there.

Comment 4

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

For example, the variable is overwritten in our flame-kk builds.

https://dxr.mozilla.org/mozilla-central/source/b2g/config/flame-kk/config.json#37

Comment 5

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

I've concluded we should do this instead:

# This variable is for customizing the keyboard layouts in a build.
# Include the ID of the layout in this variable will make both the dictionary
# and the layout included in the package.
GAIA_KEYBOARD_LAYOUTS?=en,pt-BR,es,de,fr,fr-CA,pl,ko,zh-Hans-Pinyin,en-Dvorak
# We optionally offers downloading the dictionary from the CDN, instead of
# including it in the build.
# Include the ID of the layout in this variable will make it appear in the
# keyboard settings page, allowing user to download the dictionary.
GAIA_KEYBOARD_DOWNLOADABLE_LAYOUTS?=noPreloadDictRequired

I will change the keyboard build script (and tests) to do exactly that, and enable the feature in the second commit.

Comment 6

[GitHub Autolander Bot]

Attachment: [gaia] timdream:keyboard-dyn-switch > mozilla-b2g:master

Comment 7

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

Comment on attachment 8601305 [details] [review]
[gaia] timdream:keyboard-dyn-switch > mozilla-b2g:master

r=me, thanks.

Comment 8

[GitHub Autolander Bot]

http://docs.taskcluster.net/tools/task-graph-inspector/#IJXpf2ZfQV6AyKqOKu4oew

The pull request failed to pass integration tests. It could not be landed, please try again.

Comment 9

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

Comment on attachment 8601305 [details] [review]
[gaia] timdream:keyboard-dyn-switch > mozilla-b2g:master

Please review this again. The previous patch has a side effect of shipping Emoji layout prematurely. I add a flag to exclude that.

Comment 10

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

Comment on attachment 8601305 [details] [review]
[gaia] timdream:keyboard-dyn-switch > mozilla-b2g:master

Thanks for considering to exclude emoji layout.
I think it would be better if the exclude flag could be set outside of the layout def. file, but don't have a strong opinion on that.

Comment 11

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/b9f6aa017033a4d9c71ebd09788fc127fb11945e