In the "prefs cause code complexity" department, here's a "sloppy focus

2003-10-13  Havoc Pennington  <hp@redhat.com>

        In the "prefs cause code complexity" department, here's a "sloppy
	focus die die die" kind of moment.

	* src/display.c (meta_display_grab_focus_window_button): don't
	grab in sloppy focus mode, since we were dropping the grab on
	window enter anyway this just removes races from the current
	behavior.

	* src/display.c (prefs_changed_callback): ungrab/grab on focus
	mode changes, since we treat sloppy and click differently.

5 files changed, 92 insertions, 24 deletions

diff --git a/ChangeLog b/ChangeLog
index ab710bfa..045531aa 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2003-10-13  Havoc Pennington  <hp@redhat.com>
+
+        In the "prefs cause code complexity" department, here's a "sloppy
+	focus die die die" kind of moment.
+
+	* src/display.c (meta_display_grab_focus_window_button): don't
+	grab in sloppy focus mode, since we were dropping the grab on
+	window enter anyway this just removes races from the current
+	behavior.
+
+	* src/display.c (prefs_changed_callback): ungrab/grab on focus
+	mode changes, since we treat sloppy and click differently.
+
 2003-10-11  Havoc Pennington  <hp@pobox.com>
 
 	* src/display.c (meta_display_begin_grab_op): initialize
diff --git a/src/display.c b/src/display.c
index c0986769..b1eb1b22 100644
--- a/src/display.c
+++ b/src/display.c
@@ -2,7 +2,7 @@
 
 /* 
  * Copyright (C) 2001 Havoc Pennington
- * Copyright (C) 2002 Red Hat, Inc.
+ * Copyright (C) 2002, 2003 Red Hat, Inc.
  * 
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
@@ -1335,7 +1335,12 @@ event_callback (XEvent   *event,
                    * frames.c or special-cased if the click was on a
                    * minimize/close button.
                    */
-                  if (meta_prefs_get_focus_mode () == META_FOCUS_MODE_CLICK)
+
+                  /* Raise on clicking the client area always or only
+                   * in click to focus mode? The debate rages.
+                   * Feel free to change TRUE to FALSE
+                   */
+                  if (TRUE /* meta_prefs_get_focus_mode () == META_FOCUS_MODE_CLICK */) 
                     {
                       meta_window_raise (window);
                       
@@ -1424,6 +1429,10 @@ event_callback (XEvent   *event,
                 mode = AsyncPointer; /* eat focus click */
               else
                 mode = ReplayPointer; /* give event back */
+
+              meta_verbose ("Allowing events mode %s time %lu\n",
+                            mode == AsyncPointer ? "AsyncPointer" : "ReplayPointer",
+                            (unsigned long) event->xbutton.time);
               
               XAllowEvents (display->xdisplay,
                             mode, event->xbutton.time);
@@ -3031,6 +3040,11 @@ meta_change_button_grab (MetaDisplay *display,
                          int          modmask)
 {
   unsigned int ignored_mask;
+
+  meta_verbose ("%s 0x%lx sync = %d button = %d modmask 0x%x\n",
+                grab ? "Grabbing" : "Ungrabbing",
+                xwindow,
+                sync, button, modmask);
   
   meta_error_trap_push (display);
   
@@ -3048,6 +3062,8 @@ meta_change_button_grab (MetaDisplay *display,
 
       if (meta_is_debugging ())
         meta_error_trap_push_with_return (display);
+
+      /* GrabModeSync means freeze until XAllowEvents */
       
       if (grab)
         XGrabButton (display->xdisplay, button, modmask | ignored_mask,
@@ -3149,11 +3165,29 @@ meta_display_ungrab_window_buttons  (MetaDisplay *display,
 #define MAX_FOCUS_BUTTON 4
 void
 meta_display_grab_focus_window_button (MetaDisplay *display,
-                                       Window       xwindow)
+                                       MetaWindow  *window)
 {
   /* Grab button 1 for activating unfocused windows */
-  meta_verbose ("Grabbing unfocused window buttons for 0x%lx\n", xwindow);
+  meta_verbose ("Grabbing unfocused window buttons for %s\n", window->desc);
 
+  /* Don't grab at all unless in click to focus mode. In click to
+   * focus, we may sometimes be clever about intercepting and eating
+   * the focus click. But in mouse focus, we never do that since the
+   * focus window may not be raised, and who wants to think about
+   * mouse focus anyway.
+   */
+  if (meta_prefs_get_focus_mode () != META_FOCUS_MODE_CLICK)
+    {
+      meta_verbose (" (well, not grabbing since not in click to focus mode)\n");
+      return;
+    }
+  
+  if (window->have_focus_click_grab)
+    {
+      meta_verbose (" (well, not grabbing since we already have the grab)\n");
+      return;
+    }
+  
   /* FIXME If we ignored errors here instead of spewing, we could
    * put one big error trap around the loop and avoid a bunch of
    * XSync()
@@ -3164,29 +3198,37 @@ meta_display_grab_focus_window_button (MetaDisplay *display,
     while (i < MAX_FOCUS_BUTTON)
       {
         meta_change_button_grab (display,
-                                 xwindow,
-                                 TRUE, TRUE, i, 0);
+                                 window->xwindow,
+                                 TRUE, TRUE,
+                                 i, 0);
         
         ++i;
       }
+
+    window->have_focus_click_grab = TRUE;
   }
 }
 
 void
 meta_display_ungrab_focus_window_button (MetaDisplay *display,
-                                         Window       xwindow)
+                                         MetaWindow  *window)
 {
-  meta_verbose ("Ungrabbing unfocused window buttons for 0x%lx\n", xwindow);
+  meta_verbose ("Ungrabbing unfocused window buttons for %s\n", window->desc);
 
+  if (!window->have_focus_click_grab)
+    return;
+  
   {
     int i = 1;
     while (i < MAX_FOCUS_BUTTON)
       {
-        meta_change_button_grab (display, xwindow,
-                                 FALSE, TRUE, i, 0);
+        meta_change_button_grab (display, window->xwindow,
+                                 FALSE, FALSE, i, 0);
         
         ++i;
       }
+
+    window->have_focus_click_grab = FALSE;
   }
 }
 
@@ -4010,12 +4052,17 @@ static void
 prefs_changed_callback (MetaPreference pref,
                         void          *data)
 {
-  if (pref == META_PREF_MOUSE_BUTTON_MODS)
+  /* It may not be obvious why we regrab on focus mode
+   * change; it's because we handle focus clicks a
+   * bit differently for the different focus modes.
+   */
+  if (pref == META_PREF_MOUSE_BUTTON_MODS ||
+      pref == META_PREF_FOCUS_MODE)
     {
       MetaDisplay *display = data;
       GSList *windows;
       GSList *tmp;
-
+      
       windows = meta_display_list_windows (display);
       
       /* Ungrab all */
@@ -4024,19 +4071,20 @@ prefs_changed_callback (MetaPreference pref,
         {
           MetaWindow *w = tmp->data;
           meta_display_ungrab_window_buttons (display, w->xwindow);
-          meta_display_ungrab_focus_window_button (display, w->xwindow);
+          meta_display_ungrab_focus_window_button (display, w);
           tmp = tmp->next;
         }
-      
+
       /* change our modifier */
-      update_window_grab_modifiers (display);
+      if (pref == META_PREF_MOUSE_BUTTON_MODS)
+        update_window_grab_modifiers (display);
 
       /* Grab all */
       tmp = windows;
       while (tmp != NULL)
         {
           MetaWindow *w = tmp->data;
-          meta_display_grab_focus_window_button (display, w->xwindow);
+          meta_display_grab_focus_window_button (display, w);
           meta_display_grab_window_buttons (display, w->xwindow);
           tmp = tmp->next;
         }
diff --git a/src/display.h b/src/display.h
index f96253d6..3d098a1a 100644
--- a/src/display.h
+++ b/src/display.h
@@ -369,9 +369,9 @@ void     meta_display_ungrab_window_buttons  (MetaDisplay *display,
                                               Window       xwindow);
 
 void meta_display_grab_focus_window_button   (MetaDisplay *display,
-                                              Window       xwindow);
+                                              MetaWindow  *window);
 void meta_display_ungrab_focus_window_button (MetaDisplay *display,
-                                              Window       xwindow);
+                                              MetaWindow  *window);
 
 /* make a request to ensure the event serial has changed */
 void     meta_display_increment_event_serial (MetaDisplay *display);
diff --git a/src/window.c b/src/window.c
index 9bb5b3e5..38d6e8f8 100644
--- a/src/window.c
+++ b/src/window.c
@@ -427,6 +427,7 @@ meta_window_new (MetaDisplay *display,
   window->initial_workspace_set = FALSE;
   window->calc_placement = FALSE;
   window->shaken_loose = FALSE;
+  window->have_focus_click_grab = FALSE;
   
   window->unmaps_pending = 0;
   
@@ -556,7 +557,7 @@ meta_window_new (MetaDisplay *display,
 
   meta_window_grab_keys (window);
   meta_display_grab_window_buttons (window->display, window->xwindow);
-  meta_display_grab_focus_window_button (window->display, window->xwindow);
+  meta_display_grab_focus_window_button (window->display, window);
   
   /* For the workspace, first honor hints,
    * if that fails put transients with parents,
@@ -982,7 +983,7 @@ meta_window_free (MetaWindow  *window)
 
   meta_window_ungrab_keys (window);
   meta_display_ungrab_window_buttons (window->display, window->xwindow);
-  meta_display_ungrab_focus_window_button (window->display, window->xwindow);
+  meta_display_ungrab_focus_window_button (window->display, window);
   
   meta_display_unregister_x_window (window->display, window->xwindow);
   
@@ -3994,8 +3995,11 @@ meta_window_notify_focus (MetaWindow *window,
           /* move into FOCUSED_WINDOW layer */
           meta_window_update_layer (window);
 
-          /* Ungrab click to focus button */
-          meta_display_ungrab_focus_window_button (window->display, window->xwindow);
+          /* Ungrab click to focus button since the sync grab can interfere
+           * with some things you might do inside the focused window, by
+           * causing the client to get funky enter/leave events.
+           */
+          meta_display_ungrab_focus_window_button (window->display, window);
         }
     }
   else if (event->type == FocusOut ||
@@ -4033,8 +4037,8 @@ meta_window_notify_focus (MetaWindow *window,
           /* move out of FOCUSED_WINDOW layer */
           meta_window_update_layer (window);
 
-          /* Re-grab for click to focus */
-          meta_display_grab_focus_window_button (window->display, window->xwindow);
+          /* Re-grab for click to focus, if necessary */
+          meta_display_grab_focus_window_button (window->display, window);
         }
     }
 
diff --git a/src/window.h b/src/window.h
index 3b36872e..6c6301d3 100644
--- a/src/window.h
+++ b/src/window.h
@@ -240,6 +240,9 @@ struct _MetaWindow
   /* if TRUE, window was maximized at start of current grab op */
   guint shaken_loose : 1;
 
+  /* if TRUE we have a grab on the focus click buttons */
+  guint have_focus_click_grab : 1;
+  
 #ifdef HAVE_XSYNC
   /* XSync update counter */
   XSyncCounter update_counter;