563862 - Expand support for nsIAccessibleEvent::OBJECT_ATTRIBUTE_CHANGED

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Marco Zehe (:MarcoZ)]

This is currently only being fired for aria-grabbed and aria-dropeffect (see bug 510441). However, the Instantbird team could make good use of this with some custom aria attributes on their conversation tabs which indicate whether someone is typing a message, has stopped typing, or his available/away etc. status changes.
This is currently only being indicated by color changes on the tab background.

So we could:
a) Expand the firing of nsIAccessibleEvent::OBJECT_ATTRIBUTE_CHANGED to *any* obj attr change or
b) expand it to supporting it on aria attributes we don't know about, similarly to what we do for object attr creation in this case.

I'd prefer solution b), and since these custom/unknown aria attributes aren't used widely, don't expect a huge performance impact at all.

Thoughts?

Comment 1

[alexander :surkov (:asurkov)]

If I would choose between a and b, then I choose b. However this confuse me since not all aria attributes are mapped to object attributes iirc and it should be bogus assumption that unknown aria attributes are subject of object attribute.

Comment 2

[Marco Zehe (:MarcoZ)]

Alex, we do have code in place that maps any unknown ARIA attribute to an object attribute with the same name. So if I have something like aria-ibbuddystatus, that attribute gets converted into an object attribute because we have no other rule for it to be processed. This is something Aaron put in place a long time ago for easier extensibility. It was already in when I started.
David had looked yesterday and had found the code where we do this, perhaps he can post the mxr link?

Comment 3

[Florian Quèze [:florian]]

(In reply to comment #2)

> David had looked yesterday and had found the code where we do this, perhaps he
> can post the mxr link?

I think it's this code: http://mxr.mozilla.org/mozilla-central/source/accessible/src/base/nsAccessible.cpp#1437

Comment 4

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

http://mxr.mozilla.org/mozilla-central/ident?i=EVENT_OBJECT_ATTRIBUTE_CHANGED

Comment 5

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

(In reply to comment #1)
> If I would choose between a and b, then I choose b.

Agreed.

(In reply to comment #2)
> Alex, we do have code in place that maps any unknown ARIA attribute to an
> object attribute with the same name.

Yes.

Now, if we implement this are we in danger of losing x-browser interoperability? I think it is relatively safe and perhaps this is a good way to provide extensibility. Not sure.

Comment 6

[alexander :surkov (:asurkov)]

(In reply to comment #2)
> Alex, we do have code in place that maps any unknown ARIA attribute to an
> object attribute with the same name. So if I have something like
> aria-ibbuddystatus, that attribute gets converted into an object attribute
> because we have no other rule for it to be processed. This is something Aaron
> put in place a long time ago for easier extensibility. It was already in when I
> started.

Ok, thanks.

Comment 7

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

I should probably take this one as I want to discuss it with the UAAG group.

Comment 8

[alexander :surkov (:asurkov)]

(In reply to David Bolter [:davidb] from comment #7)
> I should probably take this one as I want to discuss it with the UAAG group.

any update on this?

Comment 9

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

I need to go through a table in the implementation spec. I already have a w3 bug to track this.

Comment 10

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

Attachment: patch

Comment 11

[Marco Zehe (:MarcoZ)]

Comment on attachment 616550 [details] [diff] [review]
patch

r=me for the test part.

Comment 12

[alexander :surkov (:asurkov)]

Comment on attachment 616550 [details] [diff] [review]
patch

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

::: accessible/src/base/nsDocAccessible.cpp
@@ +1149,5 @@
> +  // change event; at least until native API comes up with a more meaningful event.
> +  PRUint8 attrFlags = nsAccUtils::GetAttributeCharacteristics(aAttribute);
> +  if (!(attrFlags & ATTR_BYPASSOBJ))
> +    FireDelayedAccessibleEvent(nsIAccessibleEvent::EVENT_OBJECT_ATTRIBUTE_CHANGED,
> +                               aContent);

not in sync with GetAttributes logic (token attributes) but it may be tricky to do this since you don't have old value (for example, I think we should fire object change event when value is changed from defined to undefined). Otherwise it would be nice to keep the code shared.

::: accessible/tests/mochitest/events/test_aria_objattr.html
@@ +53,5 @@
>  
> +      gQueue.push(new updateAttribute("sortable", "aria-sort", "ascending"));
> +
> +      // For experimental ARIA extensions
> +      gQueue.push(new updateAttribute("custom", "aria-throbbing", "fast"));

maybe: unknown aria attribute should be mapped to object attribute

Comment 13

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

Thanks. Note we won't fire events unless the value truly changes. Also note we already expose custom aria attributes via our general 'strip the aria- prefix' logic. I've added a test locally to show this.

Comment 14

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

Attachment: patch to land

Comment 15

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/c2a4edbb377b

Comment 16

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/c2a4edbb377b