Closed Bug 844717 Opened 8 years ago Closed 6 years ago

Add Thai input method for Gaia

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-master verified)

VERIFIED FIXED
2.2 S4 (23jan)
Tracking Status
b2g-master --- verified

People

(Reporter: pittaya, Assigned: gpgreen)

References

Details

Attachments

(4 files, 2 obsolete files)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:21.0) Gecko/20130222 Firefox/21.0
Build ID: 20130222111254

Steps to reproduce:

Try to type in Thai (th).


Actual results:

No Thai input method.


Expected results:

Thai should be available to select in Settings > Keyboard and in keyboard layout switcher.
Pull request from Github https://github.com/mozilla-b2g/gaia/pull/8289
Cc'ing people who'd know the right reviewer and who might have input.
Not sure how and when to drive this forward, but we'll need a bugzilla attachment pointing to the PR. Just a short html file redirecting to the actual PR url. Then request review from someone that contributed to keyboard before.
Kevin Scannell might know more about this.

I think this also provide a little bit more information on this:
https://wiki.mozilla.org/L10n:B2G/Adding_Locales
Flags: needinfo?(kscanne)
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: All → Gonk (Firefox OS)
Hardware: All → ARM
Attachment #732959 - Attachment mime type: text/plain → text/html
I can provide frequency information for Thai if needed, just ping me.  Don't know about the rest of the process, however.
Flags: needinfo?(kscanne)
Kevin, sorry for the bothering :P
I don't know who to ping actually :P

And, thanks for the helping!
Any feedback on this patch.
Vee,

Thank you for this patch!  The next step is to get it reviewed.  Rudy Lu and I are both potential reviewers.

There is a general problem with new keyboard layouts.  We want to have lots available for users around the world. But we don't want to keep adding layouts to the default build. (This is especially problematic if the layouts have associated autocorrect dictionaries).

Your patch will be much easier to land once bug 884754 is resolved and there is a way to configure the list of supported keyboards at build time.

Meanwhile, in comment 6, Kevin has volunteered to provide an autocorrect wordlist for Thai.  If he does this, you might be able to get Thai autocorrection basically for free. Perhaps you and Kevin and I should talk about this: does Thai have tricky parts like vowels as diacritcs or combining forms where two letters produce a ligature?  If it works pretty much like a latin alphabet, then maybe we can use the (increasingly misnamed) latin IM for it.  (Note that the title of this bug is now out of date.  The title specifies an "input method" and the patch just adds a keyboard layout.)
As commented on github, https://github.com/mozilla-b2g/gaia/pull/12239#issuecomment-24561488,

I think we can accept this patch into our built-in keyobard app if simply adding the layout could serve most of Thai speakers.
If they would need more than that, e.g. a dictionary for auto correction or something for them to input Thai, then we would suggest we move it into a separate app.

Vee,

Could you confirm what we need here?
Thank you.
Flags: needinfo?(vsatayamas)
You're welcome. Auto correction for Thai is a bit different from languages using latin alphabet because there is no explicit marker (space) between Thai words. In Thai (like Myanmar, Khmer, etc), space is usually used for indicating larger unit than word e.g. phrases, clause. So I don't think, using latin IM without modification is possible.

> Vee,
> 
> Thank you for this patch!  The next step is to get it reviewed.  Rudy Lu and
> I are both potential reviewers.
> 
> There is a general problem with new keyboard layouts.  We want to have lots
> available for users around the world. But we don't want to keep adding
> layouts to the default build. (This is especially problematic if the layouts
> have associated autocorrect dictionaries).
> 
> Your patch will be much easier to land once bug 884754 is resolved and there
> is a way to configure the list of supported keyboards at build time.
> 
> Meanwhile, in comment 6, Kevin has volunteered to provide an autocorrect
> wordlist for Thai.  If he does this, you might be able to get Thai
> autocorrection basically for free. Perhaps you and Kevin and I should talk
> about this: does Thai have tricky parts like vowels as diacritcs or
> combining forms where two letters produce a ligature?  If it works pretty
> much like a latin alphabet, then maybe we can use the (increasingly
> misnamed) latin IM for it.  (Note that the title of this bug is now out of
> date.  The title specifies an "input method" and the patch just adds a
> keyboard layout.)
Flags: needinfo?(vsatayamas)
The layout only without auto correction is already enough. As far as, I have seen Thai people use smartphones, they turn auto correction off even if it is available (for example, on iOS). For more example, one of the most popular Thai keyboard apps on Android, in Keyboard Manman even doesn't provide  auto correction.

(In reply to Rudy Lu [:rudyl] from comment #11)
> As commented on github,
> https://github.com/mozilla-b2g/gaia/pull/12239#issuecomment-24561488,
> 
> I think we can accept this patch into our built-in keyobard app if simply
> adding the layout could serve most of Thai speakers.
> If they would need more than that, e.g. a dictionary for auto correction or
> something for them to input Thai, then we would suggest we move it into a
> separate app.
> 
> Vee,
> 
> Could you confirm what we need here?
> Thank you.
Depends on: 905292
No longer depends on: 905292
Humm, I was curious enough to test out Pure's latest patch. Therefore something seems wrong.

"secondLayout" (which stands for making the upperCase layout an extra space for more characters) is not working when using Rocketbar with imEngine set to 'latin' in the layout file, tried removing it and it worked, it was so obvious that, the issue was caused because this is not a latin language. Same as Myanmar and Arabic.
However secondLayout in Myanmar is working well on Rocketbar which confused me.
Arky, can you please confirm the existence this particular issue? :)
Thanks
Flags: needinfo?(hitmanarky)
(In reply to Ahmed Nefzaoui [:Nefzaoui] from comment #15)
> Arky, can you please confirm the existence this particular issue? :)

No idea, I think the patch is stale anyway.
Flags: needinfo?(hitmanarky)
Attached file pull request for thai keyboard (obsolete) —
I have updated this keyboard from the previous versions submitted on the bug.
Attachment #8543187 - Flags: review?(rlu)
Greg, Could you squash the commits and push it as just one commit ?
Flags: needinfo?(gpgreen)
Attached file pull request for thai keyboard (obsolete) —
Squashed the commits in branch
Attachment #8543187 - Attachment is obsolete: true
Attachment #8543187 - Flags: review?(rlu)
Flags: needinfo?(gpgreen)
Attachment #8543380 - Flags: review?(rlu)
Comment on attachment 8543380 [details] [review]
pull request for thai keyboard

This patch would still need some updates to make it conform to our latest format of the layout definition.
Please help take a look at the comments I made on the pull request and let me know if you have any updates on this.

Thanks and sorry for the delay to look at your patch.
Attachment #8543380 - Flags: review?(rlu)
Assign to Greg since he is working on this.
Assignee: nobody → gpgreen
Status: NEW → ASSIGNED
rudyl,

Thanks for the review, I will incorporate your comments.
rudyl: The keyboard layout now uses pages. Removed the symbol page as the default one is ok. Removed spaces near commas.
Attachment #8543380 - Attachment is obsolete: true
Attachment #8549391 - Flags: review?(rlu)
Comment on attachment 8549391 [details] [review]
pull request for thai keyboard

Hi this looks good with some nits to be addressed.
Thank you very much.
Attachment #8549391 - Flags: review?(rlu) → review+
I noticed that you add "R=gpgreen@gmail.com", you should remove this.
Our convention is use "r=email_of_your_reviewer" in the merge commit, so you don't have to add this info here and it should not be your own name.
(In reply to Rudy Lu [:rudyl] from comment #26)
> I noticed that you add "R=gpgreen@gmail.com", you should remove this.
> Our convention is use "r=email_of_your_reviewer" in the merge commit, so you
> don't have to add this info here and it should not be your own name.

I followed the procedure on this link: https://developer.mozilla.org/en-US/Firefox_OS/Developing_Gaia/Submitting_a_Gaia_patch, step 3. Is there somewhere else that describes what you are asking? I suspect there are other things I need to know...
No, that's correct reference, I think.
but here r means review, you could put r=rudylu here, instead of your own name.

Thanks.
Flags: needinfo?(gpgreen)
Rudy,

I have updated the pull request.
Flags: needinfo?(gpgreen)
Attachment #8549391 - Flags: review+ → review?(rlu)
Comment on attachment 8549391 [details] [review]
pull request for thai keyboard

Updated pull request to squash commits.
Attachment #8549391 - Flags: review?(rlu) → review-
Keywords: checkin-needed
Comment on attachment 8549391 [details] [review]
pull request for thai keyboard

Looks good to me.
Thank you!

Will try to land this after CI is passed.
Attachment #8549391 - Flags: review- → review+
Master: https://github.com/mozilla-b2g/gaia/commit/5ecbc9cdb7cc2a53b594c22b0608bed1cfa805a9
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S4 (23jan)
Thai keyboard is still NOT available on Settings > Keyboards > Select Keyboards list.

See screenshot. The two options not in the screenshot are English and Number.

Tested on latest Central for which I've verified this build contains the Gaia change at comment 32.

Device: Flame 3.0 Master
BuildID: 20150121141532
Gaia: 917b6c36717fddc6e71ffc1ec249633c8044c93c
Gecko: 06b590bf59f4
Version: 38.0a1 (3.0 Master)
Firmware: V18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0
QA Whiteboard: [failed-verification][QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [failed-verification][QAnalyst-Triage?] → [failed-verification][QAnalyst-Triage+]
Flags: needinfo?(ktucker)
The thai keyboard will not be included unless the environment variable GAIA_KEYBOARD_LAYOUTS has 'th' included in the values. The Makefile in top level gaia directory has this environment variable with the selection that shows up as default. Many of the keyboard layouts that are in apps/keyboard/js/layouts/ are not in this list in the Makefile, so I did not add it there. I don't know what the criteria is to have it included there. Perhaps a more complete localization for that language?
(In reply to Greg Green from comment #34)
> Perhaps a more complete localization for that language?

Not really. Most of localizations are complete but don't ship as default keyboard. As far as I know we need bugs like 1029951 fixed before extending that list.
No longer depends on: 1125753
The Thai keyboard is verified fixed on Flame Master.

Result: After pushing Thai keyboard, it was successfully installed and fully functional. 

Device: Flame 3.0
Build ID: 20150130093240
Gaia: 5ce984fe5ac8d600bd65068ee1eee27b99c03e5b
Gecko: d7e156a7a0a6
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [failed-verification][QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Duplicate of this bug: 1138007
You need to log in before you can comment on or make changes to this bug.