Closed
Bug 685191
Opened 13 years ago
Closed 12 years ago
DOMMenuItemActive is fired for submenu parent when other menu is activated
Categories
(Core :: XUL, defect)
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: surkov, Assigned: enndeakin)
References
(Blocks 1 open bug)
Details
(Keywords: access)
Attachments
(1 file)
42.99 KB,
patch
|
neil
:
review+
surkov
:
feedback+
|
Details | Diff | Splinter Review |
steps to reproduce:
1) alt + v to select View -> Toolbars
DOMMenuItemActive events for view-menu and viewToolbarsMenu (correct)
2) right arrow to select View -> Toolbars -> Menubar
DOMMenuItemActive events for toggle_toolbar-menubar (correct)
3) right arrow to select History -> Back
DOMMenuItemActive events for history-menu, viewToolbarsMenu (incorrect), historyMenuBack
This event is quite confusing since happens between events for 'History' and 'History -> Back'.
DOMMenuItemActive event is used to fire accessible focus event what is announced by screen readers. Since basically DOMMenuItemActive events are asynchronous then this should lead to intermittent noisy screen reader's output.
Reporter | ||
Updated•13 years ago
|
Blocks: focuseventa11y
Reporter | ||
Comment 1•12 years ago
|
||
Enn, can you take a look at this issue as well since you're around menu a11y focus problems anyway? :)
These bugs aren't visible normally but they hit screen reader users.
Assignee | ||
Comment 2•12 years ago
|
||
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Attachment #675085 -
Flags: feedback?(surkov.alexander)
Reporter | ||
Comment 3•12 years ago
|
||
There's no exess DOMMenuItemActive. However some events are not in sync for example DOMMenu(Item)Inactive are fired after DOMMenuItemActive, some are fired before it. I don't know however whether this makes a problem for AT. See log below for details.
Example:
<menu id="fruit" label="Fruit">
<menupopup>
<menuitem id="apple" label="Apple"/>
<menuitem id="orange" label="Orange" disabled="true"/>
</menupopup>
</menu>
<menu id="vehicle" label="Vehicle">
<menupopup>
<menu id="cycle" label="cycle">
<menupopup>
<menuitem id="tricycle" label="tricycle"/>
</menupopup>
</menu>
<menuitem id="car" label="Car" disabled="true"/>
</menupopup>
</menu>
Test: right key on 'tricycle' menu item.
Event queue:
invoke: 'vehicle' 'right ' key
TEST-INFO | unknown test url | Invoke the ' 'vehicle' 'right ' key' test { expected 'focus' event; unexpected 'focus' event; }
A11Y DOMEvents: event 'DOMMenuItemInactive' handled; 24:44.848
{
Target: 0C7F7090; role: parent menuitem, name: 'Vehicle';
Target node: 0A879E38, menu@id='vehicle', idx in parent: 1
}
A11Y DOMEvents: event 'DOMMenuItemActive' handled; 24:44.848
{
Target: 0C7F7000; role: parent menuitem, name: 'Fruit';
Target node: 0A87A8B8, menu@id='fruit', idx in parent: 0
}
A11Y FOCUS: active item changed; 24:44.848
{
Item: 0C7F7000; role: parent menuitem, name: 'Fruit';
Item node: 0A87A8B8, menu@id='fruit', idx in parent: 0
}
{
Caused by: DOMMenuItemActive
Item: 0C7F7000; role: parent menuitem, name: 'Fruit';
Item node: 0A87A8B8, menu@id='fruit', idx in parent: 0
}
A11Y DOMEvents: event 'popuphiding' handled; 24:44.863
{
Target: 03709AD0; role: menupopup, name: 'cycle';
Target node: 0A87ABB8, menupopup@id='', idx in parent: 0
}
A11Y DOMEvents: event 'popuphiding' handled; 24:44.863
{
Target: 0AA638C0; role: menupopup, name: 'Vehicle';
Target node: 0A87A638, menupopup@id='', idx in parent: 0
}
A11Y DOMEvents: event 'DOMMenuItemInactive' handled; 24:44.957
{
Target: 05707A68; role: menuitem, name: 'tricycle';
Target node: 0A87A6B8, menuitem@id='tricycle', idx in parent: 0
}
A11Y DOMEvents: event 'DOMMenuInactive' handled; 24:44.957
{
Target: 03709AD0; role: menupopup, name: 'cycle';
Target node: 0A87ABB8, menupopup@id='', idx in parent: 0
}
Event type: menupopup end. Target: ['menupopup node', address: [object XULElement], role: menupopup, name: 'cycle', address: 0x3709ad0]
A11Y FOCUS: fire focus event; 24:44.973
{
Target: 0C7F7000; role: parent menuitem, name: 'Fruit';
Target node: 0A87A8B8, menu@id='fruit', idx in parent: 0
}
Event type: focus. Target: ['menu@id="fruit" node', address: [object XULElement], role: parent menuitem, name: 'Fruit', address: 0xc7f7000]. Listeners count: 1
A11Y DOMEvents: event 'DOMMenuItemInactive' handled; 24:44.988
{
Target: 05707678; role: parent menuitem, name: 'cycle';
Target node: 0A87A038, menu@id='cycle', idx in parent: 0
}
A11Y DOMEvents: event 'DOMMenuInactive' handled; 24:44.988
{
Target: 0AA638C0; role: menupopup, name: 'Vehicle';
Target node: 0A87A638, menupopup@id='', idx in parent: 0
}
Event type: menupopup end. Target: ['menupopup node', address: [object XULElement], role: menupopup, name: 'Vehicle', address: 0xaa638c0]
A11Y DOMEvents: event 'DOMMenuItemActive' handled; 24:44.988
{
Target: 057074C8; role: menuitem, name: 'Apple';
Target node: 0A87A9B8, menuitem@id='apple', idx in parent: 0
}
A11Y FOCUS: active item changed; 24:44.988
{
Item: 057074C8; role: menuitem, name: 'Apple';
Item node: 0A87A9B8, menuitem@id='apple', idx in parent: 0
}
{
Caused by: DOMMenuItemActive
Item: 057074C8; role: menuitem, name: 'Apple';
Item node: 0A87A9B8, menuitem@id='apple', idx in parent: 0
Document: 0AC773A8, document node: 07CDEE20
Document uri: chrome://mochitests/content/a11y/accessible/events/test_focus_menu.xul
}
A11Y DOMEvents: event 'popupshown' handled; 24:45.035
{
Target: 0AA63660; role: menupopup, name: 'Fruit';
Target node: 0A87A7B8, menupopup@id='', idx in parent: 0
}
Event type: menupopup start. Target: ['menupopup node', address: [object XULElement], role: menupopup, name: 'Fruit', address: 0xaa63660]
A11Y FOCUS: fire focus event; 24:45.160
{
Target: 057074C8; role: menuitem, name: 'Apple';
Target node: 0A87A9B8, menuitem@id='apple', idx in parent: 0
}
Event type: focus. Target: ['menuitem@id="apple" node', address: [object XULElement], role: menuitem, name: 'Apple', address: 0x57074c8]. Listeners count: 1
*****
EQ matched: focus
Reporter | ||
Comment 4•12 years ago
|
||
Comment on attachment 675085 [details] [diff] [review]
Patch to skip extra DOMMenuItemActive event
f=me per comment #3. Thank you!
Attachment #675085 -
Flags: feedback?(surkov.alexander) → feedback+
Assignee | ||
Updated•12 years ago
|
Attachment #675085 -
Flags: review?(neil)
Comment 5•12 years ago
|
||
Sorry for the delay, but I wanted to try to understand what was going on. The bit that confuses me is when the menubar navigates from one top-level menu to another. Why doesn't it pass true for aDeactivateMenu when hiding the previous menu's popup?
Flags: needinfo?(enndeakin)
Comment 6•12 years ago
|
||
Sorry for the delay, but I wanted to try to understand what was going on. The bit that confuses me is when the menubar navigates from one top-level menu to another. Why doesn't it pass true for aDeactivateMenu when hiding the previous menu's popup?
Assignee | ||
Comment 7•12 years ago
|
||
(In reply to neil@parkwaycc.co.uk from comment #6)
> Sorry for the delay, but I wanted to try to understand what was going on.
> The bit that confuses me is when the menubar navigates from one top-level
> menu to another. Why doesn't it pass true for aDeactivateMenu when hiding
> the previous menu's popup?
nsMenuBarFrame::ChangeMenuItem already deselects the old menu and selects the new one.
Flags: needinfo?(enndeakin)
Comment 8•12 years ago
|
||
(In reply to Neil Deakin from comment #7)
> (In reply to comment #6)
> > Sorry for the delay, but I wanted to try to understand what was going on.
> > The bit that confuses me is when the menubar navigates from one top-level
> > menu to another. Why doesn't it pass true for aDeactivateMenu when hiding
> > the previous menu's popup?
>
> nsMenuBarFrame::ChangeMenuItem already deselects the old menu and selects
> the new one.
Ah, right, so you would get two DOMMenuItemInactive events instead.
Comment 9•12 years ago
|
||
(In reply to from comment #8)
> (In reply to Neil Deakin from comment #7)
> > (In reply to comment #6)
> > > Sorry for the delay, but I wanted to try to understand what was going on.
> > > The bit that confuses me is when the menubar navigates from one top-level
> > > menu to another. Why doesn't it pass true for aDeactivateMenu when hiding
> > > the previous menu's popup?
> >
> > nsMenuBarFrame::ChangeMenuItem already deselects the old menu and selects
> > the new one.
>
> Ah, right, so you would get two DOMMenuItemInactive events instead.
Interestingly you already get two DOMMenuItemInactive events from closing a menubar (because MenuClosed() calls SelectMenu(false) too) but that might be a bug in MenuClosed() (the only other caller is the popup manager but only if there is no popup to close).
Comment 10•12 years ago
|
||
Comment on attachment 675085 [details] [diff] [review]
Patch to skip extra DOMMenuItemActive event
r=me if you improve the comment by mentioning that nsMenuBarFrame::ChangeMenuItem already deselects the old menu before closing the popup which is why it doesn't bother deselecting the menu again which is why we have to check that we're changing menu item before firing the active event. (If it didn't then none of this would have to happen, but I'm not going to get drawn into a long discussion of why nsMenuBarFrame::ChangeMenuItem feels it needs to deselect the old menu before closing the popup.)
Attachment #675085 -
Flags: review?(neil) → review+
Assignee | ||
Updated•12 years ago
|
Flags: in-testsuite+
Comment 11•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Updated•6 years ago
|
Component: XP Toolkit/Widgets: Menus → XUL
You need to log in
before you can comment on or make changes to this bug.
Description
•