510441 - Add support for nsIAccessibleEvent::OBJECT_ATTRIBUTE_CHANGED

Status: VERIFIED FIXED

(Core :: Disability Access APIs, defect, P1)

Description

[Marco Zehe (:MarcoZ)]

Right now, we define the constant in the nsIAccessibleEvent interface, we also create a string map, we map it to the IA2_OBJECT_ATTRIBUTE_CHANGED event, but we never fire it when an object attribute changes.

Jamie found this while implementing support for Drag And Drop into NVDA. When we change the object attribute we don't fire the event to let AT know we have changed the object attr.

Comment 1

[Marco Zehe (:MarcoZ)]

David, would it be possible for you to get to this soon'ish?

Comment 2

[David Bolter [:davidb] (NeedInfo me for attention)]

I assume we are talking about aria-grabbed. As far as I can tell we never fire IA2_OBJECT_ATTRIBUTE_CHANGED for anything. I can add this... but want to be sure we add the right event.

Comment 3

[David Bolter [:davidb] (NeedInfo me for attention)]

BTW, it is "IA2_EVENT_OBJECT_ATTRIBUTE_CHANGED" (and yeah we don't fire it ever).

Comment 4

[David Bolter [:davidb] (NeedInfo me for attention)]

The simple fix is to add a condition to nsDocAccessible::ARIAAttributeChanged. I'm hunting for existing events... I see STATE_MOVEABLE.. ?

Jamie what do you want?

Comment 5

[David Bolter [:davidb] (NeedInfo me for attention)]

Sorry I'm rambling... gotta step out. I'm not saying IA2_EVENT_OBJECT_ATTRIBUTE_CHANGED is not an option but only want to use it if there isn't a good MSAA event.

Comment 6

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: exploration

Jamie, Marco, want to give this a try? Note it includes a mochitest so should work in theory. This patch uses the EVENT_OBJECT_STATE_CHANGE which should get mapped to IA2_* ... when you get it you can look at the attributes to see what changed (i.e. "grabbed").

Let me know...

Comment 7

[James Teh [:Jamie]]

(In reply to comment #6)
> Jamie, Marco, want to give this a try?
Any chance of a try build? I don't have a Mozilla dev environment set up yet.

> This patch uses the EVENT_OBJECT_STATE_CHANGE which should get
> mapped to IA2_*
I am confused. You're saying that EVENT_OBJECT_STATE_CHANGE (an MSAA event) should get mapped to IA2_EVENT_OBJECT_ATTRIBUTE_CHANGED (an IA2 event)? Do you mean we'll get both or just one?

Practically speaking, we map the drag and drop object attributes to NVDA states, so EVENT_OBJECT_STATE_CHANGE will actually have the desired effect for us. However, this is specific to our implementation and, imo, isn't correct from a principle standpoint. You want to map the event to an appropriate MSAA event if possible, but this is relatively pointless, as you can't access object attributes via MSAA. Thus, you'd get an MSAA event for a change which you can't detect anyway. It makes sense to use an IA2 event for something which is only found in IA2.

Comment 8

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #7)
> (In reply to comment #6)
> > Jamie, Marco, want to give this a try?
> Any chance of a try build? I don't have a Mozilla dev environment set up yet.
> 
> > This patch uses the EVENT_OBJECT_STATE_CHANGE which should get
> > mapped to IA2_*
> I am confused. You're saying that EVENT_OBJECT_STATE_CHANGE (an MSAA event)
> should get mapped to IA2_EVENT_OBJECT_ATTRIBUTE_CHANGED (an IA2 event)?

Sorry Jamie, I mis-typed. I mean't this patch uses our internal event EVENT_OBJECT_ATTRIBUTE_CHANGED which should get mapped to IA2_EVENT_OBJECT_ATTRIBUTE_CHANGED.

I'll post a link to a tryserver build.

Comment 9

[David Bolter [:davidb] (NeedInfo me for attention)]

Tryserver is down ATM.

Comment 10

[Marco Zehe (:MarcoZ)]

Have done a build on my machine and will send the link to Jamie in a few minutes via e-mail.

Comment 11

[David Bolter [:davidb] (NeedInfo me for attention)]

Now that's timing -- we just mid air collided. I got an email finally from tryserver... looks like we have a windows build here if you still need it:
https://build.mozilla.org/tryserver-builds/[e-mail]-EVENT_OBJECT_ATTRIBUTE_CHANGED/

Comment 12

[David Bolter [:davidb] (NeedInfo me for attention)]

Actually I needed to refresh my browser tab more often, no excuse for that collision.

Comment 13

[James Teh [:Jamie]]

Using the Tic-tac-toe example with this patch:
http://codetalks.org/source/widgets/draganddrop/draganddrop1.html
I get the appropriate event when aria-grabbed changes from "false" to "true" as a result of pressing space on one of the buttons to "drag" it. (Thanks!) However, I don't get an event on the table cell when aria-dropeffect changes from "copy" to "none" resultant to pressing space in the table cell to "drop". This could be an issue specific to this test case; I'm not sure.

Comment 14

[Marco Zehe (:MarcoZ)]

Jamie, this patch is currently only firing the event for aria-grabbed. Great this part is working!

David, with your logic, adding the same for aria-dropeffect should be easy, right?

Comment 15

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: patch 1

Yep very easy. Note this patch doesn't address html5 drag-drop eventing.

Comment 16

[Marco Zehe (:MarcoZ)]

Comment on attachment 394807 [details] [diff] [review]
patch 1

This test will always change the aria-grabbed attribute even when you want to test inside the aria-dropeffect region.

Comment 17

[alexander :surkov (:asurkov)]

Comment on attachment 394807 [details] [diff] [review]
patch 1


>diff --git a/accessible/tests/mochitest/test_events_draganddrop.html b/accessible/tests/mochitest/test_events_draganddrop.html

test_events_attrchanged.html ?

>+  <script type="application/javascript">
>+
>+

nit: excess new line

>+    /**
>+     * Do tests.
>+     */
>+    var gQueue = null;
>+
>+    // aria grabbed invoker
>+    function changeGrabbed(aNodeOrID, aGrabValue)
>+    {
>+      this.DOMNode = getNode(aNodeOrID);
>+
>+      this.invoke = function changeGrabbed_invoke() {
>+
>+        if (aGrabValue != undefined)
>+          this.DOMNode.setAttribute("aria-grabbed", aGrabValue);
>+
>+      }

nit: too many new lines, it is expected?

>+      // TODO: uncomment this test when 472142 is fixed.

nit: I would like todo(false, uncomment this test when 472142 is fixed.);

>+      //gQueue.push(new changeGrabbed(id, "undefined"));

Comment 18

[alexander :surkov (:asurkov)]

r=me with Marco's comment fixed of course.

Comment 19

[David Bolter [:davidb] (NeedInfo me for attention)]

Attachment: patch2

Fixes missing test, and nits. Thanks!

Comment 20

[Marco Zehe (:MarcoZ)]

Comment on attachment 395300 [details] [diff] [review]
patch2

This is correct, thanks!

Comment 21

[David Bolter [:davidb] (NeedInfo me for attention)]

Pushed: http://hg.mozilla.org/mozilla-central/rev/873809032158

Comment 22

[Marco Zehe (:MarcoZ)]

Jamie, Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20090821 Minefield/3.7a1pre (.NET CLR 3.5.30729) contains the fix. Can you confirm that this works on your DragAndDrop branch of NVDA?

Comment 23

[James Teh [:Jamie]]

Confirmed. Thanks! Note that the ariaDragAndDrop NVDA branch doesn't currently react when aria-dropeffect is cleared, but I can confirm internally that we do get the appropriate event.

Comment 24

[Marco Zehe (:MarcoZ)]

See comment #22 and comment #23 for build ID and confirmation.

Thanks Jamie!

Comment 25

[Marco Zehe (:MarcoZ)]

Comment on attachment 395300 [details] [diff] [review]
patch2

Needed for ARIA 1.0 completeness and good screen reader support of ARIA drag and drop.

Comment 26

[David Bolter [:davidb] (NeedInfo me for attention)]

landed on 1.9.2 http://hg.mozilla.org/releases/mozilla-1.9.2/rev/b210bc305de8

Comment 27

[Marco Zehe (:MarcoZ)]

Verified fixed in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2b1pre) Gecko/20090922 Namoroka/3.6b1pre (.NET CLR 3.5.30729)

Comment 28

[Eric Shepherd [:sheppy]]

Added a note to this constant's description that it wasn't sent prior to 1.9.2.