810170 - Galaxy S II landscape mode's extracted text editor does not display text

Status: VERIFIED FIXED

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

Description

[Chris Peterson [:cpeterson]]

STR:
1. On a Galaxy S II, load my favorite IME test page: http://support.mozilla.org/
2. Rotate phone to landscape orientation
3. Tap input focus in the Help page's text input
4. Start typing

RESULT:
The extract text editor does not display the text you are typing, but the VKB's word suggestions match what you are typing.

Comment 2

[Cristian Nicolae (:xti)]

This is a regression from bug 805162. I cannot reproduce it on the build before the patch for 805162 has landed.

Also this issue is reproducing on other devices like Desire Z (Android 2.3.3)

--
Firefox 19.0a1 (2012-11-01)
Device: Galaxy S2
OS: Android 4.0.3

Comment 3

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

Attachment: Properly handle extracted text notification and selection notification (v1)

There are a few places where we should notify the system but we don't. The mBatchSelectionChanged and mBatchTextChanged flags specify that there has been a change during a batch edit, but since we should not notify during a batch edit per the Android docs, we save these so that we can notify later, during the endBatchEdit call.

Comment 4

[Chris Peterson [:cpeterson]]

Comment on attachment 681066 [details] [diff] [review]
Properly handle extracted text notification and selection notification (v1)

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

r+ with nits

::: mobile/android/base/GeckoInputConnection.java
@@ +104,5 @@
> +                }
> +                if (mBatchTextChanged) {
> +                    notifyTextChange();
> +                    mBatchTextChanged = false;
> +                }

I think we might want to send these notifications in the other order: notifyTextChange() before notifySelectionChange(). I worry that, after we've batched some text and selection changes, the extracted text's current selection might not make sense (i.e. out of bounds) for the old, non-yet-updated text. Whereas, it is normal for a text change event to invalidate an old selection.

@@ +225,5 @@
>          final Editable editable = getEditable();
>  
>          mUpdateExtract.flags = 0;
>          // Update from (0, oldEnd) to (0, newEnd) because some IMEs
>          // assume that updates start at zero, according to jchen.

Is this comment still relevant? Maybe we should change it to say why we don't need to send offsets for partial text changes?

Comment 5

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

Addressed review comments in commit

https://hg.mozilla.org/integration/mozilla-inbound/rev/cbff9c26a1b2

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/cbff9c26a1b2

Comment 7

[Cristian Nicolae (:xti)]

I cannot reproduce this issue anymore on the latest Nightly. Typing in a text field with the VKB in full screen mode will display the typed characters correctly.

Closing bug as verified fixed on:

Firefox 19.0a1 (2012-11-15)
Device: Galaxy S2
OS: Android 4.0.3