DOMMenuItemActive is fired for submenu parent when other menu is activated

RESOLVED FIXED in mozilla21

Status

()

RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: surkov, Assigned: enndeakin)

Tracking

(Blocks: 1 bug, {access})

unspecified
mozilla21
x86
Windows 7
access
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
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

7 years ago
Blocks: 375743
(Reporter)

Comment 1

6 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

6 years ago
Created attachment 675085 [details] [diff] [review]
Patch to skip extra DOMMenuItemActive event
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Attachment #675085 - Flags: feedback?(surkov.alexander)
(Reporter)

Comment 3

6 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

6 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

6 years ago
Attachment #675085 - Flags: review?(neil)

Comment 5

6 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

6 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

6 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

6 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

6 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 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

6 years ago
Flags: in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/2bcb93c79fff
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.