[Spice-devel] [spice-gtk 3/9] widget: Always hook keyboard on Windows

[Marc-André Lureau]

Hi

On Wed, Jun 8, 2016 at 1:10 PM, Frediano Ziglio <fziglio at redhat.com> wrote:
> This prevents some keyboard handling like IME processing to
> take place.

But it doesn't explain how it fixed it.

>
> This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1342984
> (beside removing the call from virt-viewer).
>
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  src/spice-widget.c | 30 ++++++++++++++++--------------
>  1 file changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/src/spice-widget.c b/src/spice-widget.c
> index c528614..a77fdf3 100644
> --- a/src/spice-widget.c
> +++ b/src/spice-widget.c
> @@ -716,7 +716,7 @@ void spice_display_set_grab_keys(SpiceDisplay *display, SpiceGrabSequence *seq)
>  #ifdef G_OS_WIN32
>  static LRESULT CALLBACK keyboard_hook_cb(int code, WPARAM wparam, LPARAM lparam)
>  {
> -    if  (win32_window && code == HC_ACTION && wparam != WM_KEYUP) {
> +    if  (win32_window && code == HC_ACTION) {

What is this change about?

>          KBDLLHOOKSTRUCT *hooked = (KBDLLHOOKSTRUCT*)lparam;
>          DWORD dwmsg = (hooked->flags << 24) | (hooked->scanCode << 16) | 1;
>
> @@ -799,12 +799,6 @@ static void try_keyboard_grab(SpiceDisplay *display)
>      SPICE_DEBUG("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
>      status = gdk_keyboard_grab(gtk_widget_get_window(widget), FALSE,
>                                 GDK_CURRENT_TIME);
>      if (status != GDK_GRAB_SUCCESS) {
> @@ -826,13 +820,6 @@ static void try_keyboard_ungrab(SpiceDisplay *display)
>
>      SPICE_DEBUG("ungrab keyboard");
>      gdk_keyboard_ungrab(GDK_CURRENT_TIME);
> -#ifdef G_OS_WIN32
> -    // do not use g_clear_pointer as Windows API have different linkage
> -    if (d->keyboard_hook) {
> -        UnhookWindowsHookEx(d->keyboard_hook);
> -        d->keyboard_hook = NULL;
> -    }
> -#endif
>      d->keyboard_grab_active = false;
>      g_signal_emit(widget, signals[SPICE_DISPLAY_KEYBOARD_GRAB], 0, false);
>  }
> @@ -1679,6 +1666,13 @@ static gboolean focus_in_event(GtkWidget *widget, GdkEventFocus *focus G_GNUC_UN
>
>      SPICE_DEBUG("%s", __FUNCTION__);
>
> +#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
> +
>      /*
>       * Ignore focus in when we already have the focus
>       * (this happens when doing an ungrab from the leave_event callback).
> @@ -1708,6 +1702,14 @@ static gboolean focus_out_event(GtkWidget *widget, GdkEventFocus *focus G_GNUC_U
>      SPICE_DEBUG("%s", __FUNCTION__);
>      update_display(NULL);
>
> +#ifdef G_OS_WIN32
> +    // do not use g_clear_pointer as Windows API have different linkage
> +    if (d->keyboard_hook) {
> +        UnhookWindowsHookEx(d->keyboard_hook);
> +        d->keyboard_hook = NULL;
> +    }
> +#endif
> +

Why did you move the Hook/Unhook outside of try_keyboard_grab/ungrab?
(here is a lot of conditions that are checked before grabing the
systems keys, for ex you don't want to take the grab just because you
have the focus, that would break cycling between apps with series of
alt-tab etc)


>      /*
>       * Ignore focus out after a keyboard grab
>       * (this happens when doing the grab from the enter_event callback).
> --
> 2.7.4
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



-- 
Marc-André Lureau