876689 - Virtual keyboard does not invoke on text input field focus if "Don't keep activities" is enabled

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[u421692]

Attachment: Logs

Environment:
Build: Firefox for Android 24.0a1 (2013-05-28)
Device: Samsung Galaxy S2(Android 4.0.3)

Steps to reproduce:
1. Open any site containing a Text Input Field(google.com; gmail.com; youtube.com, etc).
2. Tap on the Text Input Field.

Expected result:
VKB is opened, and the user can type in Text Field.

Actual result:
The cursor appears in the Text Field, but the VKB is never opened.

Note: I was only able to reproduce this issue on a Samsung Galaxy S2.

Comment 1

[Aaron Train [:aaronmt]]

Regression? Can you find a range if so?

Comment 2

[Aaron Train [:aaronmt]]

This works for me on my GT-I9100 (Galaxy SII International, Android 4.0.4)

Comment 3

[u421692]

The regression window for this issue is:

1. mozilla central
good build: fennec-24.0a1 (25.05.2013)
bad build: fennec-24.0a1 (26.05.2013) 

pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=218785bab4bd&tochange=0fed3377c839

Comment 4

[Aaron Train [:aaronmt]]

Nothing really sticks out from that. Can you try narrowing this with inbound builds? Bug 869156  listed in there, can you try by disabling the dynamic toolbar too?

Comment 5

[u421692]

The regression window for this issue is:

mozilla inbound:
good build: 1369415032
bad build: 1369415692

pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=554361b240cf&tochange=9a330eb80bc3

Comment 6

[Aaron Train [:aaronmt]]

I see this on my Samsung Nexus S trying to invoke the keyboard in about:config!

Finkle, do you see this?

Comment 7

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

Current Nightly works fine on my Nexus S using Android 2.3.7

I tested on abuot:config, google.com and bugzilla.mozilla.org. The VKB appears when expected.

Comment 8

[Aaron Train [:aaronmt]]

(In reply to Mark Finkle (:mfinkle) from comment #7)
> Current Nightly works fine on my Nexus S using Android 2.3.7

Video: http://www.youtube.com/watch?v=5F98NpUz7KA

My device is running Android 4.2, which I doubt should make any difference. I can still reproduce on today's Nightly. The <input> on about:config receives focus as the text selection handles do appear. There is no keyboard invocation.

Comment 9

[Aaron Train [:aaronmt]]

Apparently I was still on yesterday's update and I got an update today when I checked again and things are working for me again.

Pop, please re-open if you can still reproduce on your SII.

Comment 10

[Adrian Tamas (:AdrianT)]

I still can reproduce the issue on Nightly 24.0a1 2013-06-02 on the Samsung Galaxy S2 (Android 4.0.4) with the steps in Comment 0. Reopening. This is only reproducible on the S2 from what I can tell.

Comment 11

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

(In reply to Adrian Tamas from comment #10)
> I still can reproduce the issue on Nightly 24.0a1 2013-06-02 on the Samsung
> Galaxy S2 (Android 4.0.4) with the steps in Comment 0. Reopening. This is
> only reproducible on the S2 from what I can tell.

I see something similar when I turn on the "Do not keep activities" setting in Android. Do you have that setting turned on, Adrian?

Comment 12

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

Attachment: Get the latest LayerView from Gecko interface

FWIW, this patch fixes the particular bug I'm seeing. Brad, was there a reason to keep a separate LayerView reference in GeckoAppShell, versus always calling getGeckoInterface().getLayerView()?

Comment 13

[Aaron Train [:aaronmt]]

Hey Jim, see also bug 879791 I filed today for Kats, same symptoms as this bug.

Comment 14

[Adrian Tamas (:AdrianT)]

(In reply to Jim Chen [:jchen :nchen] from comment #11)
> (In reply to Adrian Tamas from comment #10)
> > I still can reproduce the issue on Nightly 24.0a1 2013-06-02 on the Samsung
> > Galaxy S2 (Android 4.0.4) with the steps in Comment 0. Reopening. This is
> > only reproducible on the S2 from what I can tell.
> 
> I see something similar when I turn on the "Do not keep activities" setting
> in Android. Do you have that setting turned on, Adrian?

It seems that the option was set to ON by someone and I didn't check. The issue was caused by having the option ON. Changing the title to reflect this.

Comment 15

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 758707 [details] [diff] [review]
Get the latest LayerView from Gecko interface

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

The GeckoInterface is intended to be the thing that the application implements to handle more advanced operations of the browser. In fact we probably want to get rid of the getLayerView() method from GeckoInterface and instead have GeckoApp call GeckoAppShell.setLayerView() whenever the LayerView changes.

Somewhat unrelated to this, this is one of the things that will have to be refactored to handle having multiple GeckoViews in one application.

Comment 16

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

Attachment: Update GeckoAppShell's LayerView from GeckoApp (v2)

(In reply to Brad Lassey [:blassey] from comment #15)
> Comment on attachment 758707 [details] [diff] [review]
> Get the latest LayerView from Gecko interface
> 
> Review of attachment 758707 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> The GeckoInterface is intended to be the thing that the application
> implements to handle more advanced operations of the browser. In fact we
> probably want to get rid of the getLayerView() method from GeckoInterface
> and instead have GeckoApp call GeckoAppShell.setLayerView() whenever the
> LayerView changes.

This patch does the trick.

Comment 17

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 759121 [details] [diff] [review]
Update GeckoAppShell's LayerView from GeckoApp (v2)

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

basically right, but I just wrote the same patch with the cleanup I was looking for. Review incoming to you.

Comment 18

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

Comment 19

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

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

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

Do we want to get rid of GeckoApp.getLayerView() entirely? To avoid confusion with GeckoAppShell.getLayerView()

I found quite a few remaining callers of GeckoApp.getLayerView() in Prompt, RobocopAPI, TextSelection, TextSelectionHandle, WebAppImpl.

Comment 20

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch to remove GeckoApp.getLayerView()

solid point. Here's an additional patch to do that.

Comment 21

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

Comment on attachment 759174 [details] [diff] [review]
patch to remove GeckoApp.getLayerView()

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

Looks good with a nit

::: mobile/android/base/WebAppImpl.java
@@ +217,4 @@
>      @Override
>      protected void geckoConnected() {
>          super.geckoConnected();
> +        GeckoAppShell.getLayerView().setOverScrollMode(View.OVER_SCROLL_NEVER);

You can use mLayerView here, since WebAppImpl extends GeckoApp

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7bdf86fd583f
https://hg.mozilla.org/mozilla-central/rev/510da37cba42

Comment 23

[Teodora Vermesan (:TeoVermesan)]

Verified fixed on:
Build: Firefox for Android 25 Nightly( 2013-07-17), Firefox for Android 24 Aurora( 2013-07-17)
Device: LG Nexus 4
OS: Android 4.2.2