Closed Bug 933322 Opened 11 years ago Closed 11 years ago

menustart/end events are missing when aria-owns makes a menu hierarchy

Categories

(Core :: Disability Access APIs, defect)

Product:
Core
Component:
Disability Access APIs
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla28

People

(Reporter: surkov, Assigned: surkov)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

patch
7.72 KB, patch
tbsaunde
: review+
Details | Diff | Splinter Review 
make sure to fix Dojo's example: http://hanshillen.github.io/jqtest/#goto_menubar. No menustart event when top level menuitem is focused (no menu open)
(In reply to alexander :surkov from comment #1)
> make sure to fix Dojo's example:
> http://hanshillen.github.io/jqtest/#goto_menubar. No menustart event when
> top level menuitem is focused (no menu open)

My findings with the *jQuery* example (not dojo):

The EVENT_MENU_START is emitted ONLY when one of the menus is actually popped open.  It is possible to put focus on the menu items in the menubar, and arrow across the menubar *without* opening any menus.  In that case, there is no EVENT_MENU_START.  But, using a down arrow to open a menu causes both a EVENT_MENU_START and a EVENT_MENU_POPUPSTART.

Tested with FF 25.0 on Win7.
Blocks: aria
I filed bug 934460 for jQuery Hans's example.
Attached patch patch — — Splinter Review 
not super nice but similar to what we have
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #827501 - Flags: review?(trev.saunders)
(In reply to alexander :surkov from comment #4)
> Created attachment 827501 [details] [diff] [review]
> patch
> 
> not super nice but similar to what we have

Just curious: why not use RELATION_NODE_CHILD_OF/RELATION_NODE_PARENT_OF to find the menu, in the case of aria-owns?
(In reply to Joseph Scheuhammer from comment #5)

> Just curious: why not use RELATION_NODE_CHILD_OF/RELATION_NODE_PARENT_OF to
> find the menu, in the case of aria-owns?

extra useless mechanism would be in use in this case
Comment on attachment 827501 [details] [diff] [review]
patch

>+    Accessible* ARIAMenubar = nullptr;
>+    Accessible* child = target;

what is the point in this var? it seems you could directly use target->Parent() here and don't really need it later either.

>+    Accessible* parent = child->Parent();
>+    while (parent) {

don't you loose check accessible is within doc for target?
(In reply to Trevor Saunders (:tbsaunde) from comment #7)
> Comment on attachment 827501 [details] [diff] [review]
> patch
> 
> >+    Accessible* ARIAMenubar = nullptr;
> >+    Accessible* child = target;
> 
> what is the point in this var? it seems you could directly use
> target->Parent() here and don't really need it later either.

ok

> >+    Accessible* parent = child->Parent();
> >+    while (parent) {
> 
> don't you loose check accessible is within doc for target?

it was perf check, now we are restricted by role and relation checks
> > >+    Accessible* parent = child->Parent();
> > >+    while (parent) {
> > 
> > don't you loose check accessible is within doc for target?
> 
> it was perf check, now we are restricted by role and relation checks

are you sure that's true in all cases even weird things like <body role="menu"> in <iframe role="menu"> ?
(In reply to Trevor Saunders (:tbsaunde) from comment #9)

> are you sure that's true in all cases even weird things like <body
> role="menu"> in <iframe role="menu"> ?

if people do this then they possibly have a reason, it shouldn't bring us any problems
(In reply to alexander :surkov from comment #10)
> (In reply to Trevor Saunders (:tbsaunde) from comment #9)
> 
> > are you sure that's true in all cases even weird things like <body
> > role="menu"> in <iframe role="menu"> ?
> 
> if people do this then they possibly have a reason, it shouldn't bring us
> any problems

I guess, on the other hand I'd rather be strict in what we allow in case it may constrict what we can do in future.
(In reply to Trevor Saunders (:tbsaunde) from comment #11)

> I guess, on the other hand I'd rather be strict in what we allow in case it
> may constrict what we can do in future.

I think nothing prevents the author to make cross-documents menus, the spec doesn't deny that and I don't really have an objection to deny that on our side (note, we had a perf concern previously)
(In reply to alexander :surkov from comment #12)
> (In reply to Trevor Saunders (:tbsaunde) from comment #11)
> 
> > I guess, on the other hand I'd rather be strict in what we allow in case it
> > may constrict what we can do in future.
> 
> I think nothing prevents the author to make cross-documents menus, the spec
> doesn't deny that and I don't really have an objection to deny that on our
> side (note, we had a perf concern previously)

It seems odd, but I can't think of a reason not to support it other than you aren't going to use it and it might possibly get in the way.
Attachment #827501 - Flags: review?(trev.saunders) → review+
https://hg.mozilla.org/mozilla-central/rev/b4a630050f61
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Depends on: 970005
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: