Closed Bug 919279 Opened 11 years ago Closed 10 years ago

No consistent way to get from web page to menu bar and back using braille display

Categories

(Core :: Disability Access APIs, defect)

25 Branch
All
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: henk, Assigned: fredw)

Details

Attachments

(2 files, 7 obsolete files)

7.23 KB, patch
Details | Diff | Splinter Review
5.64 KB, patch
eeejay
: review+
Details | Diff | Splinter Review
User Agent: Mozilla/5.0 (compatible; MSIE 10.0; Windows NT 6.1; WOW64; Trident/6.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C; .NET4.0E; InfoPath.2)

Steps to reproduce:

I was reading a web page on my Android device using a braille display and wanted to go somewhere else.


Actual results:

I couldn't find a key combination to switch focus to the top bar without using the toch screen.


Expected results:

There should be some keypress to switch focus from the web page to the menu and back.
Max, not sure if this is possible, but if there was a way to register commands with BrailleBack that we could execute on, it would be aesome to have a braille way to quickly get to the Awesome Bar at least. Maybe there is even a command to open the menu of any app we could also hook into?
I can confirm the same issue with Talkback when using swipe gestures to move forward/backward.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 7 → Android
Hardware: x86_64 → All
Summary: No consistant way to get from web page to menu bar and back using braille display on android → No consistent way to get from web page to menu bar and back using braille display
Attached patch Patch V1 (obsolete) — Splinter Review
The problem I see with Talkback is that we have two accessibility cursors: Android's one and Gecko's one. In order to forward the moves of the former to the latter, we use a virtual accessibility tree with three nodes:

VIRTUAL_CURSOR_PREVIOUS
VIRTUAL_CURSOR_POSITION
VIRTUAL_CURSOR_NEXT

Inside the Gecko View, we are always on the VIRTUAL_CURSOR_POSITION node and asking focus on the previous/next node is instead just transmitted as a backward/foward move to Gecko. There are two issues with that approach:

1) When we move from the previous/next view to the Gecko view, we arrive on the VIRTUAL_CURSOR_PREVIOUS or VIRTUAL_CURSOR_NEXT nodes and so Gecko's cursor unexpectedly goes backward/forward. But actually if Gecko does not have the focus yet, these moves are ignored and the focus is never done.

2) When we are inside the Gecko view and arrive at the beginning/end of the document, going backward/forward does not allow us to leave the VIRTUAL_CURSOR_POSITION node and so we can not go outside of the Gecko view, as one would expect.

I attach an experimental patch that fixes 1) and almost 2) (you need three moves to go outside the view..). 1) is relatively easy to fix, I've just added some dummy VIRTUAL_ENTER_FORWARD and VIRTUAL_ENTER_BACKWARD nodes to prevent the user to arrive on VIRTUAL_CURSOR_PREVIOUS or VIRTUAL_CURSOR_NEXT and to submit the focus signal to Gecko. For 2), I've done a hack with a sLatestCursor variable to detect whether we want to go out the Gecko View. I think the proper way to handle that would instead to make Gecko submit some signals when we are at the beginning/end and try to move backward/forward. That way, the Gecko view could list the signals and move focus to the previous/next view.
Is it a good idea (and how?) to detect when the cursor arrives at the beginning/end of the document here:

http://dxr.mozilla.org/mozilla-central/source/accessible/jsat/ContentControl.jsm#94
Flags: needinfo?(marco.zehe)
(In reply to Frédéric Wang (:fredw) from comment #4)
> Is it a good idea (and how?) to detect when the cursor arrives at the
> beginning/end of the document here:

This patch modifies ContentControl.jsm to send accessibility events to Android when we arrive at the beginning/end of the document.

I've made several attempts to move the accessibility focus before/after the Gecko view, but didn't find a simple way to do that (using focusSearch, requestFocus, etc). I suspect we'll need to send an accessibility event ACTION_NEXT/PREVIOUS_AT_MOVEMENT_GRANULARITY with the VIRTUAL_CURSOR_ENTER_FORWARD/BACKWARD as the source, but didn't try yet.
Flags: needinfo?(marco.zehe)
OK, parts 1 and 2 should definitely be reviewed by Eitan (:eeejay), as he wrote the original implementation and is our JSAT/GeckoAccessibility mastermind.
This is my attempt to fix issue 2). I'm able to detect when we arrive at the end/beginning of the Gecko view, send the "exitView" event and catch it in GeckoAccessibility. However, I'm still not able to move the accessibility focus forward/backward. Any suggestion for that would be helpful.
Attachment #8476008 - Attachment is obsolete: true
Flags: needinfo?(eitan)
Attachment #8476930 - Flags: feedback?(eitan)
Flags: needinfo?(eitan)
Comment on attachment 8476925 [details] [diff] [review]
Part 1 - Add some virtual accessibility nodes to GeckoAccessibility in order to handle when the user enters the view forward or backward

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

This looks correct. Didn't have a chance to test it, assuming you have. Thanks!
Attachment #8476925 - Flags: review?(eitan) → review+
Comment on attachment 8476930 [details] [diff] [review]
Part 2 - Send Accessibility:Event to indicate when the user wants to exit the view.

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

This looks good!

::: accessible/jsat/ContentControl.jsm
@@ +365,5 @@
>    },
>  
>    sendToParent: function cc_sendToParent(aMessage) {
> +    if (aMessage.json.origin == 'top' && aMessage.json.action != null) {
> +      if (Utils.MozBuildApp === 'mobile/android') {

I would flip these conditions, first check if we are on android, then the aMessage details.
Attachment #8476930 - Flags: feedback?(eitan) → feedback+
Comment on attachment 8476930 [details] [diff] [review]
Part 2 - Send Accessibility:Event to indicate when the user wants to exit the view.

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

(In reply to Eitan Isaacson [:eeejay] from comment #11)
> Comment on attachment 8476930 [details] [diff] [review]
> Part 2 - Send Accessibility:Event to indicate when the user wants to exit
> the view.
> 
> Review of attachment 8476930 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks good!

Yes, unfortunately the GeckoAccessibility part to move the cursor does not work for me... I'm not exactly sure what would be the right way to handle that...

::: accessible/jsat/ContentControl.jsm
@@ +365,5 @@
>    },
>  
>    sendToParent: function cc_sendToParent(aMessage) {
> +    if (aMessage.json.origin == 'top' && aMessage.json.action != null) {
> +      if (Utils.MozBuildApp === 'mobile/android') {

Don't we want an early return on other platforms (or send similar messages in the future)? Not sure how important it is...
(In reply to Eitan Isaacson [:eeejay] from comment #10)
> Comment on attachment 8476925 [details] [diff] [review]
> Part 1 - Add some virtual accessibility nodes to GeckoAccessibility in order
> to handle when the user enters the view forward or backward
> 
> Review of attachment 8476925 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This looks correct. Didn't have a chance to test it, assuming you have.
> Thanks!

I only tested it with Talkback and only part 1 works for me...

Marco: are you able to test the patches with a braille display and see if that improves the situation?
Flags: needinfo?(marco.zehe)
For me the main problem is that you can't break out of the view you are in. See the braille display as a kind of trackpad/mouse, which is being restricted by the boundaries of the view you are in. So once that mouse is in a web page, moving it up/down/right/left always makes it bounce against the borders of that current web page. With talkback this problem is not that big, because you can always tap on a spot outside the view to change focus to e.g. the address bar. With the braille display you don't have the option of picking an exact location and clicking there, you always have to move there using a mouse-like movement. So for me the prefered way would be to have a function that would cycle through the views on a page, preferably saving the focus within each view. Something like F6 on windows, where focus cycles between addres bar, tool bar and web page.
(In reply to Frédéric Wang (:fredw) from comment #13)
> Marco: are you able to test the patches with a braille display and see if
> that improves the situation?

My build system for Firefox for Android is currently busted. :( It would probably be quicker if you threw these patches against try and I pull a build from there. So why don't you kick off a try build with your latest work on this bug and put the link here so I can check it out with braille? Thanks!
Flags: needinfo?(marco.zehe)
(In reply to Henk Abma from comment #14)
> For me the main problem is that you can't break out of the view you are in.
> See the braille display as a kind of trackpad/mouse, which is being
> restricted by the boundaries of the view you are in. So once that mouse is
> in a web page, moving it up/down/right/left always makes it bounce against
> the borders of that current web page. With talkback this problem is not that
> big, because you can always tap on a spot outside the view to change focus
> to e.g. the address bar. With the braille display you don't have the option
> of picking an exact location and clicking there, you always have to move
> there using a mouse-like movement. So for me the prefered way would be to
> have a function that would cycle through the views on a page, preferably
> saving the focus within each view. Something like F6 on windows, where focus
> cycles between addres bar, tool bar and web page.

Henk, I am not sure this is possible. We'd need to add/inject some sort of braille command into BrailleBack and thus make it available, or provide mappings for, each braille display that is supported. I don't think apps can do that.
I am a bit swamped right now. But I plan to play with these patches sometime this week.
My other attempt for part 2 was to get an AccessibilityNodeInfo for the Gecko View in order to call focusSearch(forward/backward) and then move the (accessibility) focused node. However, I got a weird message "can not do that on a sealed instance". I was using View::createAccessibilityNodeInfo. I plan to try with AccessibilityNodeInfo::obtain tomorrow morning.
Attached patch Part 2 (alternative version) (obsolete) — Splinter Review
(the "sealed / not sealed" issue depends on whether or not we use the function in an AccessibilityService)

Here is another attempt for part 2 to try to exit the view, but that was not more successful.
(In reply to Eitan Isaacson [:eeejay] from comment #17)
> I am a bit swamped right now. But I plan to play with these patches sometime
> this week.

Just setting needinfo so that this is not forgotten...
Flags: needinfo?(eitan)
Comment on attachment 8476930 [details] [diff] [review]
Part 2 - Send Accessibility:Event to indicate when the user wants to exit the view.

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

Changing my feedback here, as this approach will not work. Sorry it took so long to get back to you. This codebase is out of my usual stomping grounds, I needed to test and relearn stuff.

::: mobile/android/base/GeckoAccessibility.java
@@ +269,5 @@
> +                public void run() {
> +                    sEventMessage = null;
> +                    final AccessibilityEvent accEvent = AccessibilityEvent.obtain(after ? AccessibilityNodeInfo.ACTION_NEXT_AT_MOVEMENT_GRANULARITY : AccessibilityNodeInfo.ACTION_PREVIOUS_AT_MOVEMENT_GRANULARITY);
> +                    view.onInitializeAccessibilityEvent(accEvent);
> +                    accEvent.setSource(view, after ? VIRTUAL_ENTRY_POINT_AFTER : VIRTUAL_ENTRY_POINT_BEFORE);

This won't work. This event type is for text navigation only. There is no object navigation event. The next or previous object are at the discretion of the screen reader as it examines the tree.
Attachment #8476930 - Flags: feedback+ → feedback-
The patch above works with the caveat that you need to swipe one additional time to get the the chrome. So in conjunction with Fred's Part one, this handles all the cases we need.
Flags: needinfo?(eitan)
Comment on attachment 8494932 [details] [diff] [review]
Send Accessibility:Event to indicate when the user wants to exit the view.

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

I applied this patch of top of attachment 8476925 [details] [diff] [review]. This seems to improve entering the view (sometimes my patch alone required to swipe twice to enter the view) but I'm still not able to go out of the view. I didn't try to debug. I tested with Talkback (not a braille display).

::: accessible/jsat/Presentation.jsm
@@ +454,5 @@
> +AndroidPresenter.prototype.noMove =
> +  function AndroidPresenter_noMove(aMoveMethod) {
> +    return {
> +      type: this.type,
> +      // details: [{ exitView: aMoveMethod }]

I guess this commented code should be removed
Attachment #8475361 - Attachment is obsolete: true
Attachment #8476930 - Attachment is obsolete: true
Attachment #8479160 - Attachment is obsolete: true
(In reply to Frédéric Wang (:fredw) from comment #24)
> I applied this patch of top of attachment 8476925 [details] [diff] [review].
> This seems to improve entering the view (sometimes my patch alone required
> to swipe twice to enter the view) but I'm still not able to go out of the
> view. I didn't try to debug. I tested with Talkback (not a braille display).

I tried again today and that seems to work for me now, so I guess we'll be able to take these patches.
@Marco: I just submitted https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=0a91281e301e Can you please test the geckoview_example.apk app? It basically only displays a GeckoView with the about:mozilla page. With a nightly build (https://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/latest-mozilla-central-android/en-US/) if I select the activity's title at the top left corner, then I can not swipe to enter and exit the GeckoView. With the two patches applied (https://ftp-ssl.mozilla.org/pub/mozilla.org/firefox/try-builds/fred.wang@free.fr-0a91281e301e) that works for me with Talkback. It would be good to know if that's also the case with a braille display.
Flags: needinfo?(mzehe)
Yup, works as expected for me.
Flags: needinfo?(mzehe)
Comment on attachment 8522396 [details] [diff] [review]
Part 2 - Send Accessibility:Event to indicate when the user wants to exit the view.

Thanks Marco. So if Eitan is happy with these patches, I think we can take them now.
Attachment #8522396 - Flags: review?(eitan)
Comment on attachment 8522396 [details] [diff] [review]
Part 2 - Send Accessibility:Event to indicate when the user wants to exit the view.

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

Perfect! I'm glad you found the new no-move feature.
Attachment #8522396 - Flags: review?(eitan) → review+
https://hg.mozilla.org/mozilla-central/rev/d2b731176642
https://hg.mozilla.org/mozilla-central/rev/9762c79d8f24
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.