1106779 - Tab switcher doesn't allow access to bottom tab when keyboard is up

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

Nexus 4 running nightly, but this issue has been around a long time. I only just figured out the STR. Open 4 or more tabs, for example this bug page 4 times. Make sure the URL bar is visible and put focus in a textarea/textfield so the keyboard comes up. If you get an actionbar dismiss it by clicking the check mark. While the keyboard is still up, hit the tab switcher button. ER: all tabs are visible and selectable. AR: can't scroll down enough to select the bottom tab.

Comment 3

[Tyler Geonetta]

I'd like to take this as my first bug. Could anyone help me find what specific area/files I should be looking in? I have fennec building/etc... I just need help navigating the code base!

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Hey, Tyler - you've been assigned. I apologize for the delay! I must have missed the email. -_-

You can use the needinfo flags (set below the "Additional comments" box) to give users notifications at the top of Bugzilla and send them periodic emails, which may help avoid having this happen again.

To answer your question, since we want to look dismiss the keyboard when the tab switcher button is pressed, we should look at the listener that is called when the tab switcher button is opened.

Our main java code base is in mobile/android/base. The BrowserToolbar [1] class, and its sub-classes, contain all of the implementation details of the toolbar. Looking at the XML for the phone implementation of this class, ...resources/layout/browser_toolbar.xml [2], we see that the id for this View is "tabs". This ID is found in BrowserToolbar and a listener is attached - look into adding code to the listener to dismiss the keyboard and show the TabsPanel unobstructed when it is opened.

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/toolbar/BrowserToolbar.java
[2]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/layout/browser_toolbar.xml?rev=540bc6af7020#59

Comment 5

[Tyler Geonetta]

Thank you Michael for your assistance.

I did some investigation into this issue and seem to have pin-pointed it. In the toggleTabs() method (BrowserToolbar.java), there IS logic to hide the keyboard before showing the TabsPanel (as you mentioned).

The problems lies in the fact that the logic to calculate the TabsContainer height (getTabContainerHeight()) is called from TabsPanel.show() BEFORE the keyboard is actually dismissed/hidden (before it has actually collapsed, since it does not happen instantaneously).

We can see this behavior by using the following code to wait until after the keyboard has completely closed (~1sec?):

import android.os.*;
private void toggleTabs() {
        if (activity.areTabsShown()) {
            if (activity.hasTabsSideBar())
                activity.hideTabs();
        } else {
            // hide the virtual keyboard
            InputMethodManager imm =
                    (InputMethodManager) activity.getSystemService(Context.INPUT_METHOD_SERVICE);
            boolean hideResult = imm.hideSoftInputFromWindow(tabsButton.getWindowToken(), 0);

            //handler code below is added by me
            Handler handler = new Handler();
            handler.postDelayed(new Runnable() {
              public void run() {
                Tab tab = Tabs.getInstance().getSelectedTab();
                if (tab != null) {
                  if (!tab.isPrivate())
                  activity.showNormalTabs();
                  else
                  activity.showPrivateTabs();
                }
              }
            }, 1000); //1000 milliseconds

            /* notice this code is now called from the above handler
            Tab tab = Tabs.getInstance().getSelectedTab();
            if (tab != null) {
              if (!tab.isPrivate())
              activity.showNormalTabs();
              else
              activity.showPrivateTabs();
            }
            */
        }
    }


Obviously, we don't want to use that arbitrary 1000 ms delay waiting for the keyboard to close... But I can't seem to find any way to check if the keyboard is closed or not from my research (or any events, etc)... Any ideas/direction?

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Hey, Tyler! I apologize for the delay - I won't let it happen again.

Good research!

Note that will require a little exploration because I'm honestly not sure exactly what the solution is here - I hope you don't mind! If you do, let me know and we can find a more clear-cut bug for you.

I noticed InputMethodManager has a hideSoftInputFromWindow method that accepts a `ResultReceiver` instance - i.e. a callback! You can try this. However, skimming the source [1], I'm skeptical whether layout has actually occurred yet, and thus the TabsContainer height will still be improperly set. If that's the case, we can try using the ViewTreeObserver [2] - I imagine hiding the keyboard will trigger a global layout so we can try the onGlobalLayoutObserver. If not, the OnPreDrawListener or OnDrawListener may be appropriate. I don't know the details of Android layout/drawing so I'm not sure of the implications off-hand for which one you use.

If this still doesn't work, combining both solutions seems like it should almost certainly work, but be sure to try this last as it may be excessive. Let me know if you still need help!

By the way, if you want to make sure your mentor (or other user) sees your question, add a needinfo flag (see the box below this comment field).

[1]: http://androidxref.com/4.4.4_r1/xref/frameworks/base/core/java/android/inputmethodservice/InputMethodService.java#401
[2]: https://developer.android.com/reference/android/view/ViewTreeObserver.html#addOnGlobalLayoutListener%28android.view.ViewTreeObserver

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Hi Tyler, are you still planning on working on this bug? If you have any questions please feel free to ask and we can help you out. Thanks!

Comment 8

[Tyler Geonetta]

I do intend on working on this, I just got distracted over the holidays...

Anyways, if I remember where I left off -- I tried implementing the ResultReceiver idea that Michael suggested. But for the life of me, I couldn't get it working (gist: https://gist.github.com/anonymous/92ed932fdb58f05f16f6).

I want to try the ViewTreeObserver idea that Michael proposed... but not really sure of exactly how that would work. Ideas?

Comment 9

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Tyler Geonetta from comment #8)
> I do intend on working on this, I just got distracted over the holidays...

No worries, we're all busy! :)

> Anyways, if I remember where I left off -- I tried implementing the
> ResultReceiver idea that Michael suggested. But for the life of me, I
> couldn't get it working (gist:
> https://gist.github.com/anonymous/92ed932fdb58f05f16f6).

What else have you tried here? Was the ResultReceiver getting called? Which result did it receive? It might not work if the soft input doesn't actually get hidden from this call. You can add a few print statements and check the logcat output to check the state (unfortunately, afaik we don't currently have debuggers working with our builds :\).

> I want to try the ViewTreeObserver idea that Michael proposed... but not
> really sure of exactly how that would work. Ideas?

If you'd like to try this, the TabStripView class has many examples (e.g. [1]).

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tabs/TabStripView.java?rev=915b53f26a53#126

Comment 10

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Unassigning due to inactivity - let me know if you're still working on this Tyler.

Comment 11

[Tyler St. Onge[:dropkick]]

Im

Comment 12

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Hey, Tyler.

Are you interested in working on this? I've realized this bug might be a bit much for a new contributor, I noticed you were also working on bug 1134927 - do you want to tackle this as a followup to that one?

Comment 13

[Tyler St. Onge[:dropkick]]

(In reply to Michael Comella (:mcomella) from comment #12)
> Hey, Tyler.
> 
> Are you interested in working on this? I've realized this bug might be a bit
> much for a new contributor, I noticed you were also working on bug 1134927 -
> do you want to tackle this as a followup to that one?

I'm not quite sure what happened to the rest of my comment. I was going to say that I'm not able to replicate this on my Note 4, as soon as I tab to switch tabs the keyboard disappears. I ended up starting on a different bug because of that, but thank you.

Comment 14

[Jeff Lu]

Attachment: bug-1106779.patch

Patch attached. 
Boolean returned from hideSoftInputFromWindow() tells us if the keyboard is being hidden. If so, we can listen for the global layout that's triggered after it's gone.

Comment 15

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8580809 [details] [diff] [review]
bug-1106779.patch

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

Nice work, Jeff!

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa99036546ab

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/integration/fx-team/rev/d5fe53d4a359

Comment 17

[Ryan VanderMeulen [:RyanVM]]

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

Comment 18

[Teodora Vermesan (:TeoVermesan)]

Verified fixed:
Device: Nexus 4 (Android 4.4)
Build: Firefox for Android 40.0a1 (2015-04-23) and Firefox for Android 39.0a2 (2015-04-23) 

I am still able to reproduce on 38 Beta 6 and Firefox 37.0.2