Last Comment Bug 786566 - ARIA menuitem acting as submenu should have PARENT_MENUITEM role
: ARIA menuitem acting as submenu should have PARENT_MENUITEM role
[good first bug][mentor=eitan@monoton...
: access
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla19
Assigned To: James Kitchener (:jkitch)
: alexander :surkov
Depends on:
Blocks: aria
  Show dependency treegraph
Reported: 2012-08-28 22:33 PDT by alexander :surkov
Modified: 2012-11-08 02:41 PST (History)
3 users (show)
surkov.alexander: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (4.46 KB, patch)
2012-11-02 03:14 PDT, James Kitchener (:jkitch)
tbsaunde+mozbugs: review+
Details | Diff | Splinter Review
patch v2 (4.39 KB, patch)
2012-11-05 02:55 PST, James Kitchener (:jkitch)
no flags Details | Diff | Splinter Review

Description User image alexander :surkov 2012-08-28 22:33:32 PDT
It makes sense on ATK where this role has special mapping. On the another hand it allows to workaround MustPrune issue which cuts the submenu if we keep menuitem role.

steps to fix:
1) add else if clause in Accessible::ARIATransformRole for menuitem role. If it has aria-haspopup attribute then return PARENT_MENUITEM role (refer to spec "If the menu item has its aria-haspopup attribute set to true, it indicates that the menu item may be used to launch a sub-level menu").
2) add test_aria_menu.html mochistest under tree folder (use test_aria_presentation.html) (don't forget to fix
Comment 1 User image James Kitchener (:jkitch) 2012-11-02 03:14:49 PDT
Created attachment 677697 [details] [diff] [review]
Comment 2 User image alexander :surkov 2012-11-02 22:42:45 PDT
Comment on attachment 677697 [details] [diff] [review]

Trev, have a time for review over weekends?
Comment 3 User image Trevor Saunders (:tbsaunde) 2012-11-04 17:26:41 PST
Comment on attachment 677697 [details] [diff] [review]

>+      if (mContent->AttrValueIs(kNameSpaceID_None,
>+                                nsGkAtoms::aria_haspopup,
>+                                nsGkAtoms::_true,
>+                                eCaseMatters)) {

nit, please try to reduce lines while staying within 80 char limit.

>+  <div id="menu_popup">
>+    <ul role="menu">
>+        <li role="menuitem" aria-haspopup="true">Menu with popup</li>
>+    </ul>
>+  </div>
>+  <div id="menu_nopopup">
>+    <ul role="menu">
>+      <li role="menuitem" aria-haspopup="false">Menu with explicit no popup</li>
>+    </ul>
>+  </div>

nit, it would be nice if you keep these in the same order as the tests
Comment 4 User image James Kitchener (:jkitch) 2012-11-05 02:55:11 PST
Created attachment 678257 [details] [diff] [review]
patch v2

Suggested changes have been made.
Comment 5 User image alexander :surkov 2012-11-07 15:55:37 PST

thanks, James!
Comment 6 User image Ed Morley [:emorley] 2012-11-08 02:41:42 PST

Note You need to log in before you can comment on or make changes to this bug.