WinUXTheme: Override menu visibility

[qmfrederik]

The default implementation of [NSMenu _isVisible] will use the _aWindow and _bWindow windows. However, WinUXTheme does not use these windows, and the value of _isVisible will be incorrect. Override the value by implementing proposedVisibility: forMenu, and always return TRUE.

[qmfrederik]

This is the WinUXTheme counter-part of gnustep/libs-gui#307. Together with #5 (which got merged, thanks), this fixes menu visibility on Windows.

[qmfrederik]

@fredkiefer You suggested in gnustep/libs-gui#304 (comment) to use WM_INITMENUPOPUP to determine the visibility of a menu item. This notification is sent when a menu item is about to be displayed, but I couldn't find a similar notification when a menu item would disappear. Similarly, I couldn't find a Win32 API which allows you to query the state of an individual menu item.

Therefore, I'd suggest we always return TRUE. We haven't found this to be a performance problem on Windows (at least in our use case). This fixes the issue of menu items not being updated correctly on Windows.

Should someone find this to be a performance concern, they could always come back and update this code? Would you be OK with this approach?

[fredkiefer]

There is a slight misunderstanding here. I suggested to use WM_INITMENUPOPUP not to determine visibility but as the place where the update of the menu should happen. I was not aware that there isn't a way to determine whether a menu was being displayed and I am still a bit confused about that.

[qmfrederik]

Thanks @fredkiefer.

Yes, gave that approach try. Like you suggested (gnustep/libs-gui#304 (comment)), I could:

1. Intercept the WM_INITMENUPOPUP message in [WIN32Server windowEventProc::::]
2. Have that call a new [WIN32Server decodeWM_INITMENUPOPUPParams] method
3. Have that call a new [GSTheme processInitMenuPopUp] method with a default empty implementation
4. Override that implementation in WinUXTheme and call, say [[NSApp mainMenu] update] for testing purposes

That still leaves the issue that there are a fair amount of isVisible checks embedded in NSMenu. As far as I know, there isn't a way to reliable determine that value on Win32, so isVisible would always have to return true, and the current code would just work.

As I understand it, the check for isVisible is a performance optimization. I haven't seen an example of the performance issue we're optimizing for on Win32. We're working our way through other issues we're seeing on Win32, so I was hoping we could get this change merged (which fixes a real issue on Win32) and work our way through the other issues first?

[qmfrederik]

@fredkiefer Can you let me know whether there's anything else you need from me on this PR? We've tested this with our application and this fixes the bug we've observed (which was that menus were disabled). We don't see a performance impact and could follow up with a separate PR should there be performance constraints. If I don't hear back from you, I'll go ahead and merge this next week.

[fredkiefer]

Apart from that, feel free to merge.