Just call update on all submenus instead of ones that are visible

[williameveretteggplant]

In Windows qualifying update on visible means a lot of menus don't get updated. This just updates all the menu items.

The current _updateSubmenu method calls update if it is visible but otherwise goes through the submenu items and then calls _updateSubmenu on those. So just calling update will do the same thing but without testing for _isVisible.

[fredkiefer]

What you describe is correct. But this change was done on purpose. The old implementation was just as you describe as the new aim. The problem with that was at the time that updating invisible menu items used up to much time in some applications and so the idea was to save some time by only looking at visible items (which because of detached submenus isn't as easy as it seems). If we decide to do away with this optimization, we should complete remove the _updateSubmenu method. But first somebody has to dig through the mailing list archives to find out, which applications where causing issues in 2016 and check these whether the problem persists.

[fredkiefer]

I just forgot to ask: What is the issue you are trying to solve with your change? Where are updates lost and how does this matter if these items aren't visible?

[williameveretteggplant]

What we are seeing is that menus are simply never being updated. Whenever the update is occurring, apparently the menus aren't visible.

[rfm]

Does't that suggest a bug in the visibility state on windows... Which could presumably effect other things. If so, the correct solution would be to fix the underlying bug rather than removing code that exposes the bug.

[williameveretteggplant]

Well, no. What I am seeing is the update is always called when the menus are not displayed. The menus are only displayed when you click on them, then they go away again. It would make sense to update the menus every time you click on the main menu item and it pops up, but I cannot find anywhere the code that actually does that, (at least not with WinUXTheme). I think maybe it is being done by Windows native code. The update is also called after an action is invoked from a menu, but again, the first thing that happens when you click on a menu option is that the menu disappears.

[fredkiefer]

This sounds like a bug in the WinUXTheme. Normally a submenu gets displayed via a call to the -display (or -displayTransient) method and there is a call to -update. I am not familiar with that theme, but it should implement a similar logic. And as Richard wrote, it is better to solve the root cause and not provide a workaround that breaks other stuff.

[williameveretteggplant]

I have looked into it. On windows when using WinUXTheme, a click on one of the main menu options results in calls to various processing methods in WIN32Server (libs-back) and then some low level Windows calls (user32.dll). Neither display or displayTransient is called. It looks like the opening of a window like this is handled with native Windows calls.

[gcasa]

I have looked into it. On windows when using WinUXTheme, a click on one of the main menu options results in calls to various processing methods in WIN32Server (libs-back) and then some low level Windows calls (user32.dll). Neither display or displayTransient is called. It looks like the opening of a window like this is handled with native Windows calls.

The menus using the WinUXTheme are generated by recursively walking the NSMenu data structure and constructing an HMENU attached to the window. This is using native windows calls. Otherwise the windows theme wouldn't blend very well. So a lot of the normal calls on NSMenu won't be made when the native menus are used.

Similarly open, save, and print panels are delegated to their native counterparts.

[rfm]

So presumably the fix to the theme would be, in response to the mouse click event, to mark the menu visible and update it before creating and displaying the hmenu.

…

On 17 Oct 2024 at 02:02, Gregory Casamento ***@***.***> wrote: > > > I have looked into it. On windows when using WinUXTheme, a click on one of the main menu options results in calls to various processing methods in WIN32Server (libs-back) and then some low level Windows calls (user32.dll). Neither display or displayTransient is called. It looks like the opening of a window like this is handled with native Windows calls. > > The menus using the WinUXTheme are generated by recursively walking the NSMenu data structure and constructing an HMENU attached to the window. This is using native windows calls. Otherwise the windows theme wouldn't blend very well. So a lot of the normal calls on NSMenu won't be made when the native menus are used. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: ***@***.***>

[fredkiefer]

I had a short look at the code in this theme and it looks like the method -updateMenu:forWindow: is missing an -update call on the menu. It might be that this is not enough. I did not see any code that gets called when the menu is opened.
Sorry, I had a longer look and this idea is wrong. The method -updateMenu:forWIndow: gets called from -updateAllWindowsWithMenu and this from the NSMenu -update method. So this idea would create some kind of loop.
Looks like we need to hook into the MS Windows mechanism of opening a menu and just set the enabled state according to the updated menu item.

[williameveretteggplant]

I agree it would be good to hook into the MS Windows mechanism. The problem is, I don't see any way to do that. Tracing the execution, it gets to [WIN32Server windowEventProc:::] with the uMsg of WM_NCLBUTTONDOWN. It then calls DefWindowProcW() at WIN32Server.m : 1537. From there on it's pretty much in Windows land. There's no way at that point to know the user is clicking on a Menu, short of some elaborate reverse engineering of the Windows codes and windows.

Perhaps there is some way to set up a callback with Windows APIs, but that's pretty far outside my knowledge base.

[fredkiefer]

According to the MS Window documentation (I haven't programmed for this environment for almost twenty years) we should handle the WM_INITMENUPOPUP message. Currently neither WIN32Server nor WinUXTheme handle this at the moment. It would be great if somebody could test whether this is the correct place to address this issue. A simple way for that would be to add a log message to WIN32Server -windowEventProc::: for this message and check whether they are displayed for the correct actions. After that we could discuss the best way to implement a solution based on this.

[johnathan-becker]

According to the MS Window documentation (I haven't programmed for this environment for almost twenty years) we should handle the WM_INITMENUPOPUP message. Currently neither WIN32Server nor WinUXTheme handle this at the moment. It would be great if somebody could test whether this is the correct place to address this issue. A simple way for that would be to add a log message to WIN32Server -windowEventProc::: for this message and check whether they are displayed for the correct actions. After that we could discuss the best way to implement a solution based on this.

So I have tested this and whenever you open a Menu, this event gets triggered. I am not too familiar with libs-back so I'm not sure what to do with this information.

[johnathan-becker]

According to the MS Window documentation (I haven't programmed for this environment for almost twenty years) we should handle the WM_INITMENUPOPUP message. Currently neither WIN32Server nor WinUXTheme handle this at the moment. It would be great if somebody could test whether this is the correct place to address this issue. A simple way for that would be to add a log message to WIN32Server -windowEventProc::: for this message and check whether they are displayed for the correct actions. After that we could discuss the best way to implement a solution based on this.

So I have tested this and whenever you open a Menu, this event gets triggered. I am not too familiar with libs-back so I'm not sure what to do with this information.

@fredkiefer Just following up on this comment. Wondering if you could lend any help here.

[fredkiefer]

As I wrote, I haven't programmed on Windows in ages. But looking at the code we have in the Windows backend I suggest that you add a new method
- (void) decodeWM_INITMENUPOPUPParams: (WPARAM)wParam : (LPARAM)lParam : (HWND)hwnd
and there you could either send a new message to the theme [[GSTheme theme] something]; and provide an empty default implementation for that method in gui on GSTheme overriding it in the Windows nativer theme or try to get the window via the hwnd and get the menu of that window and send an -update to that directly. The later option looks a bit wrong, although it might be simpler to implement, as it will update the menu even when not using the Windows native theme. A combination might be best, where you pass on the GNUstep window to the theme.

[qmfrederik]

@williameveretteggplant I think we fixed this via gnustep/plugins-themes-WinUXTheme#5, gnustep/plugins-themes-WinUXTheme#9 and #307. Can you confirm, or is there anything else that's missing?

[johnathan-becker]

@fredkiefer I just tested this and It does not seem to have fixed the issue.Only this MR does.

[williameveretteggplant]

I believe the change to libs-gui has to be paired with a change to WinUX theme which causes the window to always return visible as true. I don't think this has been implemented yet.

[johnathan-becker]

I believe the change to libs-gui has to be paired with a change to WinUX theme which causes the window to always return visible as true. I don't think this has been implemented yet.

You are correct. I have put up this MR for this issue gnustep/plugins-themes-WinUXTheme#10

[johnathan-becker]

@williameveretteggplant I believe this MR can be closed.