Closed
Bug 382247
Opened 18 years ago
Closed 18 years ago
JAWS says "submenu" for ARIA menuitems with a text node child
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: simon.bates, Assigned: aaronlev)
References
(Blocks 1 open bug)
Details
(Keywords: access)
Attachments
(3 files)
946 bytes,
application/xhtml+xml
|
Details | |
1.22 KB,
patch
|
aaronlev
:
review-
|
Details | Diff | Splinter Review |
1.26 KB,
patch
|
simon.bates
:
review+
damons
:
approval1.9+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3
JAWS says "submenu" when reading the names of menu items that have an MSAA ChildCount > 0. For elements marked up with ARIA role "menuitem" that have a text node child Firefox reports an MSAA ChildCount of 1: the text node.
Reproducible: Always
Steps to Reproduce:
1. open aria_menu_test.xhtml
2. put JAWS into virtual PC cursor off mode (insert-Z)
3. click on the "Go to menu" button
4. move through the items with any keypress
Actual Results:
JAWS says: "one submenu" and "two submenu"
(MS Inspect reports a ChildCount of 1: the text node containing the menuitem name)
Expected Results:
JAWS should not say "submenu" when reading the names of items "one" and "two".
Investigation:
nsAccessibleWrap::get_accChildCount() calls MustPrune() (nsAccessibleWrap.h) to determine if it should look for children. nsIAccessibleRole::ROLE_MENUITEM is not tested for in MustPrune() and I've experimented with adding a test for it. (In MustPrune() I found I had to test for nsIAccessibleRole::ROLE_MENUITEM with nsIAccessible::GetFinalRole() rather than nsIAccessible::GetRole(); nsIAccessible::GetRole() returns nsIAccessibleRole::ROLE_CELL for table cells with aria role of "menuitem".)
current behaviour for ARIA menus:
* items without haspopup ChildCount = 1 (text node, item name)
* items without haspopup JAWS says "submenu" = yes
* items with haspopup ChildCount = 1 (text node, item name)
* items with haspopup JAWS says "submenu" = yes
current behaviour for Firefox menubar menus:
* items without submenu ChildCount = 0
* items without submenu JAWS says "submenu" = no
* items with submenu ChildCount = 1 (a popup)
* items with submenu JAWS says "submenu" = yes
adding a GetFinalRole() == nsIAccessibleRole::ROLE_MENUITEM to MustPrune():
ARIA menus:
* items without haspopup ChildCount = 0
* items without haspopup JAWS says "submenu" = no
* items with haspopup ChildCount = 0
* items with haspopup JAWS says "submenu" = yes (has popup state is true)
Firefox menubar menus:
* items without submenu ChildCount = 0
* items without submenu JAWS says "submenu" = no
* items with submenu ChildCount = 0
* items with submenu JAWS says "submenu" = yes (has popup state is true)
This seems like the right behaviour for ARIA menuitems. I think we want items with haspopup="true" to have a ChildCount of 0 because we don't know exactly what form the popup will take. However, the change to MustPrune() is affecting the Firefox menubar menuitems and this doesn't seem like the right behaviour: we still want the popup child I think.
Can we distinguish between ARIA menuitems and other Firefox menu items?
Reporter | ||
Comment 1•18 years ago
|
||
Reporter | ||
Comment 2•18 years ago
|
||
I've just done a clean and rebuild of everything and I'm now seeing different behaviour: I'm seeing my ARIA menuitems being pruned but the Firefox menubar items being unaffected. It looks like we might have the correct behaviour.
The only thing is that I'm a little confused as to how I got the Firefox menu item behaviour above. Perhaps there has been a change in some other code since I last updated and build?
Reporter | ||
Comment 3•18 years ago
|
||
Assignee | ||
Comment 4•18 years ago
|
||
Hi Simon, if you are seeking review for this patch you must mark it with "r?" and the name of the reviewer you want (me in this case).
Assignee | ||
Comment 5•18 years ago
|
||
Comment on attachment 266415 [details] [diff] [review]
Add ROLE_MENUITEM test to MustPrune()
We're getting the role twice now when we should only get it once. Just use GetFinalRole() for both.
Now that the method is getting more complex it shouldn't be defined inline in the declaration anymore. It should be define in nsAccessibleWrap.cpp.
Are you sure this isn't going to prune off submenus? I thought perhaps that if the role was MENUITEM then a second check on what children there are might be necessary, to see if there is a menupopup child. But I could be wrong. Make sure this doesn't regress anything in XUL menu support.
Attachment #266415 -
Flags: review-
Updated•18 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Updated•18 years ago
|
Assignee: nobody → aaronleventhal
Blocks: aria
Component: Disability Access → Disability Access APIs
Product: Firefox → Core
QA Contact: disability.access → accessibility-apis
Assignee | ||
Comment 6•18 years ago
|
||
* Also makes sure that ARIA buttons, textboxes and password fields prune their children (the HTML versions of those already did), by using GetFinalRole() to check for them instead of GetRole() (which doesn't take ARIA into account)
* The submenu pruning concern turned out not to be a problem -- they're not children of the menu item.
Attachment #273396 -
Flags: review?(simon.bates)
Comment 7•18 years ago
|
||
Possibly move the method implementation into .cpp file and add javastyled comment?
Reporter | ||
Comment 8•18 years ago
|
||
Comment on attachment 273396 [details] [diff] [review]
Clean up
I confirm that this patch fixes the issue with JAWS saying "submenu" on menuitems with children.
Attachment #273396 -
Flags: review?(simon.bates) → review+
Assignee | ||
Comment 9•18 years ago
|
||
Has patch -- needed for support of ARIA menus on Windows.
Flags: blocking1.9?
Assignee | ||
Updated•18 years ago
|
Flags: blocking1.9?
Assignee | ||
Updated•18 years ago
|
Attachment #273396 -
Flags: approval1.9?
Updated•18 years ago
|
Attachment #273396 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Updated•18 years ago
|
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•