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.

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-12  Havoc Pennington  <hp@redhat.com>
 
         Merge reduced_resources mode patch from the branch. Offers

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
@@ -1339,7 +1339,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);
                       
@@ -1428,6 +1433,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);
@@ -3084,6 +3093,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);
   
@@ -3101,6 +3115,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,
@@ -3202,11 +3218,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()
@@ -3217,29 +3251,37 @@ meta_display_grab_focus_window_button (MetaDisplay *display,
     while (i < MAX_FOCUS_BUTTON)
       {
         meta_change_button_grab (display,
-                                 xwindow,
+                                 window->xwindow,
-                                 TRUE, TRUE, i, 0);
+                                 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,
+        meta_change_button_grab (display, window->xwindow,
-                                 FALSE, TRUE, i, 0);
+                                 FALSE, FALSE, i, 0);
         
         ++i;
       }
+
+    window->have_focus_click_grab = FALSE;
   }
 }
 
@@ -4063,12 +4105,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 */
@@ -4077,19 +4124,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;
         }

src/display.h

@@ -374,9 +374,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);

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);
   
@@ -4003,8 +4004,11 @@ meta_window_notify_focus (MetaWindow *window,
           /* move into FOCUSED_WINDOW layer */
           meta_window_update_layer (window);
 
-          /* Ungrab click to focus button */
+          /* Ungrab click to focus button since the sync grab can interfere
-          meta_display_ungrab_focus_window_button (window->display, window->xwindow);
+           * 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 ||
@@ -4042,8 +4046,8 @@ meta_window_notify_focus (MetaWindow *window,
           /* move out of FOCUSED_WINDOW layer */
           meta_window_update_layer (window);
 
-          /* Re-grab for click to focus */
+          /* Re-grab for click to focus, if necessary */
-          meta_display_grab_focus_window_button (window->display, window->xwindow);
+          meta_display_grab_focus_window_button (window->display, window);
         }
     }
 

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;