762309 - Virtual keyboard does not appear when Find in Page bar is shown, or disappear when it's hidden

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Matt Brubeck (:mbrubeck)]

Steps to reproduce:
1. Choose "Find in Page" from the fennec menu.

Expected results: On-screen keyboard appears.
Actual results: On-screen keyboard does not appear.

2. Tap in the find bar's text input field.
3. Tap on the "X" button to close the find bar.

Expected results: On-screen keyboard disappears.
Actual results: On-screen keyboard is still visible but typing on it does nothing (unless you have a page loaded that handles the key events).

Nominating for tracking because this is a bug in a new feature in Fennec 15.

Comment 1

[Matt Brubeck (:mbrubeck)]

Attachment: WIP

This isn't straightforward because requestFocus() does not take effect right away, so I found that I had to add a postDelay before calling showSoftInput(), as in this example:
http://stackoverflow.com/questions/7289335/soft-keyboard-shows-up-on-edittext-focus-only-once

I'm currently looking to see if there's a less hacky way to do this.

Comment 2

[Matt Brubeck (:mbrubeck)]

Just an update; I haven't found any less-hacky way to fix this.  I tried calling showSoftInput from a focus change listener, but that still happened too early.  I also tried passing all the various different flags to showSoftInput and/or Window.setSoftInputMode, but none of them worked any better.

I found another suggestion on Stack Overflow to send fake MotionEvents, but this still required a delay and seemed even hackier:
http://stackoverflow.com/questions/5105354/how-to-show-soft-keyboard-when-edittext-is-focused

I'll request review for the hacky approach if nothing else works...

Comment 3

[Matt Brubeck (:mbrubeck)]

Attachment: hacky patch

What do you think; is this an acceptable hack?  Anything else we should try first?

I added a delay to the hideSoftInputFromWindow call too, because I was concerned about the hide and show getting called out of order if the find bar was shown and hidden quickly.

SHOW_FORCED was necessary to get this patch to work on my Gingerbread phone.  SHOW_IMPLICIT worked only on my ICS tablet.

My least favorite part of this hack is the magic number 100.  I'm not sure how small this can be and still work across all devices. :(

Comment 4

[Brian Nicholson (:bnicholson)]

(In reply to Matt Brubeck (:mbrubeck) from comment #3)
> Created attachment 638930 [details] [diff] [review]
> hacky patch
> 
> What do you think; is this an acceptable hack?  Anything else we should try
> first?
> 

This is the statement that causes this method to return false: http://grepcode.com/file/repository.grepcode.com/java/ext/com.google.android/android/2.3.4_r1/android/view/inputmethod/InputMethodManager.java#747.

For us, it looks like 1) mServedView != view, 2) mServedView != null, and 3) mServedView.checkInputConnectionProxy(view) == false (verified via reflection).

mServedView does get set to mFindText after calling mFindText.requestFocus(), but as you said, there seems to be a delay for some reason. I wonder which view mServedView refers to...if it's always the same view, and we understand why it's the mServedView, we could perhaps override checkInputConnectionProxy() to return true to fix this.

Another option might be to simply use a different view for showSoftInput(). GeckoApp.mAppContext.getCurrentFocus() returns the focused view (which I assume is the same as mServedView?), so perhaps we could use that as the target view for showSoftInput(). This seems to work for me, but I haven't tested it too thoroughly.

Comment 6

[Matt Brubeck (:mbrubeck)]

Attachment: WIP 2

This overrides checkInputConnectionProxy as Brian suggested above, in LayerView.  It works on my phone, but not on my tablet, presumably because mServedView is set to a different view in the tablet layout.

Comment 7

[Matt Brubeck (:mbrubeck)]

On the tablet, mServedView is set to an instance of android.widget.PopupWindow$PopupViewContainer, possibly from the menu popup in BrowserToolbar.

Comment 8

[Matt Brubeck (:mbrubeck)]

This is tricky in the tablet layout, because at the time FindInPageBar is called, InputMethedManager.mCurRootView is still set to the root of the menu popup rather than the main window, which causes it to ignore focus events from the main window (see where it bails out in focusInLocked).  I tried wrapping mFindInPageBar.show in a main thread runnable, but still faced the same problem.

Comment 9

[Matt Brubeck (:mbrubeck)]

Attachment: patch

I found that the problem on the tablet was that the window focus had not been updated yet.  Waiting for onWindowFocusChanged solved the problem, and also works in the phone layout without the need for the other hacks.

This patch applies on top of the patch from bug 771525.

Comment 10

[Matt Brubeck (:mbrubeck)]

Attachment: patch v2

Comment 11

[Matt Brubeck (:mbrubeck)]

Comment on attachment 641227 [details] [diff] [review]
patch v2

Forgot a qrefresh.  (Also submitted this attachment too soon by accident.)

Comment 12

[Brian Nicholson (:bnicholson)]

Comment on attachment 641227 [details] [diff] [review]
patch v2

Congrats on getting this to work!

Comment 13

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ae98903e9a5

Comment 14

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/9ae98903e9a5

Comment 15

[Matt Brubeck (:mbrubeck)]

Comment on attachment 641227 [details] [diff] [review]
patch v2

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 695172 (find in page)

User impact if declined: Find In Page is slightly more cumbersome/confusing to use, since it requires a second tap before typing is possible.

Testing completed (on m-c, etc.): Landed on m-c 7/12.

Risk to taking this patch (and alternatives if risky): This is a mobile-only polish patch for a new feature in Fx15.  It is quite self-contained and fairly low risk.  The code it touches affects only the Find In Page bar.

String or UUID changes made by this patch: None.

Comment 16

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/39f7336125b6