360297 - Hidden cells of XUL tree item exposed to nsIAccessible::GetName().

Status: RESOLVED WORKSFORME

(Core :: Disability Access APIs, defect)

Description

[Gary Campbell]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0

The text of hidden columns in a tree view are included in the text of the tree item content returned to accessibility aids.

Reproducible: Always

Steps to Reproduce:
1.1.  Open DOM Inspector.
2.  From File menu choose to inspect new window, choose the DOM Inspector window.
3.  Navigate in the tree view with a screen reader such as JAWS.  Note that although the Namespace URI column is hidden, the URI is spoken by JAWS.

or:
1.  Open DOM Inspector.
2.  From File menu choose to inspect new window, choose the DOM Inspector window.
3.  Open Access Explorer.
4.  Select the DOM Inspector window.
5.  Navigate the object tree to:
6.  DOM Inspector[application - Visible]
7.    chrome://inspector/content/viewers/dom/dom.xul[Error: unexpected VARIANT - Visible]
8.      chrome://inspector/content/viewers/dom/dom.xul[pane - Visible]
9.        NAMELESS[outline - Visible]
10.          window  window    http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul  1    winInspectorMain    [outline item - Visible]
11.  Note that although the Namespace URI column is hidden, the URI appears in the Name property.

Actual Results:  
Text from hidden columns is spoken.

Expected Results:  
Text from hidden columns should not be spoken.

In the file seamonkey/accessible/src/msaa/nsXULTreeAccessibleWrap.cpp in method nsXULTreeitemAccessibleWrap::GetName, the following code collects the text of the treeitem's cells:

  while (column) {
    nsAutoString colText;
    mTreeView->GetCellText(mRow, column, colText);
    aName += colText + NS_LITERAL_STRING("  ");
    nsCOMPtr<nsITreeColumn> nextColumn;
    column->GetNext(getter_AddRefs(nextColumn));
    column = nextColumn;
  }

It looks like one could easily make the code that appends the text conditional on whether or not the column is hidden.

Found on Windows XP Home Edition, SP2 with Firefox 2.0 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; 
rv:1.8.1) Gecko/20061010 Firefox/2.0 
and
rv:1.9a1) Gecko/20061106 Minefield/3.0a1) (downloaded from Latest Nightly link on 11/6/06).

I don't have MSVC++ and am probably just familiar enough with the Mozilla source tree to be dangerous, or I'd implement and test it myself.  I don't know for sure that this is the place in the tree to implement this, or if there are other places.  I didn't see something similar in the atk folder.

Thanks.

Comment 1

[Gary Campbell]

Attachment: Patches MSAA version of nsIAccessible::GetName to not include hidden tree columns. This was fixed for ATK in 356347.

This was fixed for ATK in 356347.  Patch generated from /mozilla-central trunk (from changeset 65da9468c88b).  I have tested it by running DOM Inspector in the build of Firefox including the fix and verifying that namespace column was not spoken by JAWS 9.0.2169u in the DOM viewer with a XUL element selected.

Steps:
Start Firefox.
Open DOM Inspector.
Choose File > Chrome Document, and pick the DOM Inspector window.
In the left pane navigate to a window or page element.  Note that the namespace URI for XUL is not spoken by JAWS.  (To verify that there really is a hidden namespace URI, click the column picker icon and choose to show the namespace URI.  Verify that the namespace URI is now spoken by JAWS.)

Comment 2

[Gary Campbell]

OOPS!  Instead of nsIAccessible::GetName in my last comment I meant nsXULTreeitemAccessibleWrap::GetName.

Comment 3

[alexander :surkov (:asurkov)]

Gary, please add mochitest for this

Comment 4

[Aaron Leventhal]

Comment on attachment 344436 [details] [diff] [review]
Patches MSAA version of nsIAccessible::GetName to not include hidden tree columns.  This was fixed for ATK in 356347.

Thanks. I will have Alex review.

Comment 5

[alexander :surkov (:asurkov)]

Code looks ok at the first glance but I really would like to get mochitest before review.

Comment 6

[alexander :surkov (:asurkov)]

Gary, any chance to create mochitest?

Comment 7

[Gary Campbell]

(In reply to comment #6)
> Gary, any chance to create mochitest?

I'm looking at it.  My available time just disappeared for a while.  Also, When I run the Mochitest a11y tests I get a bunch of fails, including stuff like:

17 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/a11y/accessible/test_bug368835.xul | Error thrown during test: uncaught exception: [Exception... "Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsISupports.QueryInterface]"  nsresult: "0x80004002 (NS_NOINTERFACE)"  location: "native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0"  data: no] - got 0, expected 1

and:

120 INFO Running chrome://mochikit/content/a11y/accessible/test_textboxes.html...
121 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/a11y/accessible/test_textboxes.html | Error thrown during test: Components is not defined - got 0, expected 1
123 INFO Running chrome://mochikit/content/a11y/accessible/test_textboxes.xul...

124 ERROR TEST-UNEXPECTED-FAIL | chrome://mochikit/content/a11y/accessible/test_textboxes.xul | Error thrown during test: Components is not defined - got 0, expected 1

I'm running a system built on XP without the Vista stuff.

.mozconfig:
# mozilla config for Firefox
. $topsrcdir/browser/config/mozconfig
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/../obj-@CONFIG_GUESS@
ac_add_options --disable-vista-sdk-requirements   # from page refed in missing wpcapi.h error, caused by not new SDK installed.
# The following came from Mozilla Source Code (CVS) - MDC or Configuring Build Options - MDC web pages, should be covered by the above "source" command.
#ac_add_options --enable-application=browser
#mk_add_options MOZ_CO_PROJECT=browser

# Try to enable extensions Venkman and Inspector
#ac_add_options --enable-extensions=default,inspector,venkman

Now I have to go or I'm gonna have a lot more free time!

Gary

Comment 8

[alexander :surkov (:asurkov)]

Comment on attachment 344436 [details] [diff] [review]
Patches MSAA version of nsIAccessible::GetName to not include hidden tree columns.  This was fixed for ATK in 356347.

I missed actually it's MSAA part only, so there is no reason possibly to put platform-depended mochitest. Asking review from Marco to be sure all is going right.

Comment 9

[alexander :surkov (:asurkov)]

Btw, some of our mochitest fail if you run them all together.

Comment 10

[Marco Zehe (:MarcoZ)]

I'm not sure I want to take this bug yet. The problem is that most tree tables do not have a proper keyboard UI yet to change the columns configuration. In Thunderbird, for example, once this bug is fixed, the tree table of folder names will no longer expose the number of messages and the size of the folder to MSAA. If people got used to this information and want to get it back by enabling these columns, they currently have no way of doing so via the keyboard. See bug 370437.

Comment 11

[Marco Zehe (:MarcoZ)]

Friendly ping. Gary, any idea on my last comment?

Comment 12

[Gary Campbell]

I think I can manipulate the column picker with the JAWS cursor in the DOM inspector, but I wouldn't presume to speak for people using other products or other screen readers.  Still, since I haven't recreated a Mozilla build environment since my disk crash on Newyears, when I'm using the Inspector I often think about trying to figure out how to add a binary extension or something to update that file!  (Say "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" ten times fast!)

Comment 13

[alexander :surkov (:asurkov)]

I believe the bug was fixed by some table reorganization works despite comment #10.

Marco, please file new one if there is anything we should address still (and of course if we don't have proper bug).

Closing the bug as worksforme.