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

[Marc-André Lureau]

Hi

----- Original Message -----
> > 
> > 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?
> > 
> 
> Use the hook for all event, not just WM_KEYSYS*/W_KEYDOWN.
> Why KEYUP is not handled?
> 

It's related to 5f67178c9602ed43f83f35a6d3097eb137244493 (later fixed in 0fb74ae1). Please check you don't reopen any of those 2 bugs.

> > >          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)
> > 
> 
> To handle the key before the IME code kick in.

There I am lost ;), I hope somebody else can check this. I am worried about having the hook / system grab outside of the spice-gtk grab conditions.

> Yes, alt-tab is not working correctly. Easy to fix not using SendMessage
> for VK_TAB but calling CallNextHookEx.
> Are there any other special combinations which cause problems beside
> alt-tab ?

Best would be to go through the bugs we fixed (like the 2 earlier) and make sure no regressions are introduced. I don't have a list in mind, as I barely use Windows to fix this kind of bugs.

> > 
> > >      /*
> > >       * Ignore focus out after a keyboard grab
> > >       * (this happens when doing the grab from the enter_event callback).
> 
> I'm trying to remove the IME context from the single window, looks
> good, the only drawback is that the input method (like the console
> prompt) is stick with direct input but at least you can change keyboard
> (obviously not input method).
> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>