[Spice-devel] [spice-gtk] Set windows hook only when keyboard is grabbed

[Snir Sheriber]

Hi


On 05/16/2017 10:57 AM, Pavel Grunt wrote:
> Hi Snir,
>
> On Mon, 2017-05-15 at 19:02 +0300, Snir Sheriber wrote:
>> The keyboard events hook should be set only if keyboard is
>> grabbed otherwise the hook won't behave as expected, e.g alt-tab
>> may cause duplicate event in the windows manager and not releasing
>> the alt after key was released
> Great that you have found the issue! Do you know why the grab fails?
> Is the behavior documented anywhere? Probably it is worth adding a
> comment into the code.
>
> Please add a bug reference.
I found it when i worked on the other bug but that's not really
related to it, i just notice that weird things are happening if the
hook is registered while we don't have grab.
Which is also make some sense because this hook meant to intercept
some low level keyboard events that were not received on windows
client , so if for some reason widget doesn't have keyboard grab it
probably shouldn't supposed to handle any of the keyboard events.


when I'll send the bug related patch I'll mention it :)

Snir.

>
> Thanks,
> Pavel
>> ---
>>   src/spice-widget.c | 12 ++++++------
>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/spice-widget.c b/src/spice-widget.c
>> index 8203d55..e889bba 100644
>> --- a/src/spice-widget.c
>> +++ b/src/spice-widget.c
>> @@ -862,12 +862,6 @@ static void try_keyboard_grab(SpiceDisplay
>> *display)
>>       DISPLAY_DEBUG(display, "grab keyboard");
>>       gtk_widget_grab_focus(widget);
>>   
>> -#ifdef G_OS_WIN32
>> -    if (d->keyboard_hook == NULL)
>> -        d->keyboard_hook = SetWindowsHookEx(WH_KEYBOARD_LL,
>> keyboard_hook_cb,
>> -                                            GetModuleHandle(NULL),
>> 0);
>> -    g_warn_if_fail(d->keyboard_hook != NULL);
>> -#endif
>>   #if GTK_CHECK_VERSION(3, 20, 0)
>>       status = gdk_seat_grab(spice_display_get_default_seat(display),
>>                              gtk_widget_get_window(widget),
>> @@ -885,6 +879,12 @@ static void try_keyboard_grab(SpiceDisplay
>> *display)
>>           g_warning("keyboard grab failed %u", status);
>>           d->keyboard_grab_active = false;
>>       } else {
>> +#ifdef G_OS_WIN32
>> +        if (d->keyboard_hook == NULL)
>> +            d->keyboard_hook = SetWindowsHookEx(WH_KEYBOARD_LL,
>> keyboard_hook_cb,
>> +                                                GetModuleHandle(NUL
>> L), 0);
>> +        g_warn_if_fail(d->keyboard_hook != NULL);
>> +#endif
>>           d->keyboard_grab_active = true;
>>           g_signal_emit(widget, signals[SPICE_DISPLAY_KEYBOARD_GRAB],
>> 0, true);
>>       }