Closed Bug 676195 Opened 9 years ago Closed 9 years ago

IME doesn't work with hardware keyboard on Android

Categories

(Core :: Widget: Android, defect)

All
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla9

People

(Reporter: m_kato, Assigned: m_kato)

Details

(Keywords: inputmethod)

Attachments

(1 file, 2 obsolete files)

IME works well on software keyboard, but it doesn't on hardware keyboard.  It should work.

- Env
Milestone + OpenWnn
Softbank Mobile AQUOS PHONE THE HYBRID 007SH
Attached patch WIP (obsolete) — Splinter Review
Attached patch fix v1 (obsolete) — Splinter Review
Attachment #550599 - Attachment is obsolete: true
Attachment #555709 - Flags: review?(nchen)
Comment on attachment 555709 [details] [diff] [review]
fix v1

>Bug 676195 - IME doesn't work with hardware keyboard on Android
>
>@@ -560,6 +560,10 @@
> 
>     @Override
>     public boolean onKeyDown(int keyCode, KeyEvent event) {
>+        return processKeyDown(keyCode, event, false);
>+    }
>+
>+    public boolean processKeyDown(int keyCode, KeyEvent event, boolean isPreIme) {

Can this be private?

>+
>+        if (isPreIme && mIMEState != IME_STATE_DISABLED) {
>+            // To support IME on hardware keyboard, onKeyPreIme has to return false.

How about "Let active IME process pre-IME key events"?

And the style is to not use braces on single-line statements

>@@ -608,6 +618,10 @@
> 
>     @Override
>     public boolean onKeyUp(int keyCode, KeyEvent event) {
>+        return processKeyUp(keyCode, event, false);
>+    }
>+
>+    public boolean processKeyUp(int keyCode, KeyEvent event, boolean isPreIme) {

Private?

>+
>+        if (isPreIme && mIMEState != IME_STATE_DISABLED) {
>+            // To support IME on hardware keyboard, onKeyPreIme has to return false.

See above about comment and style.

Thanks!
Attached patch fix v2Splinter Review
Attachment #555709 - Attachment is obsolete: true
Attachment #555709 - Flags: review?(nchen)
Attachment #557457 - Flags: review?(nchen)
Comment on attachment 557457 [details] [diff] [review]
fix v2

Thank you!
Attachment #557457 - Flags: review?(nchen) → review+
http://hg.mozilla.org/mozilla-central/rev/b3fa7b442186
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla9
Comment on attachment 557457 [details] [diff] [review]
fix v2

requsting aurora.

In Japan mobile market, some devices with hardware keyboard are being released now.  But due to this bug, fennec cannot input any non-ascii string on these devices.
Attachment #557457 - Flags: approval-mozilla-aurora?
This bug might have caused a regression in bug 685537 .
Need to know for sure if its causing the regression before we can approve this (or if we can live with the regression).  Mark is looking at this.
Comment on attachment 557457 [details] [diff] [review]
fix v2

drivers agree we'd like this (and the regression fix) both landed in Aurora.
Attachment #557457 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 557457 [details] [diff] [review]
fix v2

cancel approval-aurora since related bug isn't in time until aurora.
Attachment #557457 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.