864666 - Two characters entered when typing into password field with 'Phone' type keyboard

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Keyboards and IME, defect)

Description

[Zac C (:zac)]

Fx: Nightly 23.0a1 (2013-04-21)
Hardware: HTC Desire HD
Android: 2.3.5

STR:
1. Change keyboard to 'Phone' type layout (2 key = 'abc')
2. Go to Zimbra mail login
3. Focus in password field
4. Double tap a key to get the 2nd letter, eg 2 for 'b'
5. Two characters will be committed to the password field

Expect: Only one character committed to the field.

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Zac, can you install this Nightly and get the logcat output when you enter the character on the keyboard?

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-f795ae30dfd3/try-android/fennec-23.0a1.en-US.android-arm.apk

See https://wiki.mozilla.org/Android#Using_logcat on how to get logcat. Let me know if you have questions!

Comment 2

[Zac C (:zac)]

Attachment: Logcat tapping '2' twice to enter one character 'b'

Comment 3

[Zac C (:zac)]

Thanks Jim- I've installed and captured a logcat with the build you attached.

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Replace composing span when committing char as key (v1)

WIP patch

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Zac C (:zac) from comment #3)
> Thanks Jim- I've installed and captured a logcat with the build you attached.

Thanks Zac, that was very helpful! Can you try this build again?

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-8044f9628414/try-android/fennec-23.0a1.en-US.android-arm.apk

Comment 6

[Zac C (:zac)]

Attachment: Logcat against comment#4 apk

Jim, with that build it enters the correct character, eg if I double-tap '2' I get 'b'. however when I go to type the next character it will overwrite 'b'. There's never more than one char in the field.

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

is this a regression (i.e. does this happen on 22, aurora)?

Comment 8

[Zac C (:zac)]

I only use Nightly on my Android (updating regularly) and this only regressed this week.

Although without downloading and tested I am comfortable saying this was not present on 22 Nightly.

I can't speak for Aurora.

Comment 9

[(inactive) Jim Chen [:jchen] [:darchons]]

(In reply to Brad Lassey [:blassey] from comment #7)
> is this a regression (i.e. does this happen on 22, aurora)?

Yes, regression from bug 859452 (so 23 only)

Comment 10

[(inactive) Jim Chen [:jchen] [:darchons]]

BTW Zac, can you try this new build?

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-a03da67c3ca7/try-android/fennec-23.0a1.en-US.android-arm.apk

Thanks!

Comment 11

[Zac C (:zac)]

:jchen, that version works for this test case. (caveat: didn't test any other keyboard types or input field types).

Comment 12

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Replace composing span when committing char as key (v1)

This patch gets rid of any existing composition, when we commit characters as keys when using HTC Touch Input. Otherwise, the new character will be appended to the old composition.

Comment 13

[Chris Peterson [:cpeterson]]

Comment on attachment 742492 [details] [diff] [review]
Replace composing span when committing char as key (v1)

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

LGTM. Even though this new code is only used once, it might make the code intention clearer if you extract it into a new utility method called something like "replaceComposingSpanWithSelection()".

::: mobile/android/base/GeckoInputConnection.java
@@ +658,5 @@
> +                return false;
> +            }
> +            int a = getComposingSpanStart(content),
> +                b = getComposingSpanEnd(content);
> +            if (a != -1 && b != -1) {

Please a short comment explaining why we need to replace the composing span with a selection.

Comment 14

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Replace composing span when committing char as key (v1.1)

Separated into function and added comment

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/795ce24dc148

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/795ce24dc148

Comment 17

[u421692]

Verified fixed on:
Build: Firefox for Android 23.0b6
Device: HTC Desire HD
OS: Android 2.3.5