Last Comment Bug 714950 - Unable to see characters typed in landscape fullscreen VKB
: Unable to see characters typed in landscape fullscreen VKB
Status: VERIFIED FIXED
[VKB]
: inputmethod, regression
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: Trunk
: ARM Android
P2 normal (vote)
: Firefox 12
Assigned To: Alex Pakhotin (:alexp)
:
: Sebastian Kaspari (:sebastian)
Mentors:
http://mail.yahoo.com
: 715993 718243 (view as bug list)
Depends on:
Blocks: 708774
  Show dependency treegraph
 
Reported: 2012-01-03 13:54 PST by Aaron Train [:aaronmt]
Modified: 2012-01-20 13:45 PST (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
11+


Attachments
screenshot on Galaxy Nexus (58.26 KB, image/png)
2012-01-04 21:22 PST, Tony Chung [:tchung]
no flags Details
screenshot on SGS2 (61.38 KB, image/png)
2012-01-04 21:23 PST, Tony Chung [:tchung]
no flags Details
Fix (12.10 KB, patch)
2012-01-06 18:34 PST, Alex Pakhotin (:alexp)
no flags Details | Diff | Splinter Review
Fix v2 (11.83 KB, patch)
2012-01-10 19:03 PST, Alex Pakhotin (:alexp)
blassey.bugs: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description User image Aaron Train [:aaronmt] 2012-01-03 13:54:44 PST
On the bug URL: http://mail.yahoo.com, rotating device to landscape any input I enter into the fields 'Yahoo! ID' or 'Password' is not displayed in the edit-box of the virtual-keyboard. This looks like a regression.
Comment 1 User image Aaron Train [:aaronmt] 2012-01-03 13:55:50 PST
Tested on:
Motorola Droid Pro (Android 2.3)
Mozilla/5.0 (Android; Linux armv7l; rv:12.0a1) Gecko/20120103 Firefox/12.0a1 Fennec/12.0a1
Comment 2 User image Alex Pakhotin (:alexp) 2012-01-03 17:32:40 PST
This is a more general issue. The problem is with native fullscreen keyboard UI. As I already mentioned in the bug 708774 it was broken by the recent major change to the GeckoInputConnection class. The input handling has been significantly simplified to fix the issues we had, but that caused some side-effects.
I am working on this.
Comment 3 User image Alex Pakhotin (:alexp) 2012-01-03 17:35:53 PST
(In reply to Alex Pakhotin (:alexp) from comment #2)
> it was broken by the recent major change to the GeckoInputConnection class.

For the record - that was bug 595008, so it's a known point of regression.
Comment 4 User image Tony Chung [:tchung] 2012-01-04 21:22:13 PST
Created attachment 585972 [details]
screenshot on Galaxy Nexus

happens on Galaxy Nexus, both Aurora and Nightly.  2012-01-04 build, 11.0a2 and 12.0a1

Repro'd on google.com
Comment 5 User image Tony Chung [:tchung] 2012-01-04 21:23:15 PST
Created attachment 585973 [details]
screenshot on SGS2

surprisingly, i can't reproduce on my SGS2, android 2.3.3
Comment 6 User image Alex Pakhotin (:alexp) 2012-01-04 21:34:52 PST
(In reply to Tony Chung [:tchung] from comment #5)
> Created attachment 585973 [details]
> screenshot on SGS2
> 
> surprisingly, i can't reproduce on my SGS2, android 2.3.3

This is not a fullscreen VKB. The problem we have is only with so called "extracted text UI" - when IME opens its own UI with an edit box, which covers the application completely.
Comment 7 User image Matt Brubeck (:mbrubeck) 2012-01-06 11:49:58 PST
*** Bug 715993 has been marked as a duplicate of this bug. ***
Comment 8 User image Alex Pakhotin (:alexp) 2012-01-06 18:34:56 PST
Created attachment 586631 [details] [diff] [review]
Fix

Update extracted text UI on the content changes.

The patch also contains some debug output tweaking for GeckoInputConnection.
Comment 9 User image Brad Lassey [:blassey] (use needinfo?) 2012-01-08 22:46:45 PST
Comment on attachment 586631 [details] [diff] [review]
Fix

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

::: mobile/android/base/GeckoInputConnection.java
@@ +210,5 @@
> +
> +        return setSelectionInternal(start, end);
> +    }
> +
> +    private boolean setSelectionInternal(int start, int end) {

why do we need setSelectionInternal()? just call super.setSelection() in setSelection();
Comment 10 User image Alex Pakhotin (:alexp) 2012-01-09 10:35:01 PST
(In reply to Brad Lassey [:blassey] from comment #9)

> why do we need setSelectionInternal()? just call super.setSelection() in
> setSelection();

It is also called from notifySelectionChange(). I wanted to add a method, which does stuff specific to this subclass, but without calling Gecko. Right now it simply calls the super class. Of course could call super.setSelection() directly in both places - I wasn't quite sure what would be better, an extra layer just seemed a bit safer for the future.
Comment 11 User image Alex Pakhotin (:alexp) 2012-01-10 19:03:57 PST
Created attachment 587572 [details] [diff] [review]
Fix v2

Got rid of setSelectionInternal().
Comment 12 User image Brad Lassey [:blassey] (use needinfo?) 2012-01-13 10:56:01 PST
Comment on attachment 587572 [details] [diff] [review]
Fix v2

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

::: mobile/android/base/GeckoInputConnection.java
@@ +629,3 @@
>          switch (type) {
>          case NOTIFY_IME_RESETINPUTSTATE:
> +            if (DEBUG) Log.d(LOGTAG, ". . . notifyIME: reset");

why change this logging?

@@ +650,5 @@
>              IMEStateUpdater.enableIME();
>              break;
>  
>          case NOTIFY_IME_CANCELCOMPOSITION:
> +            if (DEBUG) Log.d(LOGTAG, ". . . notifyIME: cancel");

why change this logging?

@@ +655,5 @@
>              IMEStateUpdater.resetIME();
>              break;
>  
>          case NOTIFY_IME_FOCUSCHANGE:
> +            if (DEBUG) Log.d(LOGTAG, ". . . notifyIME: focus");

why change this logging?
Comment 13 User image Alex Pakhotin (:alexp) 2012-01-13 11:10:26 PST
(In reply to Brad Lassey [:blassey] from comment #12)
> >          switch (type) {
> >          case NOTIFY_IME_RESETINPUTSTATE:
> > +            if (DEBUG) Log.d(LOGTAG, ". . . notifyIME: reset");

It's just for consistency with other similar logging in this class, and better readability of the debug log.
Comment 15 User image :Ms2ger (⌚ UTC+1/+2) 2012-01-14 01:49:00 PST
https://hg.mozilla.org/mozilla-central/rev/939f9467fc73
Comment 16 User image Kevin Brosnan [:kbrosnan] 2012-01-14 21:25:09 PST
*** Bug 718243 has been marked as a duplicate of this bug. ***
Comment 17 User image Martijn Wargers [:mwargers] 2012-01-16 09:13:17 PST
Verified fixed in today's build.
Comment 18 User image Brad Lassey [:blassey] (use needinfo?) 2012-01-20 13:04:09 PST
Comment on attachment 587572 [details] [diff] [review]
Fix v2

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: 
typed chars can't be seen in landscape mode
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky):
regressions for untested IMEs
Comment 19 User image Alex Keybl [:akeybl] 2012-01-20 13:05:35 PST
Comment on attachment 587572 [details] [diff] [review]
Fix v2

[Triage Comment]
Mobile only - approved for Aurora.
Comment 20 User image Brad Lassey [:blassey] (use needinfo?) 2012-01-20 13:45:48 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/b6ad62e6f637

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