Re: x11drv: (bug 4685) properly handle VK_LMENU input (resend)

[Dmitry Timoshkov]

<tkho(a)ucla.edu> wrote:

A couple of suggestions about coding style:
> +struct transition_s {
> +    BYTE wVk;

It would be more logical to use WORD for VK codes as everywhere else.
> +    BYTE before_state;
> +    BYTE _todo_wine;
> +};

Same for _todo_wine, it should be BOOL.
> +
> +struct sendinput_test_s {
> +    WORD wVk;
> +    DWORD dwFlags;
> +    struct transition_s expected_transitions[MAXKEYEVENTS+1];
> +    UINT expected_messages[MAXKEYMESSAGES+1];
> +} sendinput_test[] = {
> +    /* test ALT+F */
> +    {VK_LMENU, 0, {{0x12, 0x00, 0}, {0xa4, 0x00, 0}, {0}}, {0x104, 0}},
> +    {'F', 0, {{0x46, 0x00, 0}, {0}}, {0x104, 0x106, 0x112, 0}},
> +    {'F', KEYEVENTF_KEYUP, {{0x46, 0x80, 0}, {0}}, {0x105, 0}},
> +    {VK_LMENU, KEYEVENTF_KEYUP, {{0x12, 0x80, 0}, {0xa4, 0x80, 0}, {0}},
> +        {0x101, 0}},

Please use VK names and symbolic message names in the above arrays to make
them more readable.
> /***********************************************************************
>  *           X11DRV_send_keyboard_input
> @@ -1128,14 +1141,16 @@
>     {
>         message = WM_KEYUP;
>         if ((key_state_table[VK_MENU] & 0x80) &&
> -            ((wVk == VK_MENU) || (wVk == VK_CONTROL) || !(key_state_table[VK_CONTROL] & 0x80)))
> +            ((VkStripLR(wVk) == VK_MENU) || (VkStripLR(wVk) == VK_CONTROL)
> +             || !(key_state_table[VK_CONTROL] & 0x80)))
>         {
>             if( TrackSysKey == VK_MENU || /* <ALT>-down/<ALT>-up sequence */
> -                (wVk != VK_MENU)) /* <ALT>-down...<something else>-up */
> +                (VkStripLR(wVk) != VK_MENU)) /* <ALT>-down...<something else>-up */
>                 message = WM_SYSKEYUP;
>             TrackSysKey = 0;
>         }
>         key_state_table[wVk] &= ~0x80;
> +        key_state_table[VkStripLR(wVk)] &= ~0x80;
>         keylp.lp1.previous = 1;
>         keylp.lp1.transition = 1;
>     }
> @@ -1145,12 +1160,13 @@
>         keylp.lp1.transition = 0;
>         if (!(key_state_table[wVk] & 0x80)) key_state_table[wVk] ^= 0x01;
>         key_state_table[wVk] |= 0xc0;
> +        key_state_table[VkStripLR(wVk)] |= 0xc0;
>
> message = WM_KEYDOWN;
>         if ((key_state_table[VK_MENU] & 0x80) && !(key_state_table[VK_CONTROL] & 0x80))
>         {
>             message = WM_SYSKEYDOWN;
> -            TrackSysKey = wVk;
> +            TrackSysKey = VkStripLR(wVk);

It would be much simpler to call VkStripLR() only once and use the result
for further processing.

-- 
Dmitry.