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

[Dmitry Timoshkov]

 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.