Closed Bug 868001 Opened 12 years ago Closed 12 years ago

First time setup wizard should always enable English keyboard layout for locales with non-Latin writing system

Categories

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

Product:
Firefox OS Graveyard
Component:
Gaia::First Time Experience
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-b2g:leo+, b2g18+ fixed)

Status:
RESOLVED FIXED
Project Flags:
blocking-b2g leo+
Tracking Flags:
Tracking Status
b2g18 + fixed

People

(Reporter: unghost, Assigned: mihai)

References

(
URL
)

Details

(Whiteboard: c= [status: waiting on input from Kev and UX])

Attachments

(1 file)

Pull Request #9724 - Enable English KB for non-latins
(deleted), text/html
fcampo
: review+
Details 
Steps to reproduce:
1) Reset phone
2) Select Russian as setup language
3) Press Next till screen with list of Wi-fi networks

Expected results: Connect to protected Wi-fi network

Actual results: It's not possible to connect to protected Wi-fi network, because when I try to enter password, only Cyrillic layouts are available (Russian and Serbian). My password uses Latin letters.

Also, when I skipped Wi-fi step and finished First time setup wizard, I found out that only Cyrillic layouts are enabled in Settings. It's quite impossible to use phone without enabled English layout, cause almost all web-site URLs use latin letters (at least in Russia).
I've gathered from other bugs that Serbian (Cyrillic) is the main language for our target markets, so perhaps it's worth to nominate this bug.
Alexander, would it make sense to enable the ability to flip to English and Russian?  Ie enable both keyboard layouts when you select a language in the FTE?
blocking-b2g: --- → tef?
(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #1)
> Alexander, would it make sense to enable the ability to flip to English and
> Russian?  Ie enable both keyboard layouts when you select a language in the
> FTE?
Yes, that's what I mean. It should be possible to switch between English and Russian in FTE.
Also I feel that English layout should be enabled in Settings after FTE is finished (see last paragraph of comment 0). I don't know if separate bug is needed for that.
IMHO, this is a considerable change that will affect most users that the ones we will be solving.

Adding to the loop to product owners just to check ;)

Thanks!
F.
(In reply to Francisco Jordano [:arcturus] from comment #3)
> IMHO, this is a considerable change that will affect most users that the
> ones we will be solving.
> 
> Adding to the loop to product owners just to check ;)
> 
> Thanks!
> F.

I don't think that this change should affect languages using latin-based scripts, like french or spanish. Writing systems of these languages include basic Latin script, so there are no need to enable additional English keyboard layout for them. 
This bug affects non-Latin writing systems like Cyrillic, Greek, Hebrew, Arabic, etc.
Summary: First time setup wizard should always enable English keyboard layout → First time setup wizard should always enable English keyboard layout for locales with non-Latin writing system
Is this bug affecting any of the target markets for 1.0.1? IIRC it does not, and the use has still the option to configure WiFi later on. If that is the case I suggest we mark is as tef- or leo?
Whiteboard: [tef-triage]
(In reply to Daniel Coloma:dcoloma from comment #5)
> Is this bug affecting any of the target markets for 1.0.1? IIRC it does not,
> and the use has still the option to configure WiFi later on. If that is the
> case I suggest we mark is as tef- or leo?

Actually this bug affects not only Wi-Fi option. You cannot import contacts from Gmail/Facebook/Windows Live in FTE without logging in them. AFAIK, these networks use Latin letters for login and password. On one of next steps FTE suggests to enter e-mail address which most certainly uses letters from Latin script.
AFAIK, Mozilla plans to launch Firefox OS in Serbia and Montenegro, which languages also use Cyrillic alphabet beside Latin.
> AFAIK, Mozilla plans to launch Firefox OS in Serbia and Montenegro, which
> languages also use Cyrillic alphabet beside Latin.

But which version? If is not v1.0.1 it should not be tef? but leo?
(In reply to Daniel Coloma:dcoloma from comment #7)
> > AFAIK, Mozilla plans to launch Firefox OS in Serbia and Montenegro, which
> > languages also use Cyrillic alphabet beside Latin.
> 
> But which version? If is not v1.0.1 it should not be tef? but leo?

According to http://groups.google.com/group/mozilla.dev.l10n/msg/9b816b71c733a663 Serbian is not on v1.0.1 train, so I guess it should be tef-.
Moving to leo triage
blocking-b2g: tef? → leo?
Whiteboard: [tef-triage]
This would definitely fall into new feature work to be able to region/locale from input method(s). Will need product to weigh in on design & market planning for something like this. In the meantime not blocking until there is more info and a plan moving forward.
blocking-b2g: leo? → -
status-b2g18: --- → affected
tracking-b2g18: --- → +
Flags: needinfo?(ffos-product)
Whiteboard: c=
Kev, this could use your input.
Flags: needinfo?(ffos-product) → needinfo?(kev)
Given that this makes the phone nearly unusable for people who chose non-latin languages, I believe that we'll need to block on this.  I'm not sure if the best approach is to automatically use the latin keyboard or have a simple way for the user to switch it when needed.

UX team, this could use your input.
Flags: needinfo?(firefoxos-ux-bugzilla)
blocking-b2g: - → leo?
Whiteboard: c= → c= [status: waiting on input from Kev and UX]
Assignee: nobody → mihai
Triage - leo+ unless we confirm that non-latin language options are not available on shipped v1.1 devices for users to configure on first boot.

Product/Kev please denom this if this is the case.
blocking-b2g: leo? → leo+
This is a quick fix in the FTU app and does not treat cases outside of it, like switching the language from the Settings app (Note: doing so will not disable previously enabled keyboards -- e.g. selecting EN in FTU and then switching to RU from Settings will keep the EN keyboard enabled).

Overview of the fix: if the selected language in the FTU has a non-latin input method, the English keyboard is also enabled to ensure that authentication can be done successfully on latin-based input fields.
Attachment #748841 - Flags: review?(fernando.campo)
Target locales for 1.1 include languages that use non-latin language options. Should stay leo+.
Flags: needinfo?(kev)
(In reply to Mihai Cirlanaru [:mcirlanaru] from comment #15)
> Created attachment 748841 [details]
> Pull Request #9724 - Enable English KB for non-latins
> 
> This is a quick fix in the FTU app and does not treat cases outside of it,
> like switching the language from the Settings app (Note: doing so will not
> disable previously enabled keyboards -- e.g. selecting EN in FTU and then
> switching to RU from Settings will keep the EN keyboard enabled).
> 
> Overview of the fix: if the selected language in the FTU has a non-latin
> input method, the English keyboard is also enabled to ensure that
> authentication can be done successfully on latin-based input fields.

Are you sure that "shared/resources/keyboard_layouts.json" contains correct information? At least "ko" (Korean language) is wrongly marked as "otherlatins", though Korean doesn't use Latin alphabet. I don't see Serbian in this file at all. Bunch of Indian languages like "hi-IN" or "te" also marked as "otherlatins".
(In reply to Alexander L. Slovesnik from comment #17)
> (In reply to Mihai Cirlanaru [:mcirlanaru] from comment #15)
> > Created attachment 748841 [details]
> > Pull Request #9724 - Enable English KB for non-latins
> > 
> > This is a quick fix in the FTU app and does not treat cases outside of it,
> > like switching the language from the Settings app (Note: doing so will not
> > disable previously enabled keyboards -- e.g. selecting EN in FTU and then
> > switching to RU from Settings will keep the EN keyboard enabled).
> > 
> > Overview of the fix: if the selected language in the FTU has a non-latin
> > input method, the English keyboard is also enabled to ensure that
> > authentication can be done successfully on latin-based input fields.
> 
> Are you sure that "shared/resources/keyboard_layouts.json" contains correct
> information? At least "ko" (Korean language) is wrongly marked as
> "otherlatins", though Korean doesn't use Latin alphabet. I don't see Serbian
> in this file at all. Bunch of Indian languages like "hi-IN" or "te" also
> marked as "otherlatins".

Seems like there are quite a few inconsistencies in that shared file, which is important in setting the correct keyboard layouts (i.e. it acts as a map which is always consulted for setting a language-specific keyboard in the FTU and, with the patch for bug 828237, in the Settings app).

From my knowledge there is/will be an effort to refactor the keyboard code for v1.2 so this might be more relevant for then, when more keyboard layouts will probably be introduced. However, Serbian seems to have keyboard layouts (sr-Latin and sr-Cyrl) which are not linked in any way within keyboard_layouts.json -- this should definitely be fixed (I will file a bug for it).

For the other languages you were mentioning, there is no keyboard in apps/keyboard/js/layout.js so marking them as 'otherlatins' was probably a design decision to enable latin-based keyboard layouts for those languages. However, I don't see why the layout was not set by default to 'english' if there is no keyboard for it -- maybe the UX team can give some input on this too (?)
Hi there, maybe I can throw some light into this, as I created the file time ago.

(In reply to Mihai Cirlanaru [:mcirlanaru] from comment #18)
> (In reply to Alexander L. Slovesnik from comment #17)
> > Are you sure that "shared/resources/keyboard_layouts.json" contains correct
> > information? At least "ko" (Korean language) is wrongly marked as
> > "otherlatins", though Korean doesn't use Latin alphabet. I don't see Serbian
> > in this file at all. Bunch of Indian languages like "hi-IN" or "te" also
> > marked as "otherlatins".

The file was created based on the keyboard layouts declared on apps/keyboard/js/layout.js at that time, I ignore if we have new ones, but if we do, probably the file is out of sync. Surely at that time we didn't have any korean or indian layouts declared, but if there was any changes on keyboard after that, I can't speak of them.

> For the other languages you were mentioning, there is no keyboard in
> apps/keyboard/js/layout.js so marking them as 'otherlatins' was probably a
> design decision to enable latin-based keyboard layouts for those languages.
> However, I don't see why the layout was not set by default to 'english' if
> there is no keyboard for it -- maybe the UX team can give some input on this
> too (?)
My fault mostly. When setting up at first, I decided (without consulting UX or l10n teams) to use other latins instead of english for the default one, as that option would enable more keyboards with their respective special symbols, to give more options to the user. But of course, that might change without a problem :)

PS - @Mihai, now that the leo+ is sure, I'm taking care of the review

Thanks
Comment on attachment 748841 [details]
Pull Request #9724 - Enable English KB for non-latins

patch looks good, and works fine, thanks Mihai.

My only concern is about updating the list when needed. Keeping it as a variable on the code presents the need of constant update when supported languages or keyboard layouts are modified.

Right now I can't think about a proper system, so maybe keeping a comment on keyboard_layouts to remind devs about updating this list when modifying the file would be a good idea. WDYT guys?
Attachment #748841 - Flags: review?(fernando.campo) → review+
Hi,

can we try to avoid, that list on the code?

I mean, I would prefer to have it as a configuration option, something that we NEED to officially maintain.

Having that list in the code, doesn't look to me as the best maintainable idea.

Why don't we modify the content of the keyboard_layouts.json, with a format like:

{
   "layouts": { .... current content of that file .... },
   "nonLatingKeyboards" : ['ar', 'el', 'he', 'ja', 'ru', 'zh-CN', 'zh-TW']
}

The changes in FTU will be minimun, since the original list keeps the same, and we can add any extra config option regarding keyboard layout that we missed.

Cheers!
F.
Hi Fernando, Francisco,

(In reply to Fernando Campo (:fcampo) from comment #20)
> My only concern is about updating the list when needed. Keeping it as a
> variable on the code presents the need of constant update when supported
> languages or keyboard layouts are modified.
>
> Right now I can't think about a proper system, so maybe keeping a comment on
> keyboard_layouts to remind devs about updating this list when modifying the
> file would be a good idea. WDYT guys?

This was also a concerned I had, however, the alternative solution I had in mind: changing the keyboard layout groups logic in keyboard.js (cf. https://github.com/mozilla-b2g/gaia/blob/master/apps/keyboard/js/keyboard.js#L198) to include 'en' for non-latin keyboard groups would impact not only the FTU app, but also the Settings app, and could be a source of regression bugs. In the end, that would also be a change to a list/map in the code.

> Why don't we modify the content of the keyboard_layouts.json, with a format like:
>
> {
>   "layouts": { .... current content of that file .... },
>   "nonLatingKeyboards" : ['ar', 'el', 'he', 'ja', 'ru', 'zh-CN', 'zh-TW']
> }
>
> The changes in FTU will be minimun, since the original list keeps the same, and 
> we can add any extra config option regarding keyboard layout that we missed.

This type of change would impact both FTU and Settings apps, which would need to be adapted to account for the "layouts" and "nonLatinKeyboards" extensions. The quick fix I employed with my patch has a far smaller footprint than such a structural change to keyboard_layouts.json.

Rudy, which of the proposals makes more sense:
1. modify the keyboard layout groups logic in keyboard.js
2. change the structure of keyboard_layouts.json
3. the current fix (cf. comment 15) in the FTU app
Flags: needinfo?(rlu)
(In reply to Mihai Cirlanaru [:mcirlanaru] from comment #22)
> 
> This type of change would impact both FTU and Settings apps, which would
> need to be adapted to account for the "layouts" and "nonLatinKeyboards"
> extensions. The quick fix I employed with my patch has a far smaller
> footprint than such a structural change to keyboard_layouts.json.

I agree, but the change in both is minimun and non risky, IMHO, giving us more flexibility.

> 
> Rudy, which of the proposals makes more sense:
> 1. modify the keyboard layout groups logic in keyboard.js

I missed that 1st option, I like it as well :)

> 2. change the structure of keyboard_layouts.json
> 3. the current fix (cf. comment 15) in the FTU app

Thanks a lot Mihai!!
Flags: needinfo?(firefoxos-ux-bugzilla)
Reassigning to Francis for keyboard behavior guidance requested by Peter in comment 12: "I'm not sure if the best approach is to automatically use the latin keyboard or have a simple way for the user to switch it when needed."

To the blocking point, agree that this should be a blocker per Peter's observation that it essentially prevents the user from accomplishing his/her task.
Flags: needinfo?(fdjabri)
tprint than such a structural change to keyboard_layouts.json.
> 
> Rudy, which of the proposals makes more sense:
> 1. modify the keyboard layout groups logic in keyboard.js
> 2. change the structure of keyboard_layouts.json
> 3. the current fix (cf. comment 15) in the FTU app


Hi all,

As I said in our previous discussion on IRC, the keyboard layout dependency logic should NOT be done in keyboard app. This is a OS/system-specific preferences that should be tunable outside of the keyboard app itself.

I would prefer we do #2, to describe the dependency in a shared json file, and let both FTU and settings reference it.

Thanks for looking into this.
Flags: needinfo?(rlu)
Reassigning to Rob since this is leo+.
Flags: needinfo?(fdjabri)
Comment on attachment 748841 [details]
Pull Request #9724 - Enable English KB for non-latins

Thanks for the suggestion, Rudy, I have updated the patch to follow the approach Francisco was highlighting in comment 21.

Let me know if these changes are in the same lines with the desired solution.
Attachment #748841 - Flags: review?(rlu)
Blocks: 872825
Comment on attachment 748841 [details]
Pull Request #9724 - Enable English KB for non-latins

Switching this to Fernando, let me know how it looks. Thanks!
Attachment #748841 - Flags: review?(rlu)
Attachment #748841 - Flags: review?(fernando.campo)
Attachment #748841 - Flags: review+
Comment on attachment 748841 [details]
Pull Request #9724 - Enable English KB for non-latins

Tested and both FTU and Settings seems to work good

Thx for the fix!
Attachment #748841 - Flags: review?(fernando.campo) → review+
Thanks for the review, Fernando! Landed on master:
https://github.com/mozilla-b2g/gaia/commit/f14df69587b597c6a1d9aa0865fe468d328c5c56
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
I was not able to uplift this bug to v1-train.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1-train, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1-train
  git cherry-pick -x -m1 f14df69587b597c6a1d9aa0865fe468d328c5c56
  <RESOLVE MERGE CONFLICTS>
  git commit
I fixed the merge conflicts in another pull request for v1-train: https://github.com/mozilla-b2g/gaia/pull/10000

Fernando, can you have a look and merge it to v1-train if everything works well?
Flags: needinfo?(fernando.campo)
Yeah, I'm taking a look and uplifting it. 
Sorry for the delay, long weekend with bank holiday in here.
Flags: needinfo?(fernando.campo)
Merged on v1-train https://github.com/mozilla-b2g/gaia/commit/5f499ab1e4a274d3d28e20139c9cdc546dc4fc69

Thanks Mihai!
status-b2g18: affected → fixed
Hm, I've updated Gaia on my Keon to v1-train and now FTE wizard shows only English keyboard layout. Steps to reproduce:
1) Build and update Gaia according to https://l10n.etherpad.mozilla.org/gaia-multilocale
2) Reset your phone
3) Select Russian in FTE wizard
4) Skip to "Enter password for your Wi-Fi" step and try to enter password.

Expected results:
English and 2 Cyrillic keyboards are available.

Actual results:
Only English keyboard is available.
Hi Alexander, I could not reproduce this on v1-train (English and 2 cyrillic keyboards are available on the "Enter password for WiFi" screen) by flashing the phone with the following command (taken from https://l10n.etherpad.mozilla.org/gaia-multilocale after pulling the appropriate locales for LOCALE_BASEDIR):

> make clean && make production MAKECMDGOALS=production MOZILLA_OFFICIAL=1 GAIA_DEFAULT_LOCALE=pl LOCALES_FILE=locales/languages_all.json LOCALE_BASEDIR=locales/ REMOTE_DEBUGGER=1

From what I see in https://l10n.etherpad.mozilla.org/gaia-multilocale, the gaia branch that is used to flash the phone is v1.0.1 (line 19 in the MoPad) and this patch landed only on master and v1-train -- if this patch is relevant for v1.0.1, you should request for blocking-b2g:tef? from what I remember.
Flags: needinfo?(unghost)
(In reply to Mihai Cirlanaru [:mihai][:mcirlanaru] from comment #36)
> Hi Alexander, I could not reproduce this on v1-train (English and 2 cyrillic
> keyboards are available on the "Enter password for WiFi" screen) by flashing
> the phone with the following command (taken from
> https://l10n.etherpad.mozilla.org/gaia-multilocale after pulling the
> appropriate locales for LOCALE_BASEDIR):
> 
> > make clean && make production MAKECMDGOALS=production MOZILLA_OFFICIAL=1 GAIA_DEFAULT_LOCALE=pl LOCALES_FILE=locales/languages_all.json LOCALE_BASEDIR=locales/ REMOTE_DEBUGGER=1
> 
> From what I see in https://l10n.etherpad.mozilla.org/gaia-multilocale, the
> gaia branch that is used to flash the phone is v1.0.1 (line 19 in the MoPad)
> and this patch landed only on master and v1-train -- if this patch is
> relevant for v1.0.1, you should request for blocking-b2g:tef? from what I
> remember.

I've tweaked build instructions https://l10n.etherpad.mozilla.org/gaia-multilocale for Russian locale from v1-train. I've pulled gaia from v1-train, used GAIA_DEFAULT_LOCALE=ru and pulled Russian locale from https://hg.mozilla.org/gaia-l10n/ru/ I've not updated B2G on my Keon, it still on 1.0.1.0-prerelease. Perhaps that's why I see this issue.
This patch is not relevant for 1.0.1, only for v1-train.

I've done some more testing and I think that understood this issue:
1) Build and flash Russian Gaia from v1-train on Keon with instructions from https://l10n.etherpad.mozilla.org/gaia-multilocale as reference
2) Reset Keon. Take note, that Russian language is selected by default. You don't need to manually select it. Press "Далее" ("Next" in Russian) 
3) Skip to "Enter password for your Wi-Fi" step and try to enter password. Note that only English keyboard is available. 
4) Return back to first screen of FTE wizard, select any other language and skip to "Enter password for your Wi-Fi" step.
5) Return back to first screen of FTE wizard, select Russian language again and skip to "Enter password for your Wi-Fi" step. Note that English and 2 Cyrillic keyboards are available.

It looks like with current patch you need to actively select Russian on first FTE screen in order to get English and 2 Cyrillic keyboards. If this language is selected by default on first screen of FTE wizard by means of GAIA_DEFAULT_LOCALE=ru, only English keyboard is shown.
Perhaps it's not an issue for production phones as they use English language as default? I don't know.
Flags: needinfo?(unghost)
I understand what the issue is now and I was able to reproduce it following the build instructions in comment 37. I suggest you file a follow-up bug highlighting that language associated keyboard is not set properly through the FTE when building/flashing with a different GAIA_DEFAULT_LOCALE then 'en' (I could reproduce this with 'fr' as well -> 'otherlatins' keyboards are not activated if no selection in FTE is performed; so the issue does not depend on bug 868001).

You can assign the bug to me and I'll have a look at it on Monday.
(In reply to Mihai Cirlanaru [:mihai][:mcirlanaru] from comment #38)
> You can assign the bug to me and I'll have a look at it on Monday.

Filed Bug 878421
Blocks: 878421
Flags: in-moztrap?
URL: https://moztrap.mozilla.org/manage/ca...
Flags: in-moztrap? → in-moztrap+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: