Last Comment Bug 563862 - Expand support for nsIAccessibleEvent::OBJECT_ATTRIBUTE_CHANGED
: Expand support for nsIAccessibleEvent::OBJECT_ATTRIBUTE_CHANGED
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla15
Assigned To: David Bolter [:davidb] ***PTO until 29th***
:
Mentors:
https://bugzilla.instantbird.org/show...
Depends on:
Blocks: eventa11y Instantbird
  Show dependency treegraph
 
Reported: 2010-05-04 21:27 PDT by Marco Zehe (:MarcoZ)
Modified: 2012-04-25 07:14 PDT (History)
5 users (show)
dbolter: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (5.26 KB, patch)
2012-04-19 06:58 PDT, David Bolter [:davidb] ***PTO until 29th***
mzehe: review+
surkov.alexander: review+
Details | Diff | Splinter Review
patch to land (7.14 KB, patch)
2012-04-23 10:07 PDT, David Bolter [:davidb] ***PTO until 29th***
no flags Details | Diff | Splinter Review

Description Marco Zehe (:MarcoZ) 2010-05-04 21:27:05 PDT
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 2010-05-05 01:18:57 PDT
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) 2010-05-05 03:46:59 PDT
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] [:flo] (PTO until August 29th) 2010-05-05 05:08:17 PDT
(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] ***PTO until 29th*** 2010-05-05 07:19:09 PDT
http://mxr.mozilla.org/mozilla-central/ident?i=EVENT_OBJECT_ATTRIBUTE_CHANGED
Comment 5 David Bolter [:davidb] ***PTO until 29th*** 2010-05-05 07:24:57 PDT
(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 2010-05-06 02:59:02 PDT
(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] ***PTO until 29th*** 2010-05-06 04:31:43 PDT
I should probably take this one as I want to discuss it with the UAAG group.
Comment 8 alexander :surkov 2011-11-02 12:09:08 PDT
(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] ***PTO until 29th*** 2011-11-02 16:10:51 PDT
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] ***PTO until 29th*** 2012-04-19 06:58:03 PDT
Created attachment 616550 [details] [diff] [review]
patch
Comment 11 Marco Zehe (:MarcoZ) 2012-04-19 07:39:58 PDT
Comment on attachment 616550 [details] [diff] [review]
patch

r=me for the test part.
Comment 12 alexander :surkov 2012-04-23 00:42:50 PDT
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] ***PTO until 29th*** 2012-04-23 08:08:54 PDT
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] ***PTO until 29th*** 2012-04-23 10:07:56 PDT
Created attachment 617512 [details] [diff] [review]
patch to land
Comment 15 David Bolter [:davidb] ***PTO until 29th*** 2012-04-24 10:40:20 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/c2a4edbb377b

Note You need to log in before you can comment on or make changes to this bug.