854458 - Implement keyboard navigation in TwoWayView

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Lucas Rocha (:lucasr)]

i.e. implement keyboard navigation in TwoWayView.

Comment 1

[Lucas Rocha (:lucasr)]

Attachment: Implement keyboard navigation in TwoWayView

Comment 2

[Eitan Isaacson [:eeejay]]

(In reply to Lucas Rocha (:lucasr) from comment #1)
> Created attachment 730771 [details] [diff] [review]
> Implement keyboard navigation in TwoWayView

Is all that keypress listening and distance calculating necessary? Android has ways of arbitrarily changing focus order, etc. I don't know anything about TwoWayView implementation, so maybe no.

Comment 3

[Lucas Rocha (:lucasr)]

(In reply to Eitan Isaacson [:eeejay] from comment #2)
> (In reply to Lucas Rocha (:lucasr) from comment #1)
> > Created attachment 730771 [details] [diff] [review]
> > Implement keyboard navigation in TwoWayView
> 
> Is all that keypress listening and distance calculating necessary? Android
> has ways of arbitrarily changing focus order, etc. I don't know anything
> about TwoWayView implementation, so maybe no.

It really comes down to what level of "Android" you're talking about. The basic View framework and all built-in widgets implement input focus handling and keyboard navigation behavior in some form. But you still have to implement the specific behavior of your view.

What this is patch implements is how keyboard navigation interacts with the internal state of the view (scrolling, selection, input focus, view recycling, etc).

Comment 4

[Lucas Rocha (:lucasr)]

Eitan/Marco, could you please test this build? It has latest code with this patch applied:

  https://dl.dropbox.com/u/1187037/fennec-keyboard2.apk

Comment 5

[Marco Zehe (:MarcoZ)]

Here's some initial feedback on this build, done on my HTC Sense running CM7 Gingerbread 2.3.6:

1. Open Fennec-Lucasr, and choose one of the home screen pages. I chose "Firefox - Customize with Add-ons".
2. Open the menu and select "New Tab".
3. Choose another page that is different from the first.
4. UpArrow into the Awesome bar, right or left to the "2 tabs" button and press down on the d-pad/press enter.

Result: The tabs panel opens. Focus remains on the "2 tabs" button.
Expected: Focus should go into the panel, preferably onto either the "tabs" button or the item for the currently open tab. But the "tabs" button would do.

5. LeftArrow over the "Enter Search or Address" to the "tabs" button.
6. DownArrow. You land on the first item as expected.
7. DownArrow again. You land on the second item as expected.
8. Right Arrow. You land on a Close Tab button.
9. Press Enter.

Expected: The second tab closes.
Actual: The *first* tab closes.
RightArrowing from the second tab entry to the adjacent close button *should* close the tab that was just read. Otherwise the user interaction is not logical at all!!!

Comment 6

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

Based on Marco's evaluation, should I hold off on the review?

Comment 7

[Lucas Rocha (:lucasr)]

(In reply to Mark Finkle (:mfinkle) from comment #6)
> Based on Marco's evaluation, should I hold off on the review?

I should have probably scoped this bug a bit more narrowly. There are many levels of keyboard navigation. This bug is just about adding support for keyboard navigation in the TwoWayView itself (which is currently non-existent).

There is a broader set of issues with keyboard navigation in our UI that will have to be sorted in separate bugs. kats has found many of them while working on the Ouya stuff (some of them are described in Marco's comment above). So, I'm renaming this bug to match my original intent.

With that said, I have found a couple of bugs in the patch. I'll be submitting a new patch for review in a minute.

Comment 8

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

Comment on attachment 730771 [details] [diff] [review]
Implement keyboard navigation in TwoWayView

waiting for new patch

Comment 9

[Lucas Rocha (:lucasr)]

Attachment: Implement keyboard navigation in TwoWayView

Comment 10

[Lucas Rocha (:lucasr)]

Attachment: Use different focus navigation direction on horizontal tabs tray

Comment 11

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

Comment on attachment 733997 [details] [diff] [review]
Implement keyboard navigation in TwoWayView

Woah, this is a lot of code. I think we need to start considering a test suite for this widget.

Comment 12

[Lucas Rocha (:lucasr)]

Pushed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aca8116e7b0b
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8078621fb2a

I think bug 818438 covers most (if not all) the remaining a11y issues on tabs tray.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/aca8116e7b0b
https://hg.mozilla.org/mozilla-central/rev/b8078621fb2a