Hi, On 03/27/2013 01:41 PM, Marc-André Lureau wrote:
On Wed, Mar 27, 2013 at 11:54 AM, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: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.That's what I did first (minus the fact that we don't need the else block)- 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); }But here was my reasoning: there should never be only a mouse grab.
This is not true, in applications which disable the keyboard-grab property, such as spicy potentially does, we can have a mouse grab without a keyboard grab. And this is a fine example of how this violates the principle of least surprise / makes try_keyboard_ungrab have potentially undesirable side-effects. Really a function named try_keyboard_ungrab, should not touch the mouse_grab, it is as simple as that. Regards, Hans _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel