249104 - Duplicated menus in ATK accessibility build

Status: RESOLVED DUPLICATE of bug 277750

(Core :: Disability Access APIs, defect)

Description

[Philip K. Warren]

When viewing Mozilla under at-poke, I see duplicated menus (but not menuitems).
For example, the hierarchy in at-poke looks something like this:

...
  File ->
    File ->
      New ->
        New ->
      Open Web Location
...

I also see this when navigating through Mozilla's menus in GOK.

Comment 1

[Philip K. Warren]

I believe this is a side effect of the checkin for Bug 242589. It removed the
following from nsAccessibilityService.cpp which prevented menu popup accessibles
from being created for invisible/offscreen menus:

-    PRUint32 role, state;
-    newAcc->GetRole(&role);
-    // don't create the accessible object for popup widget when it's not visible
-    if (role == nsIAccessible::ROLE_MENUPOPUP) {
-      newAcc->GetState(&state);
-      if (state & (nsIAccessible::STATE_INVISIBLE |
nsIAccessible::STATE_OFFSCREEN))
-        return NS_ERROR_FAILURE;
     }

Comment 2

[Philip K. Warren]

I'm not sure what the proper fix for this bug should be. I first considered not
creating accessible objects for <menupopup> XUL elements, thinking that we would
only need to expose the <menu> XUL elements. This doesn't work in all cases
however, since there are instances where <menupopup> XUL elements are used
without using a <menu> to wrap them.

I then tried making <menupopup> XUL elements use the ATK role
ATK_ROLE_POPUP_MENU instead of ATK_ROLE_MENU. With this change, the hierarchy in
at-poke looks like this:

-> File (menu)
  -> File (popup menu)
    -> New (menu)
      -> New (popup menu)
      ...
    -> Open Web Location (menu item)

This looks closer to what we want, however this broke navigation of the menus in
GOK.

Comment 3

[Bill Haneman]

The redundancy in the hierarchy is not good.  It's inconsistent with other
toolkits, and confusing to ATs.  The proper hierarchy is:

File (menu)
  New (menu)
    Navigator Window (menu item)
    Navigator Tab (menu item)
    ...
  Open Web Location (menu item)

etc.

Comment 4

[Philip K. Warren]

I'm beginning to think that we shouldn't create accessible objects for
<menupoup> at all. Since almost all <menupopup> elements have a <menu> parent
element, we would be pretty safe doing this. I went through all of the Mozilla
suite in DOM inspector, and found only a few instances where <menupopup> is used
without being contained in a <menu> element:

- <toolbarbutton type="menu"> or <toolbarbutton type="menu-button">. I plan on
fixing these cases in Bug 249292.
- The url bar history widget, where the autocomplete history is implemented as a
<menupopup> element with a <textbox> parent. I'm not really sure what to do here.
- The popup blocker menu, where a <menupopup> is a child of <popupset>.

Comment 5

[Aaron Leventhal]

I think Sun already has a fix for this. Correct?

Louie, can you see if there's a patch we can apply to the trunk for this?

Comment 6

[Louie Zhao]

Attachment: patch v1

I search the checkin list and found the fix, which is quite straightforward --
don't create accessible object for menupopup. At present, I have no time to
verify whether this patch works fine with trunk code because we have several
fixes in popup.xml for other issues.

Comment 7

[[e-mail]]

(In reply to comment #4)
>- The url bar history widget, where the autocomplete history is implemented as a
><menupopup> element with a <textbox> parent. I'm not really sure what to do here.
This works like an editable menulist, if that's any help?
> - The popup blocker menu, where a <menupopup> is a child of <popupset>.
That's a context menu. Or do you need it to be a <popup>?

Comment 8

[[e-mail]]

Comment on attachment 159835 [details] [diff] [review]
patch v1

A popup's parent should never be a menulist. sr=me if you fix the condition on
the following line.

Comment 9

[Aaron Leventhal]

Comment on attachment 159835 [details] [diff] [review]
patch v1

We also need to patch toolkit.

Hmm, how would createXULSelectListAccessible ever get called if we remove that
rule?

Comment 10

[Philip K. Warren]

*** This bug has been marked as a duplicate of 277750 ***