Re: [PATCH] Grab keyboard based on focus in windows client

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,


On 06/05/2017 02:11 PM, Victor Toso wrote:
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?

Yes, maybe, i guess sometimes this grab_broken is needed, just not sure when..


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?

That's weird, works on my win7 client, if it doesn't set the focus this solution shouldn't work at all.
See if mouse hovering between windows changes the focus.


  #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@xxxxxxxxxx>

Thanks for your review :)


      release_keys(display);
      update_keyboard_focus(display, false);
--
2.9.3

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]