Closed Bug 854458 Opened 12 years ago Closed 12 years ago

Implement keyboard navigation in TwoWayView

Categories

(Firefox for Android Graveyard :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 23

People

(Reporter: lucasr, Assigned: lucasr)

References

Details

(Keywords: access)

Attachments

(2 files, 1 obsolete file)

i.e. implement keyboard navigation in TwoWayView.
Summary: Implement keyboard navigation in new tabs tray → Implement keyboard navigation in the new tabs tray
Keywords: access
Attached patch Implement keyboard navigation in TwoWayView (obsolete) (deleted) — Splinter Review
Attachment #730771 - Flags: review?(mark.finkle)
(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.
(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).
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
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!!!
Based on Marco's evaluation, should I hold off on the review?
(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.
Summary: Implement keyboard navigation in the new tabs tray → Implement keyboard navigation in TwoWayView
Comment on attachment 730771 [details] [diff] [review] Implement keyboard navigation in TwoWayView waiting for new patch
Attachment #730771 - Flags: review?(mark.finkle)
Attachment #733997 - Flags: review?(mark.finkle)
Attachment #730771 - Attachment is obsolete: true
Attachment #733998 - Flags: review?(mark.finkle)
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.
Attachment #733997 - Flags: review?(mark.finkle) → review+
Attachment #733998 - Flags: review?(mark.finkle) → review+
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
Blocks: 818438
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: