[PATCH] user32: Handle WM_SYSKEYDOWN in MENU_SuspendPopup.

[Alex Henrie]

2016-12-02 11:41 GMT-07:00 Alexandre Julliard <julliard at winehq.org>:
> Alex Henrie <alexhenrie24 at 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.

I made the function behave the way that it used to before d75b0cdc,
while being careful to not reintroduce the bug where non-keyboard
messages were accidentally removed.

-Alex