760087 - Virtual keyboard continually dismisses on first match and match advancing in Find in Page

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Aaron Train [:aaronmt]]

Steps to Reproduce:

i) http://en.wikipedia.org/wiki/Mozilla
ii) Query for 'Mozilla'
iii) Hit the down arrow, and see the keyboard dismiss


Tested via 

Samsung Galaxy Nexus (Android 4.0.4)
20120531040223
http://hg.mozilla.org/integration/mozilla-inbound/rev/ff2ad97929e8

Comment 1

[Aaron Train [:aaronmt]]

It seems to also dismiss on first match when searching in landscape and in portrait.

Comment 2

[Alex Keybl [:akeybl]]

Is the Find bar still visible? Are we sure this isn't expected behavior?

Comment 3

[Aaron Train [:aaronmt]]

The finder bar is still visible but this prevents one from completing a query for an entire string one wishes to enter. What's happening is that one first match for a partial string, the keyboard dismisses.

STR again:

i) Visit http://neowin.net with the intent to enter 'Windows'
ii) Trigger "Find in Page", start typing "win" or "wind", match is met and keyboard is dismissed without the ability to enter 'Windows'.

Comment 4

[Matt Brubeck (:mbrubeck)]

Possible related, the keyboard does not automatically appear when the Find bar is first activated, although from the code it looks like it is supposed to.

Comment 5

[Matt Brubeck (:mbrubeck)]

When a match is highlighted, we receive a NOTIFY_IME_RESETINPUTSTATE event from Gecko here:
http://hg.mozilla.org/mozilla-central/file/e96e0eaa6d85/mobile/android/base/GeckoInputConnection.java#l931

Then this code steals focus from the Find bar by focusing the content view instead:
http://hg.mozilla.org/mozilla-central/file/e96e0eaa6d85/mobile/android/base/GeckoInputConnection.java#l1043

Comment 6

[Matt Brubeck (:mbrubeck)]

This is a regression from bug 750734.

Comment 7

[Matt Brubeck (:mbrubeck)]

Attachment: patch

This fixes the problem from this bug (comment 0) without regressing 750734.  Instead of stealing focus every time the IME state changes, this just makes sure the view gets focused when it's been tapped.

Review question: Does the new requestFocus() call need to be posted to the main thread handler, like the old one was?  I'm not too clear about what happens on which thread in this code.

Comment 8

[Matt Brubeck (:mbrubeck)]

(In reply to Matt Brubeck (:mbrubeck) from comment #4)
> Possible related, the keyboard does not automatically appear when the Find
> bar is first activated.

This turned out to be unrelated.  I filed bug 762309 for this issue.

Comment 9

[Chris Peterson [:cpeterson]]

Comment on attachment 630766 [details] [diff] [review]
patch

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

r+

Sorry for the delay.

> Review question: Does the new requestFocus() call need to be posted to the main thread handler, like the old one was?  I'm not too clear about what happens on which thread in this code.

I think this is safe. I'm not familiar with the LayerView code, but LayerView.onTouchEvent() is presumably running on the UI thread because it calls mTouchEventHandler.handleEvent() and TouchEventHandler.java has big warnings that it must run on the UI thread. GeckoApp and GeckoAppShell call TouchEventHandler from a Runnable posted to mMainHandler, so I think it is safe to assume your code is safe.

We should probably add some debug code to TouchEventHandler to assert it is called on the correct thread.

Comment 10

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0a468957ace8

Comment 11

[Matt Brubeck (:mbrubeck)]

Comment on attachment 630766 [details] [diff] [review]
patch

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

User impact if declined: Find In Page is unusable because the keyboard disappears while you are typing.

Testing completed (on m-c, etc.): Landed on inbound 6/8

Risk to taking this patch (and alternatives if risky): Low-risk android-only patch.

String or UUID changes made by this patch: None.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0a468957ace8

Comment 13

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/275cf57ac110

Comment 14

[Matt Brubeck (:mbrubeck)]

We could use some FindInPageBar tests.

Comment 15

[Adrian Tamas (:AdrianT)]

The issue is still reproducible on both Nightly 16.0a1 and Aurora 15.0a2 2012-07-08. A follow-up bug has been opened for the issue - please see Bug 771989. Leaving the issue closed but unverified until Bug 771989 is fixed.