Alex Henrie alexhenrie24@gmail.com writes:
2016-12-02 11:41 GMT-07:00 Alexandre Julliard julliard@winehq.org:
Alex Henrie alexhenrie24@gmail.com writes:
Instead of adding WM_SYSKEYDOWN to the switch statement, we could fix the bug in one line by moving "pmt->trackFlags |= TF_SKIPREMOVE;" into the WM_KEYDOWN case. Or, we could delete MENU_SuspendPopup entirely and the if statements that call it. However, we wouldn't be setting TF_SKIPREMOVE anymore for unrecognized messages, likely causing a return of the bug that d75b0cdc tried to fix.
By the way, I'm not exactly reverting d75b0cdc. I changed PM_REMOVE to PM_NOREMOVE to make sure that messages are removed only if they are recognized.
I hope that makes sense; I admit that my previous explanation was not as well thought out as it could have been. Thanks for the feedback.
Sorry for the delay in reviewing this. It still doesn't quite make sense to me. Changing to PM_NOREMOVE now means that the keyboard message won't get removed if some other message happened in between. I'm not sure that's a good idea.
We definitely do not want to remove the keyboard message if some other message is in line before it. In fact, the PeekMessageW( &msg, 0, 0, 0, PM_NOYIELD | PM_NOREMOVE ) statements later in the function depend on the assumption that they are reading a message that follows the WM_KEYDOWN. They are not supposed to be looking at a message that precedes the WM_KEYDOWN like is possible in the current code.
At that point the keyboard message has already been handled, for instance by moving selection to the next item. So if you don't remove it, it will be handled twice.