[PATCH] user32: Handle WM_SYSKEYDOWN in MENU_SuspendPopup.

[Alexandre Julliard]

Alex Henrie <alexhenrie24 at gmail.com> writes:

> 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.

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.

-- 
Alexandre Julliard
julliard at winehq.org