Skip to content

fix: [a11y] fire AXMenuOpened event when ARIA menu is added to DOM#50377

Merged
VerteDinde merged 4 commits intomainfrom
fix-axmenuopened-event-firing
Mar 25, 2026
Merged

fix: [a11y] fire AXMenuOpened event when ARIA menu is added to DOM#50377
VerteDinde merged 4 commits intomainfrom
fix-axmenuopened-event-firing

Conversation

@VerteDinde
Copy link
Copy Markdown
Member

@VerteDinde VerteDinde commented Mar 19, 2026 •
edited
Loading

Description of Change

Fixes upstream bug https://issues.chromium.org/issues/40794596

This PR attempts to fix a long-standing bug with screen readers, where AXMenuOpened is not fired when an element with a role of "menu" is created and added to the DOM. If the DOM element exists and is toggled to be visible (e.g. the HTML hidden attribute is toggled), the AXMenuOpened is fired! However, it's not fired for newly created elements.

I'm also going to attempt to upstream this patch, but given the original solution was reverted twice due to issues with Gmail's menus, I'd like to land this in Electron so apps can benefit while we try to land upstream.

This patch can be removed when the upstream CL lands.

Testing:

Since this fix needs the Accessibility Inspector (or a similar tool on Windows/Linux) to validate it, I haven't yet figured out a good automated test. You can verify the fix with this Fiddle: https://gist.github.com/81c2e1de58350893627d736b3b038b91

Steps to Repro:

  1. Open the MacOS Accessibility Inspector
  2. Open the Notifications window of the Accessibility Inspector by going to the menubar and choosing "Window > Show Notifications"
  3. Run the Fiddle gist, and attach the Accessibility Inspector node to the running window
  4. Hit "Show Menu" - see the AXMenuOpened is fired
  5. Hit "Hide Menu" and then "Toggle Menu" - see the AXMenuOpened is fired
  6. In Fiddle, switch to a current version of Electron (e.g. 42.0.0-alpha.1) and repeat steps 4 & 5. You will see that AXMenuOpened is not fired when clicking "Show Menu"

Checklist

Release Notes

Notes: Fixed an accessibility issue where the AXMenuOpened event was not fired on menu creation.

@VerteDinde VerteDinde requested a review from a team as a code owner March 19, 2026 18:19
@VerteDinde VerteDinde added semver/patch backwards-compatible bug fixes target/40-x-y PR should also be added to the "40-x-y" branch. target/41-x-y PR should also be added to the "41-x-y" branch. target/42-x-y PR should also be added to the "42-x-y" branch. labels Mar 19, 2026
@electron-cation electron-cation bot added the new-pr 🌱 PR opened recently label Mar 19, 2026
@VerteDinde VerteDinde changed the title fix [a11y]: fire AXMenuOpened event when ARIA menu is added to DOM fix: [a11y] fire AXMenuOpened event when ARIA menu is added to DOM Mar 19, 2026
deepak1556
deepak1556 requested changes Mar 20, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@deepak1556 deepak1556 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a link to the active CL with this change ?

Also can you add this change behind a //ui feature flag that is disabled by default. I am trying to exercise caution here as the upstream bug has gone through multiple reverts, would like the change to be stabilized before its enabled by default and also any review changes to the CL from the component owners can go through iterations in electron if it was behind feature flag.

This will also allow the fix to be exercised for your use case without blocking the PR.

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened recently label Mar 20, 2026
@VerteDinde VerteDinde force-pushed the fix-axmenuopened-event-firing branch from 1382b41 to fb5f30a Compare March 25, 2026 05:30
deepak1556
deepak1556 approved these changes Mar 25, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@VerteDinde
Copy link
Copy Markdown
Member Author

VerteDinde commented Mar 25, 2026

Thanks @deepak1556, that was a good callout! Appreciate the review too, I'll link the upstream CL in the PR body 🙇‍♀️

VerteDinde and others added 3 commits March 25, 2026 10:27
@VerteDinde
fix: fire AXMenuOpened event when a visible ARIA menu instance is add…
10781ec 
…ed to the DOM
@VerteDinde
fix: remove redundent FireMenuPopupEndForDeletedMenus
1e0175b 
MENU_POPUP_END for deleted menus is already handled by
AXTreeManager::OnNodeWillBeDeleted, which
fires the event directly on the menu node before destruction.
@VerteDinde
chore: add feature flag (kDynamicMenuPopupEvents)
8ae963f
@VerteDinde VerteDinde force-pushed the fix-axmenuopened-event-firing branch from fb5f30a to 8ae963f Compare March 25, 2026 17:27
@VerteDinde VerteDinde enabled auto-merge (squash) March 25, 2026 20:12
@VerteDinde VerteDinde merged commit a839fb9 into main Mar 25, 2026
64 checks passed
@VerteDinde VerteDinde deleted the fix-axmenuopened-event-firing branch March 25, 2026 21:33
@release-clerk
Copy link
Copy Markdown

release-clerk bot commented Mar 25, 2026

Release Notes Persisted

Fixed an accessibility issue where the AXMenuOpened event was not fired on menu creation.

This was referenced Mar 25, 2026
Merged
Merged
Merged
@trop
Copy link
Copy Markdown
Contributor

trop bot commented Mar 25, 2026

I have automatically backported this PR to "42-x-y", please check out #50504

@trop trop bot added in-flight/42-x-y and removed target/42-x-y PR should also be added to the "42-x-y" branch. labels Mar 25, 2026
@trop
Copy link
Copy Markdown
Contributor

trop bot commented Mar 25, 2026

I have automatically backported this PR to "40-x-y", please check out #50505

@trop
Copy link
Copy Markdown
Contributor

trop bot commented Mar 25, 2026

I have automatically backported this PR to "41-x-y", please check out #50506

@trop trop bot added in-flight/41-x-y merged/40-x-y PR was merged to the "40-x-y" branch. merged/42-x-y PR was merged to the "42-x-y" branch. merged/41-x-y PR was merged to the "41-x-y" branch. and removed target/40-x-y PR should also be added to the "40-x-y" branch. target/41-x-y PR should also be added to the "41-x-y" branch. in-flight/40-x-y in-flight/42-x-y in-flight/41-x-y labels Mar 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@deepak1556 deepak1556 deepak1556 approved these changes

Assignees

No one assigned

Labels

merged/40-x-y PR was merged to the "40-x-y" branch. merged/41-x-y PR was merged to the "41-x-y" branch. merged/42-x-y PR was merged to the "42-x-y" branch. semver/patch backwards-compatible bug fixes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@VerteDinde @deepak1556