908719 - Creating a new tab doesn't put focus in the url bar on devices with hw keyboard

Status: RESOLVED INCOMPLETE

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

Description

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

When opening a new tab in Fennec, focus is not automatically placed in the URL bar like it used to be. It looks like this was intentional based on bug 885353, and that makes sense for some devices. For devices *with a hardware keyboard* however I would like to have focus automatically placed in the URL bar so that the user can type the URL right away. There should be no issue with the VKB obscuring the new about:home because the device has a hardware keyboard.

Comment 1

[Lucas Rocha (:lucasr)]

Updating title for clarity.

Comment 2

[Lukas Blakk [:lsblakk] use ?needinfo]

Tracking as part of the about:home redo.

Comment 3

[Lukas Blakk [:lsblakk] use ?needinfo]

Lucas, is this a quick and low risk fix?  We're 2 betas out from shipping so I'm no longer open to taking speculative fixes but if this is a small fix or a backout we could consider it if it's key to the about:home release otherwise we'll have to wontfix and wait for FF27.

Comment 4

[Lucas Rocha (:lucasr)]

(In reply to [e-mail] [:lsblakk] from comment #3)
> Lucas, is this a quick and low risk fix?  We're 2 betas out from shipping so
> I'm no longer open to taking speculative fixes but if this is a small fix or
> a backout we could consider it if it's key to the about:home release
> otherwise we'll have to wontfix and wait for FF27.

I haven't looked into this one yet. I think it's too late to land a fix for it in 26 (and we're not tracking it for 26 anyway).

Comment 6

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

filter on [mass-p5]

Comment 7

[Liam Quinn]

This is still reproducible using Firefox 42. It's noticeable when using Firefox on PRIV by BlackBerry; since that device has a hardware keyboard, users might expect to be able to just type on about:home without first having to tap in the URL bar.

It's also reproducible using Firefox 42 with a Bluetooth keyboard paired to a Nexus 5.

Comment 8

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

Kevin, do you know what percentage of our users have hardware keyboards? It could help us prioritize this (or not).

Comment 9

[Kevin Brosnan [Ex-Mozilla]]

There has not been a mass market Android device with a keyboard by default for 3-4 years. I suspect bluetooth users would be most likely to hit this. Though given how poor our keyboard accessibility is compared to Chrome I doubt we have too many of those users either.

Comment 10

[:Margaret Leibovic]

I filed an Aha card idea to discuss this in the funnel meeting.

Comment 11

[Andy Chen]

Attachment: Put focus on urlbar when user starts typing on the new tab

Comment 12

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

Comment on attachment 8697146 [details] [diff] [review]
Put focus on urlbar when user starts typing on the new tab

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

Looks OK to me, but somebody on the front end team should review this. Redirecting to mcomella.

Comment 13

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

Comment on attachment 8697146 [details] [diff] [review]
Put focus on urlbar when user starts typing on the new tab

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

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
@@ +443,5 @@
>      }
>  
>      @Override
> +    public boolean dispatchKeyEvent(KeyEvent event) {
> +        if (isHomePagerVisible()) {

This breaks keyboard navigation (and thus accessibility navigation). If you touch the arrow keys, the user automatically focuses the url bar, rather than navigating the content. Also, the toolbar expands, which is not intended when navigating via the keyboard.

Keyboard navigation is pretty broken atm but we don't want to break it further.

Perhaps we'd want to have a single key, or a combo, to focus the url bar (e.g. like command+l on desktop).

@@ +444,5 @@
>  
>      @Override
> +    public boolean dispatchKeyEvent(KeyEvent event) {
> +        if (isHomePagerVisible()) {
> +            enterEditingMode();

`enterEditingMode` can have a lot of side effects so it feels dangerous to use it in this context.

Perhaps what you really want is to requestFocus on the UrlEditText?

These events are also triggered for software keyboard events so watch out for side effects here.

Comment 14

[Andy Chen]

Attachment: Put focus on urlbar when user starts typing with hardware keyboard on the new tab

Comment 15

[Andy Chen]

(In reply to Michael Comella (:mcomella) from comment #13)
> Comment on attachment 8697146 [details] [diff] [review]
> Put focus on urlbar when user starts typing on the new tab
> 
> Review of attachment 8697146 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java
> @@ +443,5 @@
> >      }
> >  
> >      @Override
> > +    public boolean dispatchKeyEvent(KeyEvent event) {
> > +        if (isHomePagerVisible()) {
> 
> This breaks keyboard navigation (and thus accessibility navigation). If you
> touch the arrow keys, the user automatically focuses the url bar, rather
> than navigating the content. Also, the toolbar expands, which is not
> intended when navigating via the keyboard.
> 
> Keyboard navigation is pretty broken atm but we don't want to break it
> further.
> 
> Perhaps we'd want to have a single key, or a combo, to focus the url bar
> (e.g. like command+l on desktop).
> 
In the new patch, I added conditions to restrict keycode to 0-9 and a-z on hardware keyboard.
> @@ +444,5 @@
> >  
> >      @Override
> > +    public boolean dispatchKeyEvent(KeyEvent event) {
> > +        if (isHomePagerVisible()) {
> > +            enterEditingMode();
> 
> `enterEditingMode` can have a lot of side effects so it feels dangerous to
> use it in this context.
> 
> Perhaps what you really want is to requestFocus on the UrlEditText?
> 
> These events are also triggered for software keyboard events so watch out
> for side effects here.

I don't see any open API to make UrlEditText field focus without expanding toolbar. I think we should expand toolbar when user start typing on hardward keyboard because it should be the same as user tapping on the urlfield, otherwise it would be in an invalid state.

Comment 16

[Chenxia Liu [:liuche]]

Comment on attachment 8703769 [details] [diff] [review]
Put focus on urlbar when user starts typing with hardware keyboard on the new tab

I'll take over this review.

Comment 18

[Chenxia Liu [:liuche]]

Comment on attachment 8703769 [details] [diff] [review]
Put focus on urlbar when user starts typing with hardware keyboard on the new tab

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

These patches work fine for me, but I don't like putting this code in dispatchKeyEvent, which seems very global and opens this up for new unintended bugs.

I would prefer to see code around or inside the addTab stack for new tabs that open about:home [1] where we check for the keyboard state, and if the keyboard state is a hardware keyboard [2], call enterEditingMode().

[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/Tabs.java#885
[2] https://developer.android.com/reference/android/content/res/Configuration.html#keyboard

Comment 19

[Andy Chen]

(In reply to Chenxia Liu [:liuche] from comment #18)
> Comment on attachment 8703769 [details] [diff] [review]
> Put focus on urlbar when user starts typing with hardware keyboard on the
> new tab
> 
> Review of attachment 8703769 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> These patches work fine for me, but I don't like putting this code in
> dispatchKeyEvent, which seems very global and opens this up for new
> unintended bugs.
> 
> I would prefer to see code around or inside the addTab stack for new tabs
> that open about:home [1] where we check for the keyboard state, and if the
> keyboard state is a hardware keyboard [2], call enterEditingMode().
> 
> [1]
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/
> mozilla/gecko/Tabs.java#885
> [2]
> https://developer.android.com/reference/android/content/res/Configuration.
> html#keyboard

Ok, I'll change it.

Comment 21

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.