939791 - [settings] keyboard: {{type}} in mustHaveOneKeyboard is empty for text keyboards

Status: VERIFIED FIXED

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

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Boot2Gecko 1.2.0.0-prerelease 20131117224236 on Keon

When going to Settings > Keyboards > Selected Keyboards > Add more keyboards > And disabling all text keyboards, {{type}} in
At least one {{type}} keyboard should be selected
is an empty string which is troublesome for localization because some languages like German use concatenation and show "[...] -Tastatur [...]"

keyboardType-text should be used here.

Comment 1

[Francesco Lodolo [:flod]]

This should be the code
https://github.com/mozilla-b2g/gaia/blob/master/apps/settings/js/keyboard.js#L457

It uses keyboardType-' + layout.inputManifest.types.sort()[0] to generate the name of the key that has to be used to localize {{type}}. So the problem should be with the keyboard not having a type "text" associated, but the code is kind of hard to figure out.

Comment 2

[Francesco Lodolo [:flod]]

Actually, the problem is that keyboard layouts are defined like this (this is from en.js)

> types: ['text', 'url', 'email']

That line of JavaScript sorts the types, so instead of using 'text' it uses 'email', generating a warning for missing key.

Removing .sort() and adding the missing 'email' type should solve both issues.

Comment 3

[Francesco Lodolo [:flod]]

Attachment: Pull request on Github

Comment 4

[Evelyn Hung]

Comment on attachment 8392898 [details] [review]
Pull request on Github

Arthur is the author of this part, he knows more on the sorting here. Redirect the review to him. I also left a comment on Github.

Comment 5

[Francesco Lodolo [:flod]]

One other thought: is it possible for a keyboard layout to be defined without "types" and trigger this dialog with an empty {{type}}?

Comment 6

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8392898 [details] [review]
Pull request on Github

Thanks for the patch! The original seems buggy. I've proposed a proper fix in github, let me know if that makes sense.

Comment 7

[Francesco Lodolo [:flod]]

I commented on GitHub too: you're right about "text" being used also when I disable the last "url" keyboard, but I'm not sure I follow the suggestion (code is definitely beyond my current understanding of JavaScript, I get lost in callbacks).

From what I understand, when that code is reached, "layout" is already "en.js" and the default keyboard has been already re-enabled, so:
* I have no way to tell the type of (disabled) keyboard that triggered this dialog.
* A call to keyboardHelper.checkDefaults wouldn't give me any results.

Comment 8

[Francesco Lodolo [:flod]]

Comment on attachment 8392898 [details] [review]
Pull request on Github

Resetting review. Patch works on my system and tests pass.

Comment 9

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8392898 [details] [review]
Pull request on Github

Looks good to me. Thank you for the effort!

Comment 10

[Francesco Lodolo [:flod]]

Thanks, setting keyword for checkin.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/gaia/commit/5bf8e888d7e2f7581b75662205356f1c47276ab6

Comment 12

[Francesco Lodolo [:flod]]

Verified on Keon with build 2014-03-25