934198 - Add Vietnamese keyboard to Firefox OS

Status: RESOLVED FIXED

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

Description

[Trung Ngo]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Firefox/24.0 (Beta/Release)
Build ID: 20130917102914

Steps to reproduce:

Provide a default Vietnamese keyboard.

Comment 1

[Trung Ngo]

Attachment: vi-im.diff

Comment 2

[arky [:arky]]

Comment on attachment 826387 [details] [diff] [review]
vi-im.diff

Review of attachment 826387 [details] [diff] [review]:
-----------------------------------------------------------------

Bogo to you too!

Comment 3

[arky [:arky]]

Attachment: vi-patch.txt

Adding a patch for Vietnamese keyboard. Testing needed.

Comment 4

[arky [:arky]]

Comment on attachment 826388 [details] [diff] [review]
vi-patch.txt

Arky -r
The keyboard needs more work.

Comment 5

[arky [:arky]]

Attachment: Vi-FxOS-keyboard.patch

Adding Vietnamese FirefoxOS Keyboard for review

Comment 6

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

Comment on attachment 826392 [details] [diff] [review]
Vi-FxOS-keyboard.patch

So this patch does not need the "Bogo" input method defined in the previous patch?

--
I'll clear the review flag so that I'll get notified when you set it back to me.
Thanks.

Comment 7

[arky [:arky]]

(In reply to Rudy Lu [:rudyl] from comment #6)
> Comment on attachment 826392 [details] [diff] [review]
> Vi-FxOS-keyboard.patch
> 
> So this patch does not need the "Bogo" input method defined in the previous
> patch?
> 
This is clean patch that obsolete the previous patch(https://bugzilla.mozilla.org/attachment.cgi?id=826387). 

Thx.

Comment 8

[arky [:arky]]

Comment on attachment 826392 [details] [diff] [review]
Vi-FxOS-keyboard.patch

Requesting review from Rudy

Comment 9

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

Comment on attachment 826392 [details] [diff] [review]
Vi-FxOS-keyboard.patch

Review of attachment 826392 [details] [diff] [review]:
-----------------------------------------------------------------

If we really want to show so many alternative characters, we might need to ask for UX design to handle this case, like showing them as 2 lines or something.

Can you confirm they are all necessary or we could cut the number down?
Thanks.

::: keyboard/layouts/vi.js
@@ -7,5 @@
> -      a: '@àáảãạăằắẳẵặâầấẩẫậ',
> -      e: 'èéẻẽẹềếểễệê',
> -      i: 'ìíỉĩị',
> -      o: 'òóỏõọồốổỗộôờớởỡợơ',
> -      u: 'ùúủũụừứửữựư',

It seems the alternative chars are too many for "a", "e", "o", and "u".
Are they all necessary?

For current design, we would show these chars as one line, so it might not be able to show them all.

Comment 10

[arky [:arky]]

We looked at Android built-in keyboard and other Vietnamese keyboards to design this patch. I believe these tone mark(ed) characters are needed for writing Vietnamese. 

http://en.wikipedia.org/wiki/Vietnamese_alphabet#Tone_marks

(Am not a native Vietnamese speaker)

Comment 11

[Trung Ngo]

@rudyl: Yes, they are necessary. And I confirm that the current design cannot show that many chars on one line. I've opened bug #934209 on that.

Comment 12

[Trung Ngo]

Attachment: IME for Vietnamese

Comment 13

[Trung Ngo]

I've abandoned the keyboard layout approach and implement a full IME for Vietnamese (the standard way to input Vietnamese by a typical user on other platforms, btw). Please review it.

Comment 14

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

Comment on attachment 8373891 [details] [review]
IME for Vietnamese

Thanks for the patch.
This is in my review queue, but I want to clarify that are you the original author of bogo.js (or jsbogo.js), if yes, please help make it pass the jshint, since that's one of our policy for this code to land.

Would be even better if you could help adding unit testing for this specific module.

Thanks.

Comment 15

[Trung Ngo]

Hi Rudy,

Yes, I am the author of both bogo.js (the core engine) and jsbogo.js (the wrapper). Sorry for the mildly confusing division, I intend to reuse the core engine for other projects. Speaking of this, should I break bogo.js into a separate file and use WebWorker to communicate with it like the Chinese IMEs or should I keep it copy-pasted directly in jsbogo.js like right now?

The code cannot pass jshint right now because of a design decision in Gaia's keyboard that requires each input method to refer to the global InputMethods variable directly to register themselves and that triggers an "'InputMethods' is not defined." error in jshint. I see that other IME also hit this error and have to fallback to gjslint. Other than that, the code is clean.

I'm looking into unit testing right now and hope to get it tested in a few days.

Thanks.

Comment 16

[Jan Jongboom [:janjongboom] (inactive)]

Hi, you can make it pass jshint by declaring the global variables on top of your file:

/*global InputMethods */

Comment 17

[Trung Ngo]

Thanks, Jan. That did the trick!

Btw, is there any way I can import other scripts from my main script other than using web workers?

Comment 18

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

Trung,

Are you looking for something like this?
https://github.com/mozilla-b2g/gaia/blob/master/shared/js/lazy_loader.js

You could find its usage all around the gaia code.

If you could make jsbogo.js pass the jshint check, please help update the patch.
Thank you.

Comment 19

[arky [:arky]]

Comment on attachment 8373891 [details] [review]
IME for Vietnamese

Thanks for the patch. Perhaps it would be a good idea to gather some user experience feedback and community testing before we land this feature. 

Adding some background information for context: This feature is more specialized than the built-in Vietnamese keyboards shipped on Android. We need to access whether this is good candidate for distribution as third party keyboard for now.

Comment 20

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

Hi Trung, Arky,

Per comment 19, I think Arky is suggesting that we keep this bug to track the work of "having a basic 1:1 Vietnamese keyboard" (as Android does).

If possible, please open another bug for the new version of Vietnamese keyboard, which is enhanced by jsbogo.js.
Agree?

Comment 21

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

Comment on attachment 8373891 [details] [review]
IME for Vietnamese

Clear the review flag first before the jshint issue is addressed.
Please flag me again for review so that I'll get notified.
Thanks.

Comment 22

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

Trung,

BTW, I created another bug to track the IME-based work you're proposing here.
Please help attach your patch there.

Thanks.

Comment 23

[arky [:arky]]

@Trung, Any progress on the keyboard and dictionary? 

@Kevin Do you have any frequency list based on Vietnamese texts?

Comment 24

[Kevin Scannell]

Hi Arky, Trung and I have been emailing each other about this. He's currently looking at a draft frequency list that I created from web texts.  

The issue is going to be the use of spaces "word"-internally.  I suspect major changes will have to be made to the predictive text code to support this correctly, a la Bug 936876.

Comment 25

[arky [:arky]]

(In reply to Kevin Scannell from comment #24)
> Hi Arky, Trung and I have been emailing each other about this. He's
> currently looking at a draft frequency list that I created from web texts.  

Thanks please keep me posted. Am looping in David to talk about predictive text code.
https://bugzilla.mozilla.org/show_bug.cgi?id=936876

Comment 26

[Ted Clancy [:tedders1]]

I believe this bug is obsoleted by recent work on Bug 974794.