Closed Bug 1020068 Opened 7 years ago Closed 7 years ago

Zhuyin (注音) input method should be available by default on Flame

Categories

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

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
2.0 S4 (20june)

People

(Reporter: rudyl, Assigned: rudyl)

Details

Attachments

(1 file, 2 obsolete files)

Right now, the keyboard layouts would be opted in during build time, and we did not include Zhuyin by default, to save some ROM space for low-end devices.

However, for Flame, I guess we should enable this for dogfooder in Trad. Chinese.
Hi Ben,

This is just like Bug 1016435, but this one is for Zhuyin input method, one of the Chinese input methods we support now.

Do you know what we can do to enable this for Flame?
Thanks.
Flags: needinfo?(bhearsum)
BTW, I was wondering if we can do something like in the build script of flame? 
GAIA_KEYBOARD_LAYOUTS+=zh-Hant-Zhuyin
so that we won't need to modify this variable every time when we change the same variable in Gaia.
(In reply to Rudy Lu [:rudyl] from comment #2)
> BTW, I was wondering if we can do something like in the build script of
> flame? 
> GAIA_KEYBOARD_LAYOUTS+=zh-Hant-Zhuyin
> so that we won't need to modify this variable every time when we change the
> same variable in Gaia.

I don't think we can append a Makefile variable like that. We should be able to set GAIA_KEYBOARD_LAYOUTS in https://mxr.mozilla.org/mozilla-central/source/b2g/config/flame/config.json#27, though.

I'm looking for some build folks to confirm whether or not we can append.
Flags: needinfo?(bhearsum)
Okay, so there's a couple of things we could do. The best option is to use a different variable to do keyboard additions. Eg:
null  :=
space := $(null) #
comma := ,
GAIA_KEYBOARD_LAYOUTS ?= en pt-BR es de fr pl zh-Hans-Pinyin en-Dvorak
GAIA_KEYBOARD_LAYOUTS += $(GAIA_KEYBOARD_LAYOUTS_EXTRA)
GAIA_KEYBOARD_LAYOUTS := $(subst $(space),$(comma),$(strip $(GAIA_KEYBOARD_LAYOUTS)))

This option allows you to fully override the keyboards, or append. (The build system folks also said that using spaces and converting to commas at the end makes things easier.) If we do this, we'll need to set GAIA_KEYBOARD_LAYOUT_EXTRA in https://mxr.mozilla.org/mozilla-central/source/b2g/config/flame/config.json#27.

The simpler way is to simply do:
GAIA_KEYBOARD_LAYOUTS+=en,pt-BR,es,de,fr,pl,zh-Hans-Pinyin,en-Dvorak

This would let you prepend additional layouts by setting GAIA_KEYBOARD_LAYOUTS in the environment. This might work if the ordering of the layouts doesn't matter, and we don't care about overriding the layouts completely. If we do this, we'll need to set GAIA_KEYBOARD_LAYOUTS in https://mxr.mozilla.org/mozilla-central/source/b2g/config/flame/config.json#27.
Flags: needinfo?(rlu)
Hi Ben,

Thanks for your suggestion. Very helpful.
I think we can go the #1 way you suggested, will try to provide a patch to Gaia build system.
Assignee: nobody → rlu
Flags: needinfo?(rlu)
Attached file Patch V1 - pull request 20196 (obsolete) —
Patch to add GAIA_KEYBOARD_LAYOUTS_EXTRA for adding some layouts per different build variant.

Yuren, could you help take a look at this patch?
Thank you.
Attachment #8436617 - Flags: review?(yurenju.mozilla)
Comment on attachment 8436617 [details] [review]
Patch V1 - pull request 20196

Just discussed with Rudy and for keyboard layouts we should just add GAIA_KEYBOARD_LAYOUTS with Zhuyin into "env" property of config.json for flame to enable this input method.

Ben, what do you think? does it make sense for flame build?

BTW and we should implement automatic download dictionary then we can plreload all layouts (about 30kb).
Attachment #8436617 - Flags: review?(yurenju.mozilla)
Flags: needinfo?(bhearsum)
Yuren, thanks for the feedback.

Ben,

Since Yuren suggest we keep this layout definition into one single variable to make it simple.
Please help define the layout list as follows,
GAIA_KEYBOARD_LAYOUTS=en,pt-BR,es,de,fr,pl,zh-Hans-Pinyin,zh-Hant-Zhuyin,en-Dvorak

Note: please keep the ordering as is.

Thank you.
Attached patch zhuyin_in_flame.patch (obsolete) — Splinter Review
Here is the patch for flame config per the previous discussion.

Ben, could you please review this or I have to find another reviewer?
Thanks.
Attachment #8436805 - Flags: review?(bhearsum)
Attachment #8436617 - Attachment is obsolete: true
Component: Gaia::Keyboard → Gaia::Build
Comment on attachment 8436805 [details] [diff] [review]
zhuyin_in_flame.patch

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

If there's somewhere in https://github.com/mozilla-b2g/device-flame/ that Gaia knows about, it seems like it would be a better place. But this is the next best place.
Attachment #8436805 - Flags: review?(bhearsum) → review+
Flags: needinfo?(bhearsum)
Ben,

Thanks for the review.
Status: NEW → ASSIGNED
Keywords: checkin-needed
Add the bug number in the commit message.
Carry forward the r+.
Attachment #8436805 - Attachment is obsolete: true
Attachment #8437503 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/c16fd4dda33a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S4 (20june)
You need to log in before you can comment on or make changes to this bug.