[Spice-devel] [PATCH] Grab keyboard based on focus in windows client

[Victor Toso]

Hi,

Small note, I would for 72 of length in commit message. Seems to be 55.

On Thu, Jun 01, 2017 at 06:44:18PM +0300, Snir Sheriber wrote:
> Currently the client grabs keyboard based on session
> focus, on windows client it generates grab_broken
> event.
> This patch expands the solution presented in 143ebfd
> to work also on windows client without causing
> grab_broken event.
> This is implemented a bit differently from linux, if
> on mouse entrance session already has focus, focus
> will be set to the current window, this will generate
> focus events that will release and grab the focus again
> without grab_broken event.

I would also add the note you did in the cover-letter about it:

 | [2-more-info] The issues is that on windows gtk will generate grab
 | broken if wm_killfocus received (when focus is changed) while the
 | application has the grab. which it means that if we have the grab on
 | hover without getting the focus first (i.e by clicking) it will
 | cause grab_broken which follows by no grab at all, gtk does this on
 | purpose, i'm not sure why it is good for.

Btw, it might be good to file a bug against gtk about this, no?

> Resolves: rhbz#1429611
> Related: rhbz#1275231
> ---
>  src/spice-widget.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/src/spice-widget.c b/src/spice-widget.c
> index 1a1d5a6..b3407b5 100644
> --- a/src/spice-widget.c
> +++ b/src/spice-widget.c
> @@ -850,10 +850,17 @@ static void try_keyboard_grab(SpiceDisplay *display)
>          return;
>      if (d->keyboard_grab_active)
>          return;
> +#ifdef G_OS_WIN32
> +    if (!d->keyboard_have_focus)
> +        return;
> +    if (!d->mouse_have_pointer)
> +        return;
> +#else
>      if (!spice_gtk_session_get_keyboard_has_focus(d->gtk_session))
>          return;
>      if (!spice_gtk_session_get_mouse_has_pointer(d->gtk_session))
>          return;
> +#endif
>      if (d->keyboard_grab_released)
>          return;
>
> @@ -1543,6 +1550,12 @@ static void update_display(SpiceDisplay *display)
>      win32_window = display ?
>                          gdk_win32_window_get_impl_hwnd(gtk_widget_get_window(GTK_WIDGET(display))) :
>                          NULL;
> +    if(win32_window) {
> +        SpiceDisplayPrivate *d = display->priv;
> +        if(spice_gtk_session_get_keyboard_has_focus(d->gtk_session) &&
> +           spice_gtk_session_get_mouse_has_pointer(d->gtk_session))
> +            SetFocus(win32_window);
> +    }

Hmm, I put a SPICE_DEBUG() after SetFocus() but the debug messages was
never printed. Windows 7.

That makes me think we don't really need to set the focus or it might
depend on gtk/windows version?

>  #endif
>  }
>
> @@ -1862,7 +1875,6 @@ static gboolean focus_in_event(GtkWidget *widget, GdkEventFocus *focus G_GNUC_UN
>  static gboolean focus_out_event(GtkWidget *widget, GdkEventFocus *focus G_GNUC_UNUSED)
>  {
>      SpiceDisplay *display = SPICE_DISPLAY(widget);
> -    SpiceDisplayPrivate *d = display->priv;
>
>      DISPLAY_DEBUG(display, "%s", __FUNCTION__);
>      update_display(NULL);
> @@ -1871,8 +1883,11 @@ static gboolean focus_out_event(GtkWidget *widget, GdkEventFocus *focus G_GNUC_U
>       * Ignore focus out after a keyboard grab
>       * (this happens when doing the grab from the enter_event callback).
>       */
> +#ifndef G_OS_WIN32
> +    SpiceDisplayPrivate *d = display->priv;
>      if (d->keyboard_grab_active)
>          return true;
> +#endif

Your solution works fine.

Reviewed-by: Victor Toso <victortoso at redhat.com>

>
>      release_keys(display);
>      update_keyboard_focus(display, false);
> -- 
> 2.9.3
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170605/61296a6f/attachment-0001.sig>