1104142 - Shift-tab doesn't go back to location bar if there is text in the search field

Status: VERIFIED FIXED

(Firefox :: Search, defect)

Description

[:Gijs (out for now; he/him)]

STR:

1. open Firefox beta on a new profile
2. focus location bar
3. hit tab -> focus goes to the search bar
4. hit shift-tab -> focus goes back to the url bar
5. hit tab again to focus the search bar
6. type something
7. hit shift-tab

ER:
back in the url bar

AR:
wikipedia is focused

This breaks my muscle memory and is inconsistent.

Comment 1

[Florian Quèze [:florian]]

Philipp, this is something we marked as wontfix in our initial triage, but I don't think Gijs is the only one annoyed by this. Could you please give this some thoughts next year?

Comment 3

[Florian Quèze [:florian]]

Bug 1107266 mentions a use case I wouldn't have expected for <tab>ing out of the searchbox:

(john ruskin from bug 1107266 comment #0)

> This disrupts using the shiftTab + Tab to back out and re-enter and select
> all, to start typing over prior entry (or to erase and start over)

Comment 4

[:Gijs (out for now; he/him)]

I also don't really understand why this is so onerous to fix. People can hit <tab> directly to jump to the one-off search items, instead of shift-tab. I don't see any good reason for <shift-tab> to go to the last item in the list of one-off providers. If we want to provide shortcuts for them, we should find some other way of doing that. The current behaviour breaks basic accessibility guarantees (where you navigate forward with tab and backwards with shift-tab, and so a tab+shift-tab sequence should get you back to where you started) and I can't imagine the current behaviour is per-spec - I assumed the wontfix for 34, but tracking for 35/36 meant that it was too late/risky to fix this for 34, not that we were never going to fix it.

I don't think this needs ux-feedback; I think it just needs a patch that bails out of the tab-handling keyhandler when modifiers (including shift) are pressed. Shouldn't be hard, either.

Florian, does this make sense, or am I missing something?

Comment 5

[Florian Quèze [:florian]]

Attachment: Possible patch

I think this does something that Gijs will like better than the current behavior.

This does a couple things that diverge from behaviors Philipp had specifically specified:
- Pressing <tab> when the last one-off item is highlighted will close the panel; focus moves to the next focusable element (ie. the web content area).
- Pressing shift+<tab> when no one-off item is highlighted will close the panel; focus moves to the previous element (ie. the location bar).
- When the panel is closed due to pressing (shift+)<tab>, the searchbox's text field value isn't reverted to what the user typed. The text field value will only be different if the user used the up/down arrows before pressing <tab>, so this is really an edge case.

I personally like the behavior with this patch better than one implemented in 34, but if we want to take this, I would like someone from UX to ui-review.

Comment 6

[:Gijs (out for now; he/him)]

Comment on attachment 8538509 [details] [diff] [review]
Possible patch

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

I will look at fixing the third thing you noted.

Stephen, can you ux-review+ the first two results of this patch?

::: browser/components/search/content/search.xml
@@ +1007,5 @@
>                // If no selection, select the first or last one-off button.
>                if (aEvent.shiftKey) {
> +                // If shift+tab is pressed when there was no selected one-off,
> +                // let the focus escape the panel.
> +                return;

Remove the else and braces after this (no else after return)

Comment 7

[john]

The use case suggested by me in Bug 1107266 has more to do with the behavior brought on by the new search UI (and does not involve the location bar as a problem), and tabbing out & back from the search text box.  

From the search Box: 
   Tabbing out (to the drop down engine dialog) and back in with a Shift-Tab -does- result in a selected search field

versus

   Shift-Tab out and returning with a Tab  (that is backward into the location bar, and returning forward to the search) does -not- 'select' the text in the search box.   This is the essence of my report, now -- a new behavior with the new engine stuff, that came with v34.  I'm not sure that Florian's patch description solves this problem, or makes it more cumbersome, or doesn't address it at all.



Oddly, I can repeat my reported shift tab backing into the engine area, and getting stuck there, but there is something which results in the above instead, a use distinction which perplexes me.   I've tried using a mouse to land in the search box, in lieu of a tab move, and all sorts of other things, and I can't figure out why in some situations i back into the search engines, and loop, and why sometimes I back into the Location bar.

In any case, the original bug 1107266 issues should be addressed here, in this fix:  non-selecting the search field when re-entering, and getting stuck in a loop and backs over the search and loops back to the engines.

Comment 8

[Florian Quèze [:florian]]

Attachment: Patch v2

The third point I mentioned in comment 5 can in my opinion really be handled in a follow-up.

Comment 9

[:Gijs (out for now; he/him)]

Comment on attachment 8538703 [details] [diff] [review]
Patch v2

Still need that UI-review though... Stephen?

Comment 10

[Stephen Horlander [:shorlander]]

Comment on attachment 8538703 [details] [diff] [review]
Patch v2

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

(In reply to Florian Quèze [:florian] [:flo] from comment #5)
> Created attachment 8538509 [details] [diff] [review]
> Possible patch
> 
> I think this does something that Gijs will like better than the current
> behavior.
> 
> This does a couple things that diverge from behaviors Philipp had
> specifically specified:
> - Pressing <tab> when the last one-off item is highlighted will close the
> panel; focus moves to the next focusable element (ie. the web content area).
> - Pressing shift+<tab> when no one-off item is highlighted will close the
> panel; focus moves to the previous element (ie. the location bar).
> - When the panel is closed due to pressing (shift+)<tab>, the searchbox's
> text field value isn't reverted to what the user typed. The text field value
> will only be different if the user used the up/down arrows before pressing
> <tab>, so this is really an edge case.
> 
> I personally like the behavior with this patch better than one implemented
> in 34, but if we want to take this, I would like someone from UX to
> ui-review.

Makes sense.

Comment 11

[Florian Quèze [:florian]]

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

Comment 12

[Carsten Book [:Tomcat]]

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

Comment 13

[Florian Quèze [:florian]]

Comment on attachment 8538703 [details] [diff] [review]
Patch v2

Approval Request Comment
[Feature/regressing bug #]: fixes annoying regression caused by bug 1088660 in 34 for keyboard users.
[User impact if declined]: no way to shift+tab out of the new search UI.
[Describe test coverage new/current, TBPL]: will be verified by QA.
[Risks and why]: simple patch.
[String/UUID change made/needed]: none.

Comment 14

[Florian Quèze [:florian]]

Hi Florin, could you please ensure this gets the appropriate QA? Thanks!

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b573f064a725
https://hg.mozilla.org/releases/mozilla-beta/rev/cf5b60ff001d

Comment 16

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

I've verified this on Windows 7 x64, Mac OS X 10.9.5 and Ubuntu 14.04 x64, using Firefox 35 Beta 6 (BuildID=20141222200458), latest Firefox 36 Aurora (BuildID=20141223004006), and latest Firefox 37 Nightly (BuildID=20141223030207). Scenario from comment #0 now works as expected (dropdown is also closed as anticipated) and also other scenarios like selecting a suggestion or further tabbing do not affect Shift-Tab.

Comment 18

[Florian Quèze [:florian]]

(In reply to Florian Quèze [:florian] [:flo] from comment #8)
> Created attachment 8538703 [details] [diff] [review]
> Patch v2
> 
> The third point I mentioned in comment 5 can in my opinion really be handled
> in a follow-up.

Filed bug 1124747.