[PATCH] user32: Handle WM_SYSKEYDOWN in MENU_SuspendPopup.

[Alex Henrie]

2016-12-05 3:42 GMT-07:00 Alexandre Julliard <julliard at winehq.org>:
> 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.

Good point. I'll try again.

-Alex