Hi, On 03/26/2013 09:33 PM, Marc-André Lureau wrote:
On windows, the client receives a WM_KILLFOCUS event which generates solely a keyboard grab-broken event. This event is received when pressing ctrl-alt-del (to show up the task manager), and we need to release the pointer grab and clip region in this case for the client to be usable. This also clear the clipping region when the client pops-up a dialog. Since keyboard focus is a pre-requisite for pointer grab, it sounds logical to release the pointer grab if losing keyboard focus.
Hmm, I consider the adding of the try_mouse_ungrab(display) call to try_keyboard_ungrab(display) adding of an undesirable side effect, and thus breaking the principle of least surprise. IOW someone who does not know every detail of the code and adds a try_keyboard_ungrab(display) somewhere may end up being surprised that the mouse also get ungrabbed. See my comments below for a proposed alternative fix.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=857114 https://bugzilla.redhat.com/show_bug.cgi?id=922818 --- gtk/spice-widget.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c index fb8094e..324f5ee 100644 --- a/gtk/spice-widget.c +++ b/gtk/spice-widget.c @@ -477,15 +477,13 @@ static GdkCursor* get_blank_cursor(void) static gboolean grab_broken(SpiceDisplay *self, GdkEventGrabBroken *event, gpointer user_data G_GNUC_UNUSED) { - SpiceDisplayPrivate *d = self->priv; + SPICE_DEBUG("%s (implicit: %d, keyboard: %d)", __FUNCTION__, + event->implicit, event->keyboard); - SPICE_DEBUG("%s (%d)", __FUNCTION__, event->implicit); if (event->keyboard) { - d->keyboard_grab_active = false; - g_signal_emit(self, signals[SPICE_DISPLAY_KEYBOARD_GRAB], 0, false); + try_keyboard_ungrab(self); } else {
Instead of adding the try_mouse_ungrab call to try_keyboard_ungrab I would like to see the above block changed to: if (event->keyboard) { try_keyboard_ungrab(self); try_mouse_ungrab(self); } else { This should also fix the issue you're trying to fix. With the added advantage that it will release the mouse even if for some reason d->keyboard_grab_active is false when we get the grab_broken.
- d->mouse_grab_active = false; - g_signal_emit(self, signals[SPICE_DISPLAY_MOUSE_GRAB], 0, false); + try_mouse_ungrab(self); } return false; @@ -751,6 +749,9 @@ static void try_keyboard_ungrab(SpiceDisplay *display) #endif d->keyboard_grab_active = false; g_signal_emit(widget, signals[SPICE_DISPLAY_KEYBOARD_GRAB], 0, false); + + /* for consistency, there should not be only a mouse grab */ + try_mouse_ungrab(display); }
And then the call here can be dropped... Regards, Hans _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel