Add support for nsIAccessibleEvent::OBJECT_ATTRIBUTE_CHANGED

VERIFIED FIXED

Status

()

defect
P1
normal
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: MarcoZ, Assigned: davidb)

Tracking

(Blocks 2 bugs, {access, dev-doc-complete, verified1.9.2})

Trunk
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(status1.9.2 beta1-fixed)

Details

(Whiteboard: in-testsuite+)

Attachments

(1 attachment, 2 obsolete attachments)

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.
David, would it be possible for you to get to this soon'ish?
Priority: -- → P1
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.
Assignee: nobody → bolterbugz
BTW, it is "IA2_EVENT_OBJECT_ATTRIBUTE_CHANGED" (and yeah we don't fire it ever).
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?
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.
Posted patch exploration (obsolete) — Splinter Review
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

10 years ago
(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.
(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.
Tryserver is down ATM.
Have done a build on my machine and will send the link to Jamie in a few minutes via e-mail.
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/dbolter@mozilla.com-EVENT_OBJECT_ATTRIBUTE_CHANGED/
Actually I needed to refresh my browser tab more often, no excuse for that collision.
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.
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?
Posted patch patch 1 (obsolete) — Splinter Review
Yep very easy. Note this patch doesn't address html5 drag-drop eventing.
Attachment #394565 - Attachment is obsolete: true
Attachment #394807 - Flags: review?(surkov.alexander)
Attachment #394807 - Flags: review?(marco.zehe)
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.
Attachment #394807 - Flags: review?(marco.zehe) → review-
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"));
Attachment #394807 - Flags: review?(surkov.alexander) → review+
r=me with Marco's comment fixed of course.
Posted patch patch2Splinter Review
Fixes missing test, and nits. Thanks!
Attachment #394807 - Attachment is obsolete: true
Attachment #395300 - Flags: review?(marco.zehe)
Comment on attachment 395300 [details] [diff] [review]
patch2

This is correct, thanks!
Attachment #395300 - Flags: review?(marco.zehe) → review+
Pushed: http://hg.mozilla.org/mozilla-central/rev/873809032158
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
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?
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.
See comment #22 and comment #23 for build ID and confirmation.

Thanks Jamie!
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
Comment on attachment 395300 [details] [diff] [review]
patch2

Needed for ARIA 1.0 completeness and good screen reader support of ARIA drag and drop.
Attachment #395300 - Flags: approval1.9.2?

Updated

10 years ago
Attachment #395300 - Flags: approval1.9.2? → approval1.9.2+
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)
Keywords: verified1.9.2
Added a note to this constant's description that it wasn't sent prior to 1.9.2.
You need to log in before you can comment on or make changes to this bug.