246062 - No accessible name for tree views and listboxes

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect, P1)

Description

[Aaron Leventhal]

Tree views often have a name, which is not getting exposed in the accessible
name for the tree view accessible via nsIAccessible::GetName().

Examples:
Bookmarks -> File bookmark             Name = "Destination:"
View -> Char Endcoding -> Customize    Names = "Available Character Encodings:",
"Active Character Encdoings:"
Edit -> Preferences                    Name = "Category"

Comment 1

[Aaron Leventhal]

Attachment: Change GetXULName to be more flexible about where label is, and handle labels for the binding parent of a widget (fixes bookmarks-tree case)

Comment 2

[Aaron Leventhal]

*** Bug 184499 has been marked as a duplicate of this bug. ***

Comment 3

[[e-mail]]

Comment on attachment 150382 [details] [diff] [review]
Change GetXULName to be more flexible about where label is, and handle labels for the binding parent of a widget (fixes bookmarks-tree case)

Just a general question really... are labels more accessible than header rows
(if and when header rows are accessible, of course)?

>+  nsCOMPtr<nsIContent> content(do_QueryInterface(mDOMNode));
>+  NS_ASSERTION(content, "No content for accessible DOM node!");
These lines were put in a strange-looking place.

>+          if (!controlID.IsEmpty()) {
>+            labelSearchStart = bindingParent->GetParent();
>             }
Odd indentation...
>+        }
>+      }
>+      else {
>+        // We have an id
>+        labelSearchStart = content->GetParent();
>+      }
Don't you need to test for labelSearchStart before continuing?

>+        PRUint32 length = 0;
>+        labelList->GetLength(&length);
>+        for (PRUint32 index = 0; index < length; ++index) {
>+          nsCOMPtr<nsIDOMNode> child;
>+          if (NS_SUCCEEDED(labelList->Item(index, getter_AddRefs(child) ))) {
>+            AppendLabelText(child, label);
>           }
>         }
If you really want all the labels, then it would be slightly more efficient to
loop through until the returned child is null. As it is, GetLength does its own
loop through all the elements.

>+  <separator/> <separator/>
???

>-    <label value="&fileTypes.label;"/>
>+    <label value="&fileTypes.label;" control="appList"/>
>     <vbox flex="1">
Could you possibly put the label inside the vbox where it would incidentally be
closer to its tree.
>       <hbox flex="1">
>         <tree id="appList" flex="1" seltype="single" hidecolumnpicker="true"

Comment 4

[Aaron Leventhal]

(In reply to comment #3)
> (From update of attachment 150382 [details] [diff] [review])
> Just a general question really... are labels more accessible than header rows
> (if and when header rows are accessible, of course)?
Header rows are already exposed as a "column header" object, but labels are
getting missed. This means that for a lot of lists or tree views, as you tab to
it, you don't know what list or tree view you're on.

Comment 5

[Aaron Leventhal]

(In reply to comment #3)
> >+  nsCOMPtr<nsIContent> content(do_QueryInterface(mDOMNode));
> >+  NS_ASSERTION(content, "No content for accessible DOM node!");
> These lines were put in a strange-looking place.
I moved it to the top of the method, is that better?

> >+        }
> >+      }
> >+      else {
> >+        // We have an id
> >+        labelSearchStart = content->GetParent();
> >+      }
> Don't you need to test for labelSearchStart before continuing?
No, these lines within the for loop actually test that:
        nsCOMPtr<nsIDOMXULElement> xulElement(do_QueryInterface(labelSearchStart));
        NS_ASSERTION(xulElement, "No xulElement for parent DOM Node!");
        if (!xulElement) {
          break;
I should probably get rid of the assertion though, now that we're going up more
than one parent.

> 
> >+  <separator/> <separator/>
> ???
Odd I don't know how that got in. Thanks for catching it.

> 
> >-    <label value="&fileTypes.label;"/>
> >+    <label value="&fileTypes.label;" control="appList"/>
> >     <vbox flex="1">
> Could you possibly put the label inside the vbox where it would incidentally be
> closer to its tree.
> >       <hbox flex="1">
> >         <tree id="appList" flex="1" seltype="single" hidecolumnpicker="true"
> 

Comment 6

[Aaron Leventhal]

Attachment: Heeding Neil's comments

Comment 7

[[e-mail]]

Ok, so except in the case of Preferences, which was only changed to a label
because Jan couldn't be bothered to fix it as a column header, I see what the
difference between a label and a header is.

Comment 8

[[e-mail]]

Comment on attachment 150813 [details] [diff] [review]
Heeding Neil's comments

Nit: when clicking on a controlled label it focuses its control: in the case of
the add bookmark dialog the focus goes nowhere because the control you focus is
just a box; ideally you should focus the anonymous tree somehow.

I noticed that the customize character encoding dialog is very screwy :-/

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 150813 [details] [diff] [review]
Heeding Neil's comments

sr=jst

Comment 10

[OstGote!]

Note: The patch contains a typo which caused bug 248453.
http://bugzilla.mozilla.org/attachment.cgi?id=150813&action=diff#mozilla/xpfe/components/prefwindow/resources/content/pref-winhooks.xul_sec2

Comment 11

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1527381