Closed Bug 757598 Opened 12 years ago Closed 12 years ago

[AccessFu] Have fallback click for inaccessible javascript

Categories

(Core :: Disability Access APIs, defect)

Product:
Core
Component:
Disability Access APIs
Platform:
ARM
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla15

People

(Reporter: eeejay, Assigned: eeejay)

Details

Attachments

(1 file)

Fallback for items that don't have accessible actions.
1.71 KB, patch
davidb
: review+
Details | Diff | Splinter Review 
http://m.southwest.com uses an inaccessible widget set called kony widgets. It listens to click events on arbitrary un-ARIAed divs. We need a fallback for that, so users could still activate an item.
Comment on attachment 626160 [details] [diff] [review]
Fallback for items that don't have accessible actions.

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

Initial thoughts...

::: accessible/src/jsat/VirtualCursorController.jsm
@@ +154,5 @@
> +      let cwu = this.chromeWin.QueryInterface(Ci.nsIInterfaceRequestor).
> +        getInterface(Ci.nsIDOMWindowUtils);
> +      cwu.sendMouseEventToWindow('mousedown', x, y, 0, 1, 0, false);
> +      cwu.sendMouseEventToWindow('mouseup', x, y, 0, 1, 0, false);
> +    }

Probably safest to scroll into view first (like we do in nsAccessible::DispatchClickEvent)?

Hmmm, now I'm wondering why we don't support forced mouse simulation in our core engine. After all, a user can click anywhere they like... so why not AT?

Anyways, note browser.js has _dispatchMouseEvent - can you get at that?
(In reply to David Bolter [:davidb] from comment #2)
> Comment on attachment 626160 [details] [diff] [review]
> Fallback for items that don't have accessible actions.
> 
> Review of attachment 626160 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Initial thoughts...
> 
> ::: accessible/src/jsat/VirtualCursorController.jsm
> @@ +154,5 @@
> > +      let cwu = this.chromeWin.QueryInterface(Ci.nsIInterfaceRequestor).
> > +        getInterface(Ci.nsIDOMWindowUtils);
> > +      cwu.sendMouseEventToWindow('mousedown', x, y, 0, 1, 0, false);
> > +      cwu.sendMouseEventToWindow('mouseup', x, y, 0, 1, 0, false);
> > +    }
> 
> Probably safest to scroll into view first (like we do in
> nsAccessible::DispatchClickEvent)?
> 

I guess I could add that. The item should already be scrolled to via the visual presenter.

> Hmmm, now I'm wondering why we don't support forced mouse simulation in our
> core engine. After all, a user can click anywhere they like... so why not AT?
> 

Sure, good idea.

> Anyways, note browser.js has _dispatchMouseEvent - can you get at that?

you mean _sendMouseEvent? I don't feel great about accessing a "private" function.
(In reply to Eitan Isaacson [:eeejay] from comment #3)
> (In reply to David Bolter [:davidb] from comment #2)
> > Comment on attachment 626160 [details] [diff] [review]
> > Anyways, note browser.js has _dispatchMouseEvent - can you get at that?
> 
> you mean _sendMouseEvent? I don't feel great about accessing a "private"
> function.

Especially since we won't know they changed it until a user notices.
Comment on attachment 626160 [details] [diff] [review]
Fallback for items that don't have accessible actions.

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

OK r=me
Attachment #626160 - Flags: review?(dbolter) → review+
Comment on attachment 626160 [details] [diff] [review]
Fallback for items that don't have accessible actions.

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

::: accessible/src/jsat/VirtualCursorController.jsm
@@ +153,5 @@
> +
> +      let cwu = this.chromeWin.QueryInterface(Ci.nsIInterfaceRequestor).
> +        getInterface(Ci.nsIDOMWindowUtils);
> +      cwu.sendMouseEventToWindow('mousedown', x, y, 0, 1, 0, false);
> +      cwu.sendMouseEventToWindow('mouseup', x, y, 0, 1, 0, false);

why you don't do acc.DOMNode.click() instead all these expensive computations?
(In reply to Eitan Isaacson [:eeejay] from comment #3)
> (In reply to David Bolter [:davidb] from comment #2)

> > Hmmm, now I'm wondering why we don't support forced mouse simulation in our
> > core engine. After all, a user can click anywhere they like... so why not AT?
> > 
> 
> Sure, good idea.

usually the user operates with something meaningful so why would user need to click at something that doesn't have proper semantics? Are those widgets special?
(In reply to alexander :surkov from comment #6)
> Comment on attachment 626160 [details] [diff] [review]
> why you don't do acc.DOMNode.click()

A good question for Eitan - I was assuming no click handler (since we expose actions in that case IIRC).

> instead all these expensive
> computations?

At least it is not a hotpath (since user action).


(In reply to alexander :surkov from comment #7)
> (In reply to Eitan Isaacson [:eeejay] from comment #3)
> > (In reply to David Bolter [:davidb] from comment #2)
> 
> > > Hmmm, now I'm wondering why we don't support forced mouse simulation in our
> > > core engine. After all, a user can click anywhere they like... so why not AT?
> > > 
> > 
> > Sure, good idea.
> 
> usually the user operates with something meaningful so why would user need
> to click at something that doesn't have proper semantics? Are those widgets
> special?

I assume they are meaningful (interactive).
(In reply to David Bolter [:davidb] from comment #8)
> (In reply to alexander :surkov from comment #6)
> > Comment on attachment 626160 [details] [diff] [review]
> > why you don't do acc.DOMNode.click()
> 
> A good question for Eitan - I was assuming no click handler (since we expose
> actions in that case IIRC).

it's not about click handler iirc

> > instead all these expensive
> > computations?
> 
> At least it is not a hotpath (since user action).

yes but anyway, no point to be slower if we can be faster :)

> > usually the user operates with something meaningful so why would user need
> > to click at something that doesn't have proper semantics? Are those widgets
> > special?
> 
> I assume they are meaningful (interactive).

for AT users?
(In reply to alexander :surkov from comment #6)
> Comment on attachment 626160 [details] [diff] [review]
> Fallback for items that don't have accessible actions.
> 
> Review of attachment 626160 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/src/jsat/VirtualCursorController.jsm
> @@ +153,5 @@
> > +
> > +      let cwu = this.chromeWin.QueryInterface(Ci.nsIInterfaceRequestor).
> > +        getInterface(Ci.nsIDOMWindowUtils);
> > +      cwu.sendMouseEventToWindow('mousedown', x, y, 0, 1, 0, false);
> > +      cwu.sendMouseEventToWindow('mouseup', x, y, 0, 1, 0, false);
> 
> why you don't do acc.DOMNode.click() instead all these expensive
> computations?

For the same reason we don't do it in nsCoreUtils? :)

The click() method is less than bomb-proof. For example, in the site above it does not work. I have no idea how they are listening for events. It might have something to do with the fact that the coordinates that are on the event that click() generates for non-clickable elements are not in the bounding box of the element. Or possibly the event does not bubble up from the element you call click() on.

Besides the xpc calls, this is not more expensive than what is already happening in the a11y engine since it employs the exact same method.
(In reply to alexander :surkov from comment #7)
> (In reply to Eitan Isaacson [:eeejay] from comment #3)
> > (In reply to David Bolter [:davidb] from comment #2)
> 
> > > Hmmm, now I'm wondering why we don't support forced mouse simulation in our
> > > core engine. After all, a user can click anywhere they like... so why not AT?
> > > 
> > 
> > Sure, good idea.
> 
> usually the user operates with something meaningful so why would user need
> to click at something that doesn't have proper semantics? Are those widgets
> special?

Look at the website above. It is a bunch of divs that are styled to look like a mobile app list view.

If it is half inaccessible (no semantic information for the user), it doesn't need to be completely inaccessible. Unfortunately, blind users need to get accustomed to kinks like this on familiar web sites. The fact is that voiceover in iOS deals with this the best it can, so we should as well.
(In reply to Eitan Isaacson [:eeejay] from comment #10)

> > why you don't do acc.DOMNode.click() instead all these expensive
> > computations?
> 
> For the same reason we don't do it in nsCoreUtils? :)
> 
> The click() method is less than bomb-proof. For example, in the site above
> it does not work. 

click generates gecko events while nsCoreUtils emulates user input. So if click() is not enough for you then at least you can skip accessible boundaries computation. Anyway the logic you do is likely wrong because I doubt that nsIDOMWindowUtils works with device pixels.
(In reply to Eitan Isaacson [:eeejay] from comment #11)

> If it is half inaccessible (no semantic information for the user), it
> doesn't need to be completely inaccessible. Unfortunately, blind users need
> to get accustomed to kinks like this on familiar web sites. The fact is that
> voiceover in iOS deals with this the best it can, so we should as well.

ok, I'm fine if Marco finds this useful.
(In reply to alexander :surkov from comment #12)
> (In reply to Eitan Isaacson [:eeejay] from comment #10)
> 
> > > why you don't do acc.DOMNode.click() instead all these expensive
> > > computations?
> > 
> > For the same reason we don't do it in nsCoreUtils? :)
> > 
> > The click() method is less than bomb-proof. For example, in the site above
> > it does not work. 
> 
> click generates gecko events while nsCoreUtils emulates user input.

That is what we are trying to do as well.

> So if
> click() is not enough for you then at least you can skip accessible
> boundaries computation.

You mean if it *is* enough? It isn't.

> Anyway the logic you do is likely wrong because I
> doubt that nsIDOMWindowUtils works with device pixels.

I tested this. at different zoom levels etc. And I was inspired by similar code in browser.js, so I am assuming it works.
(In reply to Eitan Isaacson [:eeejay] from comment #14)

> > So if
> > click() is not enough for you then at least you can skip accessible
> > boundaries computation.
> 
> You mean if it *is* enough? It isn't.

I meant if it *is not* enough then

> > Anyway the logic you do is likely wrong because I
> > doubt that nsIDOMWindowUtils works with device pixels.
> 
> I tested this. at different zoom levels etc. And I was inspired by similar
> code in browser.js, so I am assuming it works.

sometimes it's not easy to catch a difference, but nsIDOMWindowUtils methods as I assumed work in CSS pixels https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsIDOMWindowUtils#sendMouseEventToWindow%28%29
Comment on attachment 626160 [details] [diff] [review]
Fallback for items that don't have accessible actions.

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

::: accessible/src/jsat/VirtualCursorController.jsm
@@ +140,4 @@
>        acc.doAction(0);
> +    } else {
> +      // XXX Some mobile widget sets do not expose actions properly
> +      // (via ARIA roles, etc.), so we need to generate a click.

Since we are iterating, how about adding a comment like:
// XXX TODO simplify this block ... blah blah

I'd be okay with you landing as long as we don't lose any nice ideas Alexander has (that will still work for this case).
http://hg.mozilla.org/integration/mozilla-inbound/rev/995cc950c60e
Assignee: nobody → eitan
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: