[Spice-devel] [spice-gtk] widget: avoid gdk_seat_grab/ungrab() API temporarily

[Pavel Grunt]

The intention of those commits was to be ready for new things (wayland,
gtk4... wait, actually gtk3). That side effect was not desired...
I'm not sure whether is better to revert it completely and go back to gtk2
api.

Ack,
Pavel

If there is a bug/regression in gtk+, it'd be good to have reference to it
in the commit ;)

Maybe it's easier for you to come up with a simple gtk reproducer since you
analyzed the issue properly.
If the comment is still valid, pity that the gtk+ bug did not get any
reaction (in the bug report).


2018-05-22 8:42 GMT+02:00 Victor Toso <victortoso at redhat.com>:

> From: Victor Toso <me at victortoso.com>
>
> We are using those APIs to avoid the deprecated ones from GTK but
> unexpected events are handled different by them introducing a
> regression that is easy to reproduce when Spice client holds the
> focus and user locks the screen.
>
> We need to better handle the situation where gdk_seat_grab() fails,
> specifically with GDK_GRAB_ALREADY_GRABBED which is the situation
> for the bugs mentioned below. The grab does not fail with older API
> in the same situation.
>
> Note that gdk_seat_grab() returning GDK_GRAB_ALREADY_GRABBED is due
> the locking screen holding the keyboard device which also means that
> remote-viewer isn't visible/show but gdk_window_is_visible() returns
> true till gdk_seat_grab() is called. The failure in gtk+ function
> call sets the visibility to false and the next gdk_seat_grab() call
> does return GDK_GRAB_NOT_VIEWABLE and gdk_window_is_visible() false
> as it should.
>
> So, gtk+ needs to fix some behavior and spice-gtk improve error
> handle but till then, let's avoid the regression for users.
>
> This patch basically avoid the changes introduced by following
> commits:
>
>  | commit 283f41cd289084689fbdf1151da55aa451ba343c
>  | gtk: Use gdk_window_get_device_position
>  |
>  | commit a718aec66658ba6bb3bb45a9af81a3aa2a652d18
>  | widget: Use deprecated function to ungrab device
>  |
>  | commit 3f4c5bcc88ca5db125ec48ebf696cb23a8e6339a
>  | gtk: Avoid deprecated gdk_keyboard_grab
>  |
>  | commit ef7a6fa1798c8e53c0b77330b398a78181cf90e5
>  | gtk: Avoid deprecated gdk_pointer_grab
>
> Related: https://bugzilla.redhat.com/show_bug.cgi?id=1485968
> Related: https://bugzilla.redhat.com/show_bug.cgi?id=1571422
>
> Signed-off-by: Victor Toso <victortoso at redhat.com>
> ---
>  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 72f5334..ec6a197 100644
> --- a/src/spice-widget.c
> +++ b/src/spice-widget.c
> @@ -811,7 +811,7 @@ SpiceGrabSequence *spice_display_get_grab_keys(SpiceDisplay
> *display)
>      return d->grabseq;
>  }
>
> -#if GTK_CHECK_VERSION(3, 20, 0)
> +#if 0 && GTK_CHECK_VERSION(3, 20, 0)
>  static GdkSeat *spice_display_get_default_seat(SpiceDisplay *display)
>  {
>      GdkWindow *window = gtk_widget_get_window(GTK_WIDGET(display));
> @@ -867,7 +867,7 @@ static void try_keyboard_grab(SpiceDisplay *display)
>                                              GetModuleHandle(NULL), 0);
>      g_warn_if_fail(d->keyboard_hook != NULL);
>  #endif
> -#if GTK_CHECK_VERSION(3, 20, 0)
> +#if 0 && GTK_CHECK_VERSION(3, 20, 0)
>      status = gdk_seat_grab(spice_display_get_default_seat(display),
>                             gtk_widget_get_window(widget),
>                             GDK_SEAT_CAPABILITY_KEYBOARD,
> @@ -892,7 +892,7 @@ static void try_keyboard_grab(SpiceDisplay *display)
>  static void ungrab_keyboard(G_GNUC_UNUSED SpiceDisplay *display)
>  {
>      G_GNUC_BEGIN_IGNORE_DEPRECATIONS
> -#if GTK_CHECK_VERSION(3, 20, 0)
> +#if 0 && GTK_CHECK_VERSION(3, 20, 0)
>      /* we want to ungrab just the keyboard - it is not possible using
> gdk_seat_ungrab().
>         See also https://bugzilla.gnome.org/show_bug.cgi?id=780133 */
>      GdkDevice *keyboard = gdk_seat_get_keyboard(spice_
> display_get_default_seat(display));
> @@ -1042,7 +1042,7 @@ static gboolean do_pointer_grab(SpiceDisplay
> *display)
>
>      try_keyboard_grab(display);
>      G_GNUC_BEGIN_IGNORE_DEPRECATIONS
> -#if GTK_CHECK_VERSION(3, 20, 0)
> +#if 0 && GTK_CHECK_VERSION(3, 20, 0)
>      status = gdk_seat_grab(spice_display_get_default_seat(display),
>                             window,
>                             GDK_SEAT_CAPABILITY_ALL_POINTING,
> @@ -1174,7 +1174,7 @@ static void mouse_wrap(SpiceDisplay *display,
> GdkEventMotion *motion)
>  static void ungrab_pointer(G_GNUC_UNUSED SpiceDisplay *display)
>  {
>      G_GNUC_BEGIN_IGNORE_DEPRECATIONS
> -#if GTK_CHECK_VERSION(3, 20, 0)
> +#if 0 && GTK_CHECK_VERSION(3, 20, 0)
>      /* we want to ungrab just the pointer - it is not possible using
> gdk_seat_ungrab().
>         See also https://bugzilla.gnome.org/show_bug.cgi?id=780133 */
>      GdkDevice *pointer = gdk_seat_get_pointer(spice_
> display_get_default_seat(display));
> @@ -2431,7 +2431,7 @@ static GdkDevice *spice_gdk_window_get_pointing_device(GdkWindow
> *window)
>  {
>      GdkDisplay *gdk_display = gdk_window_get_display(window);
>      G_GNUC_BEGIN_IGNORE_DEPRECATIONS
> -#if GTK_CHECK_VERSION(3, 20, 0)
> +#if 0 && GTK_CHECK_VERSION(3, 20, 0)
>      return gdk_seat_get_pointer(gdk_display_get_default_seat(gdk_
> display));
>  #else
>      return gdk_device_manager_get_client_pointer(gdk_display_get_
> device_manager(gdk_display));
> --
> 2.17.0
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180522/9fa9763b/attachment-0001.html>