Last Comment Bug 391829 - Add support for container-live-role to object attributes
: Add support for container-live-role to object attributes
Status: VERIFIED FIXED
orca:normal
: access, dev-doc-complete
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: David Bolter [:davidb]
:
: alexander :surkov
Mentors:
Depends on: 499140
Blocks: aria fox3access
  Show dependency treegraph
 
Reported: 2007-08-11 07:10 PDT by Aaron Leventhal
Modified: 2011-02-17 06:52 PST (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch 1 (with tests) (4.19 KB, patch)
2009-06-12 13:15 PDT, David Bolter [:davidb]
no flags Details | Diff | Splinter Review
patch 2 (4.08 KB, patch)
2009-06-12 19:06 PDT, David Bolter [:davidb]
mzehe: review+
surkov.alexander: review+
neil: superreview+
Details | Diff | Splinter Review

Description Aaron Leventhal 2007-08-11 07:10:32 PDT
Firefox 3 already has a new set of object attributes designed to make live regions easier for ATs to deal with. See http://developer.mozilla.org/en/docs/AJAX:WAI_ARIA_Live_Regions/API_Support for information on the current support.

One piece of information that was overlooked, however, is providing the live region role, when a container role is used that is live.

There should be an object attribute called
container-live-role, which is set to the name of the first role in the ancestor elements which automatically has some kind of special live behavior.

E.g. "alert", "log", "marquee", "statusbar", "timer", "tooltip"
Comment 1 Aaron Leventhal 2007-08-11 07:18:37 PDT
I think we should add table (from HTML) and "grid" to that. Not sure how we'll say it's an HTML table.
Comment 2 Aaron Leventhal 2007-09-15 14:43:57 PDT
Scott, could you give your opinion on what would be useful, if anything?
Comment 3 Scott Haeger 2007-09-17 06:06:35 PDT
Any additional information of this type would be useful.  I believe this will help me find a solution to the problems I am having with alerts/tooltips.  I would focus on alerts/tooltips, tables, statusbar, and perhaps marquee roles.  I am not familiar with the other type of roles you mentioned and I don't believe they are currently supported in Orca.  

Comment 4 Aaron Leventhal 2007-09-21 09:41:54 PDT
Scott, "log" is used for a chat log, error log, game log -- anything where content is continually added as time moves forward.

A timer, well, is for a timer. Usually you wouldn't want to hear it unless the updates were infrequent. If it's counting down toward zero you might want to hear it. Not sure what the logic should be -- and we don't have any examples of it yet.

Comment 5 David Bolter [:davidb] 2009-06-12 13:05:31 PDT
Taking...
Comment 6 David Bolter [:davidb] 2009-06-12 13:15:32 PDT
Created attachment 383009 [details] [diff] [review]
patch 1 (with tests)

This is deemed a worthwhile attribute; we should expose it. I'll tell the other browser a11y folk.
Comment 7 David Bolter [:davidb] 2009-06-12 13:25:10 PDT
Comment on attachment 383009 [details] [diff] [review]
patch 1 (with tests)

Note: please check the string conversion stuff.
Comment 8 alexander :surkov 2009-06-12 17:42:32 PDT
Comment on attachment 383009 [details] [diff] [review]
patch 1 (with tests)

>diff --git a/accessible/src/base/nsAccessibilityAtomList.h b/accessible/src/base/nsAccessibilityAtomList.h
>--- a/accessible/src/base/nsAccessibilityAtomList.h
>+++ b/accessible/src/base/nsAccessibilityAtomList.h
>@@ -254,14 +254,15 @@ ACCESSIBILITY_ATOM(aria_valuetext, "aria
> // of an HTML button from the button frame
> ACCESSIBILITY_ATOM(defaultLabel, "defaultLabel")
> 
> // Object attributes
> ACCESSIBILITY_ATOM(tableCellIndex, "table-cell-index")
> ACCESSIBILITY_ATOM(containerAtomic, "container-atomic")
> ACCESSIBILITY_ATOM(containerBusy, "container-busy")
> ACCESSIBILITY_ATOM(containerLive, "container-live")
>+ACCESSIBILITY_ATOM(containerLiveRole, "container-live-role")

"live-role" confuse me, could we think more fortunate name for better reflection of meaning?

>+
>+    // For default live containers, expose the container-live-role attribute
>+    if (mRoleMapEntry->liveAttRule != eNoLiveAttr) {
>+      nsAccUtils::SetAccAttr(attributes,
>+                             nsAccessibilityAtoms::containerLiveRole,
>+                             NS_ConvertUTF8toUTF16(nsDependentCString(
>+                                                   mRoleMapEntry->roleString)));
>+    }

logic suggests "container" word is excess here

btw, you didn't expose this attribute inside of "live role", though you should.
Comment 9 David Bolter [:davidb] 2009-06-12 17:46:50 PDT
I guess the name makes more sense if we set this obj attr on the dom children of such a node with such a role.
Comment 10 David Bolter [:davidb] 2009-06-12 17:55:36 PDT
Jamie, Joanmarie, before we burn more cycles on this... do you think/agree it is valuable info?
Comment 11 David Bolter [:davidb] 2009-06-12 19:06:32 PDT
Created attachment 383056 [details] [diff] [review]
patch 2

This patch illustrates using container-live-role on children of container nodes that have a role that has a live value by default. I think the name makes more sense in this context, but I'm open to suggestions.

So this should improve things as Scott suggests. Also one of you screen reader froods mentioned it would probably be useful, but I can't remember who :)

Anyways, we need to make a good case for committing this; otherwise it should be closed as a WONTFIX. It allows the checking of the object attributes to find out what sort of live-region container the added child node is in; which should in theory allow better heuristics for deciding what to report to the user, as well as additional info about the context.

Another option is to commit the patch, and then in a year or so, if nobody is using it, pull it.
Comment 12 Marco Zehe (:MarcoZ) 2009-06-13 00:30:10 PDT
Comment on attachment 383056 [details] [diff] [review]
patch 2

This looks correct. However I agree with Surkov that the attribute name is chosen a bit...well...confusingly, because it's actually the live-container's role we're exposing, so the more intuitive name would be live-container-role, but since we already have a container-live attribute that's established, I guess we'll have to live with it for now.
Comment 13 alexander :surkov 2009-06-13 03:34:23 PDT
I really not sure how it can help. AT could easy can go up to get live region to grab xml-roles attribute (if we won't consider aria-owns usage). Nevertheless I don't know if ARIA role is helpful here. Author can place aria-live on native markup element (that is exposed as alert or tooltip accessible for example) but we won't expose "container-live-role" attribute since ARIA role wasn't used.
Comment 14 David Bolter [:davidb] 2009-06-15 08:06:36 PDT
Okay it is good to be cautious. Before we closet his I'll attempt to defend it:

1. We don't do any extra work really, we are just adding another object attribute. The logic that decides when we add it is already there.
2. With this patch, an AT can immediately check to see if the live region addition came from one of the 'live-roles' on the containing node. In addition they can check which 'live-role' is on the containing node. I say 'live-role' to denote a role with an implicit live quality.
3. Having cases where we don't supply a this attribute doesn't mean we never should ;)

So as an example, if an AT gets an event for a live region addition and sees a container-live-role of 'timer', it might choose not to expose it. If 'status' it might choose to say 'status' + ...

Thoughts?
Comment 15 David Bolter [:davidb] 2009-06-15 08:07:00 PDT
Heh, where closet his == close this :)
Comment 16 David Bolter [:davidb] 2009-06-15 10:57:41 PDT
OK I spoke with Will and he saw some use cases where this would be handy, but would like to see specific existing cases in the wild that this bug fixes.

I spoke with Aaron and he still feels this is very useful information, especially for live regions with lots of changes where performance comes into play. The need to walk up the tree can be circumvented in many cases, which is especially useful for out-of-process applications.

Still, I think we need to hear from Mick and Jamie.

Also, I've thought of another example.
http://www.w3.org/TR/wai-aria/#marquee

Marquee will constantly update, checking the container-live-role when you get the event accessible will immediately give you "marquee" which probably means don't speak it. Handy?
Comment 17 Marco Zehe (:MarcoZ) 2009-06-15 12:37:42 PDT
Mick and Jamie, can you comment? Thanks!
Comment 18 James Teh [:Jamie] 2009-06-15 16:47:11 PDT
I'll let Mick comment separately, as he may have some slightly differing viewpoints on this.

To be brutally honest, I'm still a little warey/cynical of live regions. I believe that they are necessary, but I am keenly aware that, like everything else, they will probably be abused and may end up being more of a hinderance than a help.

Marquee is a prime example. If we are going to end up ignoring marquee regions most of the time, why bother making them an implicit live region in the first place? We now have a situation where the AT chooses to ignore the author's suggestion anyway, which seems pretty counter-productive to me. Not only does this mean that there will be widely differing behaviour between ATs, but it also encourages irresponsible authoring; authors don't need to think about the consequences of, for example, using a marquee role. This is one reason I am not such a fan of using heuristics like this.

That said, the timer example (reading the last few seconds of a countdown) does make some sense.

My rambling aside, this is probably something we're going to have to deal with. If we are going to use heuristics to determine how we react to live regions, having this information exposed on the region would certainly be easier than crawling up the ancestors, especially if Gecko can expose this without any additional work on its side.

Note that for us, out-of-process has no significance here. We will need to handle live regions in-process. (We currently handle them out-of-process, but our current implementation really sucks.) Primarily, this is because instantiating an object out-of-process for every event that might affect a live region is far too slow. There are no specific events for live regions, which means we need to handle all show and hide events among others, which happen a hell of a lot.
Second, IA2 text inserted/text removed events are fatally flawed for out-of-process clients because out-of-process win events are asynchronous and cannot pass parameters. All of this said, crawling up the ancestors is tedious and there will obviously be a very slight performance hit even in-process.
Comment 19 alexander :surkov 2009-06-15 19:08:58 PDT
David, probably it's worth to start wiki page for this attribute before its implementation. I think it's worth to summarize when and how to expose this attribute and its use cases. And we should start kind of investigations from use cases :) I agree it won't hit us to expose it but we need to be sure we need it. Also we need to bring it up to IA2 group to ensure it's not firefox only feature.
Comment 20 David Bolter [:davidb] 2009-06-16 07:20:56 PDT
I really don't want to spend a lot of time on this, considering all the better places we can spend time. My proposal is:

1. Drop it and reopen if people ask for it or,
2. Land it and I'll go to the UIA-TF group to discuss. If no other browsers think it is a good idea we can pull it quick.
Comment 21 Marco Zehe (:MarcoZ) 2009-06-16 07:30:34 PDT
I'd vote for option 2, since we have several statements now that say it'd be useful.
Comment 22 David Bolter [:davidb] 2009-06-16 07:33:07 PDT
Comment on attachment 383056 [details] [diff] [review]
patch 2

Aaron convinced us this is worth landing.
Comment 23 alexander :surkov 2009-06-16 18:45:26 PDT
Comment on attachment 383056 [details] [diff] [review]
patch 2


>+          // For default live containers, expose the container-live-role attribute
>+          nsAccUtils::SetAccAttr(aAttributes,
>+                                 nsAccessibilityAtoms::containerLiveRole,
>+                                 NS_ConvertUTF8toUTF16(nsDependentCString(
>+                                                         role->roleString)));

possibly it's best to use NS_ConvertASCIItoUTF16 like
NS_ConvertASCIItoUTF16 roleString = role->roleString; 

but it's safely to get Neil's sr.

> <body>
>   <a target="_blank"
>      href="https://bugzilla.mozilla.org/show_bug.cgi?id=475006"
>      title="Extend nsARIAMap to capture ARIA attribute characteristics">
>-    Mozilla Bug 475006
>+    Mozilla Bug 475006 (and 391829)

nit: add new html:a instead

>   </a>
Comment 24 neil@parkwaycc.co.uk 2009-06-17 06:43:20 PDT
Comment on attachment 383056 [details] [diff] [review]
patch 2

>+                                 NS_ConvertUTF8toUTF16(nsDependentCString(
>+                                                         role->roleString)));
Yes, I'd go with NS_ConvertASCIItoUTF16 [sadly the internal API doesn't support
NS_ConvertASCIItoUTF16(role->roleString) although the external API does!]
Comment 25 David Bolter [:davidb] 2009-06-17 07:29:00 PDT
Thanks guys.

Addressed Alexander's and Neil's comments and pushed as changeset:
http://hg.mozilla.org/mozilla-central/rev/3d1618d1f7a4
Comment 26 Marco Zehe (:MarcoZ) 2009-06-18 07:39:35 PDT
Verified fixed in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090618 Minefield/3.6a1pre (.NET CLR 3.5.30729)
Comment 27 Eric Shepherd [:sheppy] 2011-02-17 06:52:35 PST
This appears to already be documented here:

https://developer.mozilla.org/en/Accessibility/AT-APIs/Gecko/Attrs#Live_region_attribues

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