290344 - Implement DHTML accessibility description relations

Status: RESOLVED FIXED

(Core :: Disability Access APIs, enhancement)

Description

[Aaron Leventhal]

DHTML needs a way of relating a description to an element. For example, imagine
an installer dialog with buttons that say "Compact", "Full" and "Custom". To the
right of each button is a description of what the button really does.

If we have a describedby relationship that points to the ID of another element,
we could expose the description in the MSAA accessible description, and also via
ATK relationships.

Comment 1

[Aaron Leventhal]

Bill, can we have 2 relationships added to ATK?

+  enum { RELATION_DESCRIPTION_FOR = 3U };        // ATK_RELATION_LABEL_FOR
+  enum { RELATION_DESCRIBED_BY = 4U };      // ATK_RELATION_LABELLED_BY

It is really separate from the label for an control. It is an additional
description, and right now the AT has no good heuristic to associate them.

Comment 2

[Bill Haneman]

Aaron: I don't think the semantic differences between DESCRIPTION and LABEL
warrant a new relation.

Comment 3

[Bill Haneman]

BY the way, if you feel this descriptive info should not be exposed via 'LABEL',
then I'd advise exposing it in your implementation as the text from
"atk_object_get_description()".  If it's appropriate that the description be an
AtkObject in its own right, then I believe LABEL_FOR/BY is the correct relation.  

There is no guarantee that LABEL_FOR objects implement AtkText, or even
AtkComponent, so no reason that I can see why LABEL_FOR isn't appropriate.

Comment 4

[Aaron Leventhal]

Bill, I agree we can start out using
atk_object_get_description()

However, I think a label is different from a desciption. The label is always
spoken by a screen reader. The description is only spoken when the verbosity
setting is set to speak descriptions.

Imagine this example:

First choice: ____________              We will try to get everyone their first 
                                        choice, please see the __guidelines__ 
                                        for more information on how we decide
                                        which members receive this.
Second choice: ___________              Your backup choice.
Third choice: ____________              Only in extreme circumstances will
                                        we need to make use of anyone's third
                                        choice.   

Comment 5

[Bill Haneman]

I agree that labels and descriptions differ, but I don't see any utility in a
DESCRIPTION relation; the description in this case should just be a text string
returned by atk_object_get_description(), i.e. not a separate AtkObject.

We've discussed allowing markup in name/description strings, which may be useful
in the future, but is only tangentially related to this request, I believe.

Comment 6

[Aaron Leventhal]

Attachment: 1) Implement describedby and labeledby which override normal description/name, 2) Streamline GetName/GetXULName/GetHTMLName usage, 3) allow <label for> to apply to DHTML controls

From http://www.w3.org/TR/REC-html40/interact/forms.html#h-17.9.1
> for = idref [CS]
>    This attribute explicitly associates the label being defined with another 

> control. When present, the value of this attribute must be the same as the
value 
> of the id attribute of some other control in the same document. When absent,
the 
> label being defined is associated with the element's contents. 
With this patch I am counting DHTML controls as "controls"

Comment 7

[timeless]

Comment on attachment 181203 [details] [diff] [review]
1) Implement describedby and labeledby which override normal description/name, 2) Streamline GetName/GetXULName/GetHTMLName usage, 3) allow <label for> to apply to DHTML controls

>Index: accessible/src/base/nsAccessible.cpp
>@@ -130,55 +131,57 @@ nsAccessible::~nsAccessible()
>+  // 4. its title attribute -already equals to its- accName nsAutoString name; 
_is already equal to its_

>+  nsCOMPtr<nsIContent> content(do_QueryInterface(mDOMNode));
>+  if (!content) {
>+    return NS_ERROR_FAILURE;  // Node shut down
>+  }
>@@ -1224,10 +1226,42 @@ nsIContent* nsAccessible::GetHTMLLabelCo
>   }
> 
>   return nsnull;
> }
> 
>+nsresult nsAccessible::GetTextFromRelationID(nsIAtom *aIDAttrib, nsString &aName)
>+  nsCOMPtr<nsIContent> content(do_QueryInterface(mDOMNode));
>+  NS_ASSERTION(content, "Called from shutdown accessible");
you'll crash shortly.
>+  if (NS_CONTENT_ATTR_HAS_VALUE !=
>+      content->GetAttr(kNameSpaceID_StatesWAI_Unofficial, aIDAttrib, id)) {

Comment 8

[Aaron Leventhal]

I'll fix the first thing, but on the crash comment - the crash won't happen
because we check for this condition in all of the callers.

Comment 9

[Aaron Leventhal]

Comment on attachment 181203 [details] [diff] [review]
1) Implement describedby and labeledby which override normal description/name, 2) Streamline GetName/GetXULName/GetHTMLName usage, 3) allow <label for> to apply to DHTML controls

No response from jst after weeks.

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 181203 [details] [diff] [review]
1) Implement describedby and labeledby which override normal description/name, 2) Streamline GetName/GetXULName/GetHTMLName usage, 3) allow <label for> to apply to DHTML controls

>Index: accessible/src/base/nsDocAccessible.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/accessible/src/base/nsDocAccessible.cpp,v
>retrieving revision 1.58
>diff -p -u -5 -r1.58 nsDocAccessible.cpp
>--- accessible/src/base/nsDocAccessible.cpp	15 Apr 2005 21:22:25 -0000	1.58
>+++ accessible/src/base/nsDocAccessible.cpp	19 Apr 2005 21:26:19 -0000
>@@ -126,11 +126,20 @@ NS_INTERFACE_MAP_END_INHERITING(nsBlockA
> NS_IMPL_ADDREF_INHERITED(nsDocAccessible, nsBlockAccessible)
> NS_IMPL_RELEASE_INHERITED(nsDocAccessible, nsBlockAccessible)
> 
> NS_IMETHODIMP nsDocAccessible::GetName(nsAString& aName) 
> { 
>-  return GetTitle(aName);
>+  nsresult rv = NS_OK;
>+  aName.Truncate();
>+  if (mRoleMapEntry) {
>+    nsAccessible::GetName(aName);

Doesn't the above line really want to start with "rv = "?

>Index: accessible/src/html/nsHTMLAreaAccessible.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/accessible/src/html/nsHTMLAreaAccessible.cpp,v
>retrieving revision 1.24
>diff -p -u -5 -r1.24 nsHTMLAreaAccessible.cpp
>--- accessible/src/html/nsHTMLAreaAccessible.cpp	20 Aug 2004 20:34:28 -0000	1.24
>+++ accessible/src/html/nsHTMLAreaAccessible.cpp	19 Apr 2005 21:26:22 -0000
>@@ -54,18 +54,24 @@ nsLinkableAccessible(aDomNode, aShell)
>   Init(); // Make sure we're in cache
>   mParent = aParent;
> }
> 
> /* wstring getName (); */
>-NS_IMETHODIMP nsHTMLAreaAccessible::GetName(nsAString & _retval)
>+NS_IMETHODIMP nsHTMLAreaAccessible::GetName(nsAString & aName)
> {
>+  if (mRoleMapEntry) {
>+    nsAccessible::GetName(aName);
>+    if (!aName.IsEmpty()) {
>+      return NS_OK;
>+    }
>+  }
>   nsCOMPtr<nsIDOMElement> elt(do_QueryInterface(mDOMNode));
>   if (elt) {
>     nsAutoString hrefString;
>-    elt->GetAttribute(NS_LITERAL_STRING("title"), _retval);
>-    if (_retval.IsEmpty())
>-      GetValue(_retval);
>+    elt->GetAttribute(NS_LITERAL_STRING("title"), aName);
>+    if (aName.IsEmpty())
>+      GetValue(aName);

Same question here.

>Index: accessible/src/html/nsHTMLFormControlAccessible.cpp
>===================================================================
> [...]
>
>+    if (mRoleMapEntry) {
>+      // Use HTML label or DHTML accessibility's labeledby attribute for name
>+      GetHTMLName(name, PR_FALSE);
>+    }
>+    if (name.IsEmpty()) {
>+      // Use anonymous text child of button if nothing else works.
>+      // This is necessary for submit, reset and browse buttons.
>+      nsCOMPtr<nsIPresShell> shell(GetPresShell());
>+      NS_ENSURE_TRUE(shell, NS_ERROR_FAILURE);
>+      nsCOMPtr<nsISupportsArray> anonymousElements;
>+      shell->GetAnonymousContentFor(content, getter_AddRefs(anonymousElements));

Would some error-checking be useful here?  That is, if the above getter fails
for some reason and returns null, won't the next line crash?

>+      nsCOMPtr<nsIDOMNode> domNode(do_QueryElementAt(anonymousElements, 0));

sr=dmose

Comment 11

[Aaron Leventhal]

(In reply to comment #10)
First two items: return values fixed
Last item: do_QueryElementAt() is conveniently null safe

Comment 12

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 181203 [details] [diff] [review]
1) Implement describedby and labeledby which override normal description/name, 2) Streamline GetName/GetXULName/GetHTMLName usage, 3) allow <label for> to apply to DHTML controls

a=shaver

Comment 13

[Aaron Leventhal]

Checking in src/base/nsAccessibilityAtomList.h;
/cvsroot/mozilla/accessible/src/base/nsAccessibilityAtomList.h,v  <-- 
nsAccessibilityAtomList.h
new revision: 1.20; previous revision: 1.19
done
Checking in src/base/nsAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsAccessible.cpp,v  <--  nsAccessible.cpp
new revision: 1.149; previous revision: 1.148
done
Checking in src/base/nsAccessible.h;
/cvsroot/mozilla/accessible/src/base/nsAccessible.h,v  <--  nsAccessible.h
new revision: 1.63; previous revision: 1.62
done
Checking in src/base/nsDocAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsDocAccessible.cpp,v  <--  nsDocAccessible.cpp
new revision: 1.61; previous revision: 1.60
done
Checking in src/base/nsFormControlAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsFormControlAccessible.cpp,v  <-- 
nsFormControlAccessible.cpp
new revision: 1.11; previous revision: 1.10
done
Checking in src/base/nsFormControlAccessible.h;
/cvsroot/mozilla/accessible/src/base/nsFormControlAccessible.h,v  <-- 
nsFormControlAccessible.h
new revision: 1.8; previous revision: 1.7
done
Checking in src/base/nsOuterDocAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsOuterDocAccessible.cpp,v  <-- 
nsOuterDocAccessible.cpp
new revision: 1.22; previous revision: 1.21
done
Checking in src/base/nsRootAccessible.cpp;
/cvsroot/mozilla/accessible/src/base/nsRootAccessible.cpp,v  <-- 
nsRootAccessible.cpp
new revision: 1.118; previous revision: 1.117
done
Checking in src/html/nsHTMLAreaAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLAreaAccessible.cpp,v  <-- 
nsHTMLAreaAccessible.cpp
new revision: 1.25; previous revision: 1.24
done
Checking in src/html/nsHTMLFormControlAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLFormControlAccessible.cpp,v  <-- 
nsHTMLFormControlAccessible.cpp
new revision: 1.61; previous revision: 1.60
done
Checking in src/html/nsHTMLFormControlAccessible.h;
/cvsroot/mozilla/accessible/src/html/nsHTMLFormControlAccessible.h,v  <-- 
nsHTMLFormControlAccessible.h
new revision: 1.30; previous revision: 1.29
done
Checking in src/html/nsHTMLImageAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLImageAccessible.cpp,v  <-- 
nsHTMLImageAccessible.cpp
new revision: 1.27; previous revision: 1.26
done
Checking in src/html/nsHTMLTableAccessible.cpp;
/cvsroot/mozilla/accessible/src/html/nsHTMLTableAccessible.cpp,v  <-- 
nsHTMLTableAccessible.cpp
new revision: 1.26; previous revision: 1.25
done
Checking in src/msaa/nsAccessibleWrap.cpp;
/cvsroot/mozilla/accessible/src/msaa/nsAccessibleWrap.cpp,v  <-- 
nsAccessibleWrap.cpp
new revision: 1.28; previous revision: 1.27
done
Checking in src/xul/nsXULFormControlAccessible.cpp;
/cvsroot/mozilla/accessible/src/xul/nsXULFormControlAccessible.cpp,v  <-- 
nsXULFormControlAccessible.cpp
new revision: 1.50; previous revision: 1.49
done
Checking in src/xul/nsXULFormControlAccessible.h;
/cvsroot/mozilla/accessible/src/xul/nsXULFormControlAccessible.h,v  <-- 
nsXULFormControlAccessible.h
new revision: 1.25; previous revision: 1.24
done
Checking in src/xul/nsXULTabAccessible.cpp;
/cvsroot/mozilla/accessible/src/xul/nsXULTabAccessible.cpp,v  <-- 
nsXULTabAccessible.cpp
new revision: 1.24; previous revision: 1.23
done
Checking in src/xul/nsXULTabAccessible.h;
/cvsroot/mozilla/accessible/src/xul/nsXULTabAccessible.h,v  <-- 
nsXULTabAccessible.h
new revision: 1.10; previous revision: 1.9
done