Last Comment Bug 786566 - ARIA menuitem acting as submenu should have PARENT_MENUITEM role
: ARIA menuitem acting as submenu should have PARENT_MENUITEM role
Status: RESOLVED FIXED
[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)
:
Mentors:
http://www.w3.org/TR/wai-aria/roles#m...
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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
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 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 Makefile.in)
Comment 1 James Kitchener (:jkitch) 2012-11-02 03:14:49 PDT
Created attachment 677697 [details] [diff] [review]
patch
Comment 2 alexander :surkov 2012-11-02 22:42:45 PDT
Comment on attachment 677697 [details] [diff] [review]
patch

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

>+      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 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 alexander :surkov 2012-11-07 15:55:37 PST
landed https://hg.mozilla.org/integration/mozilla-inbound/rev/b029a90619f5

thanks, James!
Comment 6 Ed Morley [:emorley] 2012-11-08 02:41:42 PST
https://hg.mozilla.org/mozilla-central/rev/b029a90619f5

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