1805415 - Use XULPopupElement.activateElement rather than menuitem.click() for testing menuitems

Status: RESOLVED FIXED

(Firefox :: Menus, task)

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1805415 - Use activateItem() rather than click() to activate menuitems. r=Gijs!

Bug 1805414 will move menu event handling to the DOM.

With that change the current synthetic click behavior of XUL menuitems
breaks. On current central, we rely on nsMenuFrame::HandleEvent not
getting called at all for synthetic clicks, and instead we just fire a
command event synchronously here:

https://searchfox.org/mozilla-central/rev/a0d4f8f112c5c792ae272bf6ce50763ddd23ffa2/dom/xul/nsXULElement.cpp#1071

After my patch the command event is fired properly (potentially
asynchronously too) by the regular menu activation machinery, which is
preferable.

• They fire a command event synchronously (even though on some
  platforms like macOS activating a context menu item is async).

• They use a totally different codepath from what a user does.

• They don't deal with native menus, etc.

We have a proper API for this (activateItem) which takes a much more
closer codepath to what users do, requires that the menu is shown, etc.
Use that API instead for testing.

As a benefit, tests now do not need to close the context menu manually
when clicking on a menu item (because we trigger the same code path as
users clicking the menu).

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e13513500184
Use activateItem() rather than click() to activate menuitems. r=Gijs,extension-reviewers,pip-reviewers,search-reviewers

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5056d7df9f1e
Revert most changes to browser_page_style_menu_update.js since the native menu bar is hard.

Comment 4

[Cristian Tuns]

Backed out for causing dt failures on browser_net_telemetry_throttle_changed.js

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | devtools/client/netmonitor/test/browser_net_telemetry_throttle_changed.js | Uncaught exception in test - InvalidStateError: XULPopupElement.activateItem: Menu containing menu item is not open

Comment 5

[Cristian Tuns]

And also this: https://treeherder.mozilla.org/logviewer?job_id=399626455&repo=autoland

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f446a96d1211
Use activateItem() rather than click() to activate menuitems. r=Gijs,extension-reviewers,pip-reviewers,search-reviewers

Comment 7

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/f446a96d1211