Menubar: fixes to Alt handling to match native

[vslavik]

There were some differences in handling of Alt in mcIsMenubarMessage() compared to native menu:

1. Typing with Alt+numpad (e.g. Alt+0171) was impossible, it only worked once.
2. Pressing Alt, then pressing a key that is not a menu hot key and releasing Alt activated the menu.

Correct behavior, implemented by this change, is to

1. Not eat Alt in WM_(SYS)KEYDOWN and let win32 process it further. This requires a corresponding change when leaving active menu with Alt in menubar_key_down() so that win32's internal Alt handling isn't left in an inconsistent state: without this matching change, pressing Alt twice to enter and leave the menubar, then typing would try to activate an accelerator (e.g. Alt, Alt, Space would show system menu).

2. Don't activate the menu, only hide accelerators, if any additional keys were pressed why Alt was held down. Releasing alt becomes WM_KEYUP instead of WM_SYSKEYUP, so account for that too.

To the best of my knowledge, things work correctly now without breaking anything, but I'm no win32 expert and it's all to easy to accidentally break something in input handling, so a careful review would very much be appreciated. I'm happy to make any changes you may want or clarify anything if I didn't manage to explain it clearly.

This PR was prompted by this bug report: https://github.com/vslavik/poedit/issues/333 (point 2 is something I found out during testing).

[mity]

To the best of my knowledge, things work correctly now without breaking anything, but I'm no win32 expert and it's all to easy to accidentally break something in input handling, so a careful review would very much be appreciated. I'm happy to make any changes you may want or clarify anything if I didn't manage to explain it clearly.

I don't think that win32 experience can help here unless someone has access to MS sources and can see exactly how the [ALT] is handled in DefWindowProc() and/or IsDialogMessage() (on different Window versions). We can only incrementally improve to the behavior whenever we detect a difference.

I'll try to look at this during the next week and do some testing on various Windows versions before I merge it.

Anyway, big thanks for it.

[mity]

@vslavik I tried to reproduce the issues with mixed results. I used adapted src/example-menubar.c (with an edit-box edit into the window, where I can type all those alt+keypad combinations):

• Win 7, Win 10: I did not reproduce at all.
• Win 2000: I reproduced once (both issues happened at the single run of the example). But after restarting the app, I was not able to reproduce anymore.

So I was more focusing on reviewing the related code, especially the function mcIsMenubarMessage().

I have found some problems which may be relevant to the issues you describe. In particular, there was likely no good guaranty that the delayed activation (activate_with_f10) could not be set for much longer then it should be. It could likely be set from WM_(SYS)KEYDOWN but not reset in WM_(SYS)KEYUP, if some of the conditions (e.g. state of <SHIFT>) changed in the mean time, potentially leaving the logic of mcIsMenubarMessage() in an inconsistent state.

I hope that should be fixed in #52, which treats it much more carefully and which also brings the behavior much closer to the native menu in few other cases I noticed during the testing.

With this PR, I am not sure whether it just does not try to live with the inconsistent state, hiding the real nature of the bug.

I would therefore appreciate if you can provide me some binary/installation of poedit which I can try to reproduce the issue; or test on your side whether #52 fixes it in your environment, and I would consider merging this (or some part of it) if #52 does not fix it all.

[vslavik]

Thanks for looking into it! I'll make a build that enables mCtrl menubar on all Windows version for you later today (I use it on Win10 only).

I'll comment on my testing over at #52 — it has some regressions/differences, but it does fix Alt+numpad problem.

Finally, regarding testing: I initially had some difficulties seeing the reported bug because I use Parallels on macOS to run Windows in a VM. On the off-chance that you do the same: Parallels has some weird keyboard handling and Alt behaves differently (manifested by holding Alt not underlining anything), so I had to connect a USB keyboard directly to the VM.

[mity]

Finally, regarding testing: I initially had some difficulties seeing the reported bug because I use Parallels on macOS to run Windows in a VM. On the off-chance that you do the same: Parallels has some weird keyboard handling and Alt behaves differently (manifested by holding Alt not underlining anything), so I had to connect a USB keyboard directly to the VM.

No, I tested on native Win 10, and the other systems in VirtualBox.

[mity]

Closing. Fixed through PR #52.