Context menu on secondary windows doesn't work

[planger]

Bug Description:

If I open a context menu in a secondary window, the context menu shows up in the main window and not on the secondary window. The position in the window seems to be correct, but the window is the wrong one.

Debugging into it briefly (in the browser case at least), it seems that Phosphor needs to be made aware of the current window. At the moment it just plainly adds the context menu to document. So we need to pass the correct window or its document from the context-menu event on to Phosphor's openRootMenu() function.

The call stack is as follows:

-  openRootMenu (theia/node_modules/@phosphor/widgets/lib/menu.js:1133)
- __webpack_modules__.../../node_modules/@phosphor/widgets/lib/menu.js.Menu.open (theia/node_modules/@phosphor/widgets/lib/menu.js:437)
- open (theia/packages/core/src/browser/menu/browser-menu-plugin.ts:276)
- doRender (theia/packages/core/src/browser/menu/browser-context-menu-renderer.ts:43)
- render (theia/packages/core/src/browser/context-menu-renderer.ts:85)
- contextMenuListener (theia/packages/terminal/src/browser/terminal-widget-impl.ts:262)

Steps to Reproduce:

1. Start Theia
2. Open a terminal view
3. Detach the terminal view into a secondary window
4. Open context menu via right-click or context menu key
5. Observe how the context menu doesn't show up in the secondary window but in the main window

Additional Information

• Operating System: Ubuntu 22.04
• Theia Version: master (3deecfe)

Related:

• secondary-window: add support for additional views besides webviews #11643
• Multi Window support (Floating windows) #10526

[tsmaeder]

The trouble is that the openRootMenu function does not take such a parameter. @sdirix maybe another patch we can propose once we move to Lumino.

[tsmaeder]

@planger did you already have an idea on how to fix this in Phosphor menu case?

[planger]

@tsmaeder No sorry, I didn't investigate this issue yet more closely. But as far as I remember, this would indeed need a patch in the Phosphor code to allow passing the current window to the openRootMenu() function.

[tsmaeder]

I've been able to hack the phosphor menu code to handle the multi-window case. Still a ways to go, but in principle, it should work. The necessary changes probably need some more fancy tooling for patching Phosphor than the string-replace we're using now.

2024-03-08.14-09-46.mp4