Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why is a menuitem in a group special, but only on OS X? #78

Closed
aleventhal opened this issue Aug 12, 2020 · 10 comments · Fixed by #132
Closed

Why is a menuitem in a group special, but only on OS X? #78

aleventhal opened this issue Aug 12, 2020 · 10 comments · Fixed by #132
Assignees
Milestone

Comments

@aleventhal
Copy link
Contributor

A menuitem in a group is weird in one way. Spec says that it is exposed as an AXMenuButton instead of an AXMenuItem.
See https://www.w3.org/TR/core-aam-1.2/#details-id-45

Seems bad to me, because:

  • It's confusing that there is nothing ARIA spec that mentions any special meaning or behavior of a menuitem in a group, and yet it's mapped in a special way. What's the use case, and, if it's important, why is it only a thing on OS X?
  • I just discovered that Chrome gets this wrong, but no one actually ever filed an issue for it over the years.
  • What happens when an author actually means to have a menu with several groups of menuitems. Will it behave differently, but only with VoiceOver? Is this only meant to be special when there is a single menuitem in the group?

I'm wondering if we should be fixing this in Chrome (and if so clarifying the specs), or, should it be removed as a thing from CORE-AAM?

@aleventhal
Copy link
Contributor Author

@cookiecrook can you help me understand this one?

@aleventhal
Copy link
Contributor Author

Also, a menu button sounds like it should have a drop down, which this doesn't, or does it support that but is undocumented?

blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue Aug 13, 2020
Fixes regression from CL:2142611 where a menuitem inside of a
group no longer gets an accessible name.

This was due to moving kMenuButton inside the NameFromContents()
switch, such that it would no longer get a name from contents.
Currently, a role=menuitem inside a role=group gets remapped to
kMenuButton via AXObject::RemapAriaRoleDueToParent().

The purpose of remapping menuitem inside of a group to kMenuButton is
to expose it as AXMenuButton instead of AXMenuItem on OS, per CORE-AAM.
This remapping may be removed in the future, and is an open issue
in CORE-AAM. See w3c/core-aam#78

Bug: 1114712
Change-Id: Iae23b2351e72486e1cf3bf4b96ce1ff283e4ad97
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2353019
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Auto-Submit: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797667}
@aleventhal
Copy link
Contributor Author

Also, if we keep the rule, maybe it should only be a lone menu item in a group?

blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue Aug 14, 2020
A kMenuButton is a menuitem inside of a group. The Mac mapping is
special. Everything else just maps to the platform's menu item role.

Without this change, non-Mac platforms announce a menuitem in a group
as a menu, when it's actually a menuitem.

In the future, the special Mac mapping may change, as it is likely
problematic. More info at
w3c/core-aam#78

TBR=haraken@chromium.org

Bug: 1114712
Change-Id: I3c065f474e235915e5b22569dad08a0e250077f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2353431
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Auto-Submit: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798109}
@cookiecrook
Copy link
Contributor

The only guess I have is that the [contained by group | not contained by group ] distinction is a typo that should be referencing menubar versus menu instead of group versus no group. items in a menubar are typically exposed as menu buttons.

This is only a guess.

@cookiecrook cookiecrook removed their assignment Aug 17, 2020
@aleventhal
Copy link
Contributor Author

@cookiecrook, the WebKit code also remaps a role=menuitem inside of a role=group to be a menu button, in remapAriaRoleDueToParent(). Possible bug in implementations and specs?

@aleventhal
Copy link
Contributor Author

If it helps, here is the WebKit commit where it was added, along with the author and reviewers:

commit 478547cfff0c1c4d0d1def735f9576ac60711d16
Author: alice.liu@apple.com <alice.liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Fri May 23 06:44:23 2008 +0000

    2008-05-22  Alice Liu  <alice.liu@apple.com>

            Reviewed by Adele, Dan Bernstein.

            Fixed <rdar://problem/5943104> Need to implement ARIA role="menu" and related roles
            <rdar://problem/5943132> Need to implement ARIA role="menuitem"
            <rdar://problem/5943173> Need to implement ARIA role="menubar"

            These changes added a handler for determining the ARIA role when the
            attribute changes.  Before we were querying for the attribute over and
            over every time we queried for the role.
            * dom/Element.cpp:
            (WebCore::Element::attributeChanged):
            * page/AXObjectCache.cpp:
            (WebCore::AXObjectCache::handleAriaRoleChanged):
            * page/AXObjectCache.h:

Here's the code that was introduced in that commit that does it:

+    if (equalIgnoringCase(ariaRole,"menuitem")) {
+        if (parentObjectUnignored()->ariaRoleAttribute() == GroupRole)
+            return MenuButtonRole;

@aleventhal
Copy link
Contributor Author

Here's an HTML example that shows both Chrome and Safari report "menubutton" w/ VoiceOver.

<div role="group">
  <div role="menuitem" tabindex="0">Single menu item in a group</div>
</div>

<div role="group">
  <div role="menuitem" tabindex="0">Menu item #1 in a group</div>
  <div role="menuitem" tabindex="0">Menu item #2 in a group</div>
</div>

@aleventhal
Copy link
Contributor Author

Also, I don't see any mapping in WebKit or Chrome changing a role=menuitem inside of a role=menubar to a menu button.
Testing the ARIA practices example for menubar shows that the items are read as menuitem, not menu button:
https://w3c.github.io/aria-practices/examples/menubar/menubar-editor.html

@cookiecrook
Copy link
Contributor

cookiecrook commented Aug 21, 2020

In the 2020-08-20 ARIA WG meeting we agreed to remove this advice from core-aam.

I've filed the related WebKit bug as https://webkit.org/b/215739

@cookiecrook cookiecrook self-assigned this Aug 21, 2020
@cookiecrook cookiecrook added this to the 1.3 milestone Aug 21, 2020
@carmacleod
Copy link
Contributor

Agree that the whole "menu item owned by or child of group" entry should be removed, and the "menuitem not owned by or child of group" entry should just be for "menuitem". Reminder that some tests will need to be rewritten/removed:
https://www.w3.org/wiki/Core_AAM_1.1_Testable_Statements#menuitem_not_owned_by_or_child_of_group
https://www.w3.org/wiki/Core_AAM_1.1_Testable_Statements#menuitem_owned_by_or_child_of_group


I almost wonder if it was originally some kind of typo or misunderstanding in the core-aam spec that was propagated to later versions of the spec and also into Chrome.

The WAI-ARIA 1.0 User Agent Implementation Guide entry for menuitem has slightly different wording, using the word "option" for some reason (maybe copy/pasted from a special case in option role?):

If the option's parent has a group role, then role="menuitem" maps to AXMenuButton
If the option's parent has a menu role, then role="menuitem" maps to AXMenuItem

Someone from Chromium picked up on this "special case" in https://codereview.chromium.org/699083007, in which dmazzoni even said:

I don't understand the reasoning behind the spec here. It seems incomplete.

and then in a later comment:

I'm going to file a bug asking for clarification, because I still don't understand the purpose of the spec.

I don't know where core-aam bugs were reported back in 2014, so I don't know if the request for clarification was ever filed.

Later, "special case" role mappings in the core-aam spec were broken into separate tables, and the word "option" was dropped. The special case was interpreted as belonging to menuitem (which was logical because it was in the entry for menuitem).

mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
Fixes regression from CL:2142611 where a menuitem inside of a
group no longer gets an accessible name.

This was due to moving kMenuButton inside the NameFromContents()
switch, such that it would no longer get a name from contents.
Currently, a role=menuitem inside a role=group gets remapped to
kMenuButton via AXObject::RemapAriaRoleDueToParent().

The purpose of remapping menuitem inside of a group to kMenuButton is
to expose it as AXMenuButton instead of AXMenuItem on OS, per CORE-AAM.
This remapping may be removed in the future, and is an open issue
in CORE-AAM. See w3c/core-aam#78

Bug: 1114712
Change-Id: Iae23b2351e72486e1cf3bf4b96ce1ff283e4ad97
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2353019
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Auto-Submit: Aaron Leventhal <aleventhal@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#797667}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: e12d47e157372df244fa13d095c86da967aea8bc
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
A kMenuButton is a menuitem inside of a group. The Mac mapping is
special. Everything else just maps to the platform's menu item role.

Without this change, non-Mac platforms announce a menuitem in a group
as a menu, when it's actually a menuitem.

In the future, the special Mac mapping may change, as it is likely
problematic. More info at
w3c/core-aam#78

TBR=haraken@chromium.org

Bug: 1114712
Change-Id: I3c065f474e235915e5b22569dad08a0e250077f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2353431
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Auto-Submit: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798109}
GitOrigin-RevId: efbb0a5667b07383dd47ad5c3c65324f8a86f1e0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants