[Spice-devel,spice-gtk,08/10] spice-widget: ignore focus in / out events caused by keyb ungrab/grab

Submitted by Hans de Goede on Aug. 12, 2011, 9:50 p.m.

Details

Message ID 1313160637-5187-8-git-send-email-hdegoede@redhat.com
State New, archived
Headers show

Not browsing as part of any series.

Commit Message

Hans de Goede Aug. 12, 2011, 9:50 p.m.
As documented in XGrabKeyboard(3): "The XGrabKeyboard function actively grabs
control of the keyboard and generates FocusIn and FocusOut events."

Note that for some reason this only happens when we call XGrabKeyboard
from our enter_event / leave_event callbacks and not from our focus_in /
focus_out callbacks? Either way we still need to filter these out.

Filtering these out fixes 4 issues:
1) keyboard_have_focus now no longer gets unset when the keyboard is grabbed,
   making USB auto redirection when focussed actually work
2) Before this patch, if you pressed alt and then (accidentally) moved the
   cursor out of the spice-widget window before pressing a second key,
   the focus in event would clear the keyboard status causing the guest to no
   longer see alt as pressed and register the second key press as a regular
   keypress rather then as alt-foo.
3) It allows us to remove the keyboard_grab_count / keyboard_grab_time hack
   from try_keyboard_grab, although since we are no longer doing an
   ungrab on focus out, this likely could have been removed before.
   I will do this in a separate patch for easier reverting if necessary.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 gtk/spice-widget.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

Patch hide | download patch | download mbox

diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c
index c377489..30036ce 100644
--- a/gtk/spice-widget.c
+++ b/gtk/spice-widget.c
@@ -885,6 +885,14 @@  static gboolean focus_in_event(GtkWidget *widget, GdkEventFocus *focus G_GNUC_UN
     SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
 
     SPICE_DEBUG("%s", __FUNCTION__);
+
+    /*
+     * Ignore focus in when we already have the focus
+     * (this happens when doing an ungrab from the leave_event callback).
+     */
+    if (d->keyboard_have_focus)
+        return true;
+
     release_keys(display);
     sync_keyboard_lock_modifiers(display);
     d->keyboard_have_focus = true;
@@ -903,6 +911,14 @@  static gboolean focus_out_event(GtkWidget *widget, GdkEventFocus *focus G_GNUC_U
     SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
 
     SPICE_DEBUG("%s", __FUNCTION__);
+
+    /*
+     * Ignore focus out after a keyboard grab
+     * (this happens when doing the grab from the enter_event callback).
+     */
+    if (d->keyboard_grab_active)
+        return true;
+
     d->keyboard_have_focus = false;
     update_auto_usbredir(display);
     return true;

Comments

Looks good to me, though Marc-André knows this code a lot more than I do :)

Christophe

On Fri, Aug 12, 2011 at 04:50:35PM +0200, Hans de Goede wrote:
> As documented in XGrabKeyboard(3): "The XGrabKeyboard function actively grabs
> control of the keyboard and generates FocusIn and FocusOut events."
> 
> Note that for some reason this only happens when we call XGrabKeyboard
> from our enter_event / leave_event callbacks and not from our focus_in /
> focus_out callbacks? Either way we still need to filter these out.
> 
> Filtering these out fixes 4 issues:
> 1) keyboard_have_focus now no longer gets unset when the keyboard is grabbed,
>    making USB auto redirection when focussed actually work
> 2) Before this patch, if you pressed alt and then (accidentally) moved the
>    cursor out of the spice-widget window before pressing a second key,
>    the focus in event would clear the keyboard status causing the guest to no
>    longer see alt as pressed and register the second key press as a regular
>    keypress rather then as alt-foo.
> 3) It allows us to remove the keyboard_grab_count / keyboard_grab_time hack
>    from try_keyboard_grab, although since we are no longer doing an
>    ungrab on focus out, this likely could have been removed before.
>    I will do this in a separate patch for easier reverting if necessary.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  gtk/spice-widget.c |   16 ++++++++++++++++
>  1 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c
> index c377489..30036ce 100644
> --- a/gtk/spice-widget.c
> +++ b/gtk/spice-widget.c
> @@ -885,6 +885,14 @@ static gboolean focus_in_event(GtkWidget *widget, GdkEventFocus *focus G_GNUC_UN
>      SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
>  
>      SPICE_DEBUG("%s", __FUNCTION__);
> +
> +    /*
> +     * Ignore focus in when we already have the focus
> +     * (this happens when doing an ungrab from the leave_event callback).
> +     */
> +    if (d->keyboard_have_focus)
> +        return true;
> +
>      release_keys(display);
>      sync_keyboard_lock_modifiers(display);
>      d->keyboard_have_focus = true;
> @@ -903,6 +911,14 @@ static gboolean focus_out_event(GtkWidget *widget, GdkEventFocus *focus G_GNUC_U
>      SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
>  
>      SPICE_DEBUG("%s", __FUNCTION__);
> +
> +    /*
> +     * Ignore focus out after a keyboard grab
> +     * (this happens when doing the grab from the enter_event callback).
> +     */
> +    if (d->keyboard_grab_active)
> +        return true;
> +
>      d->keyboard_have_focus = false;
>      update_auto_usbredir(display);
>      return true;
> -- 
> 1.7.5.1
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel