667927 - Problem with deletions in IME when in composition mode on Honeycomb

Status: VERIFIED FIXED

(Core Graveyard :: Widget: Android, defect, P2)

Description

[Thomas Arend [:tarend]]

Attachment: Screenshots and Steps to Reproduce Input Issue

Samsung kindly reported this bug to us - see all details in the attached PDF

Steps to reproduce:
(1) Install Korean IME dingulhangul from Android Market. Long press any input field and select the new Input method in the pop-up window
(2) Press “ᄅ” button (Korean consonant). Then press “ᄋ” button (Korean consonant). Press “space key", then press “backspace key” (see screenshots in attached PDF)

Result: When backspace key is tapped, both space and one consonant are removed.
Expected: Only space should be removed with the Backspace

Comment 1

[Thomas Arend [:tarend]]

I forgot: In step (1) after installing the IME, go to Android Settings -> Select Language and keyboard menu in setting -> Check the box for the newly installed input method to activate it. Otherwise you won't be able to select it when long-tapping in an input field.

Also: I tried to reproduce, but could NOT reproduce the issue on my original 7inch Samsung Galaxy Tab running Android 2.2. Also, I could NOT reproduce on a Samsung Galaxy S (GT-I9000).

Comment 2

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I could not reproduce using :

Mozilla/5.0 (Android; Linux armv71; rv7.0a1) Gecko/20110628 Firefox/7.0a1 Fennec/7.0a1
Device: HTC Flyer
OS: Android 2.3

Mozilla/5.0 (Android; Linux armv71; rv7.0a1) Gecko/20110628 Firefox/7.0a1 Fennec/7.0a1
Device: Thunderbolt
OS: Android 2.2

Maybe this is a honeycomb issue?  Another possibility is that maybe one of the options for the VKB/IME was toggled that is not default?

Comment 3

[Sungwook Park]

This bug can be NOT reproduce on Galaxy Tab 7 inch and Galaxy S running on version below Android 2.3.

I can find this issue when I have tested Galaxy Tab 10.1 inch and Galaxy Tab 8.9 inch running Android 3.0(Honeycom)

This issue is reproduced on all Korean IME except for Google Korean IME.

Comment 4

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Thanks for the information, Sungwook.  It does appear to be a Honeycomb issue.  Android 3.0

I was able to reproduce with :
Mozilla/5.0 (Android; Linux armv71; rv7.0a1) Gecko/20110628 Firefox/7.0a1 Fennec/7.0a1
Device: Motorola Xoom
OS: Android 3.0

It appears that it may have to do with languages that commit such as Korean, Japanse (with Simeji) and Chinese (with Google Pinyin) on Honeycomb

Comment 5

[Sungwook Park]

yes, I think so. 
This issue is only appeared to Honeycomb with language using commit composing.
I guess the cause of this issue is come from something between Android Inputconnection and the edit of Firefox.

Comment 6

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Changing the title since it's not just limited to Korean.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

maybe, this is a bug of widget.

Comment 8

[Makoto Kato [:m_kato]]

This occurs when onTextChanged is called...

Comment 9

[Makoto Kato [:m_kato]]

Attachment: fix

Comment 10

[Makoto Kato [:m_kato]]

Comment on attachment 550288 [details] [diff] [review]
fix

canel due to some bugs

Comment 11

[Makoto Kato [:m_kato]]

Attachment: fix v2

Comment 12

[Makoto Kato [:m_kato]]

Simple test step
1. Turn on dingulhangul IME
2. input abc
3. tap [Back] key

Result
"b" and "c" is removed

Expected Result
only "c" is removed.

Comment 13

[Makoto Kato [:m_kato]]

Attachment: fix v2

Comment 14

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Adding comment of https://bugzilla.mozilla.org/show_bug.cgi?id=672661#c6 .

Comment 15

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

Comment on attachment 550964 [details] [diff] [review]
fix v2

Thanks. Do you know why we didn't need this before? Does Honeycomb handle KEYCODE_DEL in TextKeyListener?

Comment 16

[Makoto Kato [:m_kato]]

(In reply to Jim Chen [:jchen] (mobile intern :) from comment #15)
> Comment on attachment 550964 [details] [diff] [review] [diff] [details] [review]
> fix v2
> 
> Thanks. Do you know why we didn't need this before? Does Honeycomb handle
> KEYCODE_DEL in TextKeyListener?

When press [del] on default keyboard (or most IME keyboards except to dingulhangul IME), GeckoSurface.onKeyDown() is called, then, KeyListener returns false.  So key press event is generated by sendEventToGecko().

But, when using dingulhangul IME, KeyListener calls onTextChanged, then return false.  This IME's behavior is strange.

[del] should be ignore for KeyListener and it should be handled by Gecko's editor code.

Comment 17

[Makoto Kato [:m_kato]]

Oops, previous comment is mistake.  So, please ignore comment #16.

(In reply to Jim Chen [:jchen] (mobile intern :) from comment #15)
> Comment on attachment 550964 [details] [diff] [review] [diff] [details] [review]
> fix v2
> 
> Thanks. Do you know why we didn't need this before? Does Honeycomb handle
> KEYCODE_DEL in TextKeyListener?

On all software keyboards except to dingulhangul IME, GeckoSurface.onKeyDown() is called, event flags has software keyboard flag, So key press event is generated by sendEventToGecko() without KeyListener.

But, dingulhangul IME has no software keyboard flag.  So, when using dingulhangul IME, KeyListener calls onTextChanged, then return false.  So delete becomes twice.

So we should send to Gecko directly if onKeyDown for [del].

Comment 18

[Makoto Kato [:m_kato]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/10b15bde43e4

Comment 19

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/10b15bde43e4

Comment 21

[Mark Finkle (:mfinkle) (use needinfo?)]

Very simple patch. Know to fix this issue on samsung galaxy tab 10.1, which is a popular tablet, even at Mozilla.

Comment 22

[Mark Finkle (:mfinkle) (use needinfo?)]

Also fixes the same problem on the ASUS transformer tablet.

Comment 23

[Tony Chung [:tchung]]

I hit this on the Samsung Galaxy Tab 10.1, default Keyboard today.  No special IME keyboard. Since this is not fixed on beta, this will need to be relnoted when we live to Market.

I dont feel its a blocker.  This works on nightly as expected.

Broken on Mozilla/5.0 (Android; Linux armv7l; rv:6.0) Gecko/20110810 Firefox/6.0 Fennec/6.0

It is verified on Nightly: Mozilla/5.0 (Android; Linux armv7l; rv:8.0a1) Gecko/20110811 Firefox/8.0a1 Fennec/8.0a1

Comment 24

[Asa Dotzler [:asa]]

Comment on attachment 550964 [details] [diff] [review]
fix v2

Aurora [7] window has closed. Moving approval request to Beta.

Comment 25

[LegNeato]

Comment on attachment 550964 [details] [diff] [review]
fix v2

Approved for mozilla-beta

Comment 26

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed http://hg.mozilla.org/releases/mozilla-beta/rev/7e3966aece57

Comment 27

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

verified-beta (works with swype, simeji, dingulhangul, and more)

Mozilla/5.0 (Android;Linux armv7I; rv:7.0) Gecko/20110824 Firefox/7.0 Fennec/7.0
7 Beta 2
Device: Toshiba Thrive
OS Android 3.1

Comment 28

[Kevin Brosnan [Ex-Mozilla]]

Just a note Verified aurora is not needed since the patch landed on the trunk during the previous 6 week cycle. The patch was part of the Aurora merge.