Last Comment Bug 676195 - IME doesn't work with hardware keyboard on Android
: IME doesn't work with hardware keyboard on Android
Status: RESOLVED FIXED
: inputmethod
Product: Core
Classification: Components
Component: Widget: Android (show other bugs)
: Trunk
: All Android
: -- normal (vote)
: mozilla9
Assigned To: Makoto Kato [:m_kato]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-03 00:53 PDT by Makoto Kato [:m_kato]
Modified: 2011-09-24 18:27 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
WIP (16.25 KB, patch)
2011-08-03 21:13 PDT, Makoto Kato [:m_kato]
no flags Details | Diff | Splinter Review
fix v1 (2.31 KB, patch)
2011-08-25 05:27 PDT, Makoto Kato [:m_kato]
no flags Details | Diff | Splinter Review
fix v2 (3.41 KB, patch)
2011-09-01 04:18 PDT, Makoto Kato [:m_kato]
nchen: review+
Details | Diff | Splinter Review

Description Makoto Kato [:m_kato] 2011-08-03 00:53:18 PDT
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
Comment 1 Makoto Kato [:m_kato] 2011-08-03 21:13:39 PDT
Created attachment 550599 [details] [diff] [review]
WIP
Comment 2 Makoto Kato [:m_kato] 2011-08-25 05:27:32 PDT
Created attachment 555709 [details] [diff] [review]
fix v1
Comment 3 Jim Chen [:jchen] [:darchons] 2011-08-30 09:28:12 PDT
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!
Comment 4 Makoto Kato [:m_kato] 2011-09-01 04:18:52 PDT
Created attachment 557457 [details] [diff] [review]
fix v2
Comment 5 Jim Chen [:jchen] [:darchons] 2011-09-01 12:02:58 PDT
Comment on attachment 557457 [details] [diff] [review]
fix v2

Thank you!
Comment 7 Ed Morley [:emorley] 2011-09-04 16:49:53 PDT
http://hg.mozilla.org/mozilla-central/rev/b3fa7b442186
Comment 8 Makoto Kato [:m_kato] 2011-09-07 21:54:19 PDT
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.
Comment 9 Aaron Train [:aaronmt] 2011-09-08 08:18:16 PDT
This bug might have caused a regression in bug 685537 .
Comment 10 JP Rosevear [:jpr] 2011-09-08 14:37:23 PDT
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 11 Asa Dotzler [:asa] 2011-09-22 14:19:23 PDT
Comment on attachment 557457 [details] [diff] [review]
fix v2

drivers agree we'd like this (and the regression fix) both landed in Aurora.
Comment 12 Makoto Kato [:m_kato] 2011-09-24 18:27:35 PDT
Comment on attachment 557457 [details] [diff] [review]
fix v2

cancel approval-aurora since related bug isn't in time until aurora.

Note You need to log in before you can comment on or make changes to this bug.