some changes to focus handling.

most interesting is the change in focus_fallback, which means that it won't
call xsetinput focus in some cases, potentially reducing flicker and stuff.
also potentially producing bugs? heh.

the screen.c focus fallback code doesn't seem to need special cases anymore,
which is really good, if that is really the case.

move the focus_tried stuff out of event.c into focus.c, where it seems to
belong.

openbox/client.c

@@ -466,11 +466,6 @@ void client_manage(Window window)
     */
     client_show(self);
 
-    /* use client_focus instead of client_activate cuz client_activate does
-       stuff like switch desktops etc and I'm not interested in all that when
-       a window maps since its not based on an action from the user like
-       clicking a window to activate it. so keep the new window out of the way
-       but do focus it. */
     if (activate) {
         gboolean stacked = client_restore_session_stacking(self);
         client_present(self, FALSE, !stacked);

openbox/event.c

@@ -98,13 +98,6 @@ Time event_curtime = CurrentTime;
 static guint ignore_enter_focus = 0;
 static gboolean menu_can_hide;
 static gboolean focus_left_screen = FALSE;
-/*! This variable is used for focus fallback. If we fallback to a window, we
-  set this to the window. And when focus goes somewhere after that, it will
-  be set to NULL. If between falling back to that window and something
-  getting focused, the window gets unmanaged, then if there are no incoming
-  FocusIn events, we fallback again because focus has just gotten itself lost.
- */
-static ObClient *focus_tried = NULL;
 
 #ifdef USE_SM
 static void ice_handler(gint fd, gpointer conn)
@@ -356,12 +349,12 @@ static gboolean wanted_focusevent(XEvent *e, gboolean in_client_only)
     }
 }
 
-static Bool look_for_focusin(Display *d, XEvent *e, XPointer arg)
+static Bool event_look_for_focusin(Display *d, XEvent *e, XPointer arg)
 {
     return e->type == FocusIn && wanted_focusevent(e, FALSE);
 }
 
-static Bool look_for_focusin_client(Display *d, XEvent *e, XPointer arg)
+Bool event_look_for_focusin_client(Display *d, XEvent *e, XPointer arg)
 {
     return e->type == FocusIn && wanted_focusevent(e, TRUE) &&
         e->xfocus.window != screen_support_win;
@@ -494,7 +487,9 @@ static void event_process(const XEvent *ec, gpointer data)
                But if the other focus in is something like PointerRoot then we
                still want to fall back.
             */
-            if (XCheckIfEvent(ob_display, &ce, look_for_focusin_client, NULL)){
+            if (XCheckIfEvent(ob_display, &ce, event_look_for_focusin_client,
+                              NULL))
+            {
                 XPutBackEvent(ob_display, &ce);
                 ob_debug_type(OB_DEBUG_FOCUS,
                               "  but another FocusIn is coming\n");
@@ -511,7 +506,7 @@ static void event_process(const XEvent *ec, gpointer data)
                     focus_left_screen = FALSE;
 
                 if (!focus_left_screen)
-                    focus_tried = focus_fallback(TRUE);
+                    focus_fallback(TRUE);
             }
         } else if (client && client != focus_client) {
             focus_left_screen = FALSE;
@@ -519,15 +514,13 @@ static void event_process(const XEvent *ec, gpointer data)
             focus_set_client(client);
             client_calc_layer(client);
             client_bring_helper_windows(client);
-
-            focus_tried = NULL; /* focus isn't "trying" to go anywhere now */
         }
     } else if (e->type == FocusOut) {
         gboolean nomove = FALSE;
         XEvent ce;
 
         /* Look for the followup FocusIn */
-        if (!XCheckIfEvent(ob_display, &ce, look_for_focusin, NULL)) {
+        if (!XCheckIfEvent(ob_display, &ce, event_look_for_focusin, NULL)) {
             /* There is no FocusIn, this means focus went to a window that
                is not being managed, or a window on another screen. */
             Window win, root;
@@ -562,7 +555,7 @@ static void event_process(const XEvent *ec, gpointer data)
                 ob_debug_type(OB_DEBUG_FOCUS,
                               "Focus went to an unmanaged window 0x%x !\n",
                               ce.xfocus.window);
-                focus_tried = focus_fallback(TRUE);
+                focus_fallback(TRUE);
             }
         }
 
@@ -1090,40 +1083,10 @@ static void event_handle_client(ObClient *client, XEvent *e)
                  client->window, e->xunmap.event, e->xunmap.from_configure,
                  client->ignore_unmaps);
         client_unmanage(client);
-
-        /* we were trying to focus this window but it's gone */
-        if (client == focus_tried) {
-            ob_debug_type(OB_DEBUG_FOCUS, "Tried to focus window 0x%x and it "
-                          "is being unmanaged:\n");
-            if (XCheckIfEvent(ob_display, &ce, look_for_focusin_client, NULL)){
-                XPutBackEvent(ob_display, &ce);
-                ob_debug_type(OB_DEBUG_FOCUS,
-                              "  but another FocusIn is coming\n");
-            } else {
-                ob_debug_type(OB_DEBUG_FOCUS,
-                              "  so falling back focus again.\n");
-                focus_tried = focus_fallback(TRUE);
-            }
-        }
         break;
     case DestroyNotify:
         ob_debug("DestroyNotify for window 0x%x\n", client->window);
         client_unmanage(client);
-
-        /* we were trying to focus this window but it's gone */
-        if (client == focus_tried) {
-            ob_debug_type(OB_DEBUG_FOCUS, "Tried to focus window 0x%x and it "
-                          "is being unmanaged:\n");
-            if (XCheckIfEvent(ob_display, &ce, look_for_focusin_client, NULL)){
-                XPutBackEvent(ob_display, &ce);
-                ob_debug_type(OB_DEBUG_FOCUS,
-                              "  but another FocusIn is coming\n");
-            } else {
-                ob_debug_type(OB_DEBUG_FOCUS,
-                              "  so falling back focus again.\n");
-                focus_tried = focus_fallback(TRUE);
-            }
-        }
         break;
     case ReparentNotify:
         /* this is when the client is first taken captive in the frame */
@@ -1142,21 +1105,6 @@ static void event_handle_client(ObClient *client, XEvent *e)
      
         ob_debug("ReparentNotify for window 0x%x\n", client->window);
         client_unmanage(client);
-
-        /* we were trying to focus this window but it's gone */
-        if (client == focus_tried) {
-            ob_debug_type(OB_DEBUG_FOCUS, "Tried to focus window 0x%x and it "
-                          "is being unmanaged:\n");
-            if (XCheckIfEvent(ob_display, &ce, look_for_focusin_client, NULL)){
-                XPutBackEvent(ob_display, &ce);
-                ob_debug_type(OB_DEBUG_FOCUS,
-                              "  but another FocusIn is coming\n");
-            } else {
-                ob_debug_type(OB_DEBUG_FOCUS,
-                              "  so falling back focus again.\n");
-                focus_tried = focus_fallback(TRUE);
-            }
-        }
         break;
     case MapRequest:
         ob_debug("MapRequest for 0x%lx\n", client->window);

openbox/event.h

@@ -51,4 +51,7 @@ void event_halt_focus_delay();
   comes at the same time or later than t2. */
 gboolean event_time_after(Time t1, Time t2);
 
+/*! Used with XCheckIfEvent to find a focusin event on a client window */
+Bool event_look_for_focusin_client(Display *d, XEvent *e, XPointer arg);
+
 #endif

openbox/focus.c

@@ -43,6 +43,14 @@ ObClient *focus_client = NULL;
 GList *focus_order = NULL;
 ObClient *focus_cycle_target = NULL;
 
+/*! This variable is used for focus fallback. If we fallback to a window, we
+  set this to the window. And when focus goes somewhere after that, it will
+  be set to NULL. If between falling back to that window and something
+  getting focused, the window gets unmanaged, then if there are no incoming
+  FocusIn events, we fallback again because focus has just gotten itself lost.
+ */
+static ObClient *focus_tried = NULL;
+
 struct {
     InternalWindow top;
     InternalWindow left;
@@ -60,6 +68,7 @@ static gboolean valid_focus_target(ObClient *ft,
                                    gboolean dock_windows,
                                    gboolean desktop_windows);
 static void focus_cycle_destructor(ObClient *client, gpointer data);
+static void focus_tried_destructor(ObClient *client, gpointer data);
 
 static Window createWindow(Window parent, gulong mask,
                            XSetWindowAttributes *attrib)
@@ -78,6 +87,7 @@ void focus_startup(gboolean reconfig)
         XSetWindowAttributes attr;
 
         client_add_destructor(focus_cycle_destructor, NULL);
+        client_add_destructor(focus_tried_destructor, NULL);
 
         /* start with nothing focused */
         focus_nothing();
@@ -131,6 +141,7 @@ void focus_shutdown(gboolean reconfig)
 
     if (!reconfig) {
         client_remove_destructor(focus_cycle_destructor);
+        client_remove_destructor(focus_tried_destructor);
 
         /* reset focus to root */
         XSetInputFocus(ob_display, PointerRoot, RevertToNone, CurrentTime);
@@ -184,9 +195,12 @@ void focus_set_client(ObClient *client)
         PROP_SET32(RootWindow(ob_display, ob_screen),
                    net_active_window, window, active);
     }
+
+
+    focus_tried = NULL; /* focus isn't "trying" to go anywhere now */
 }
 
-ObClient* focus_fallback_target(gboolean allow_refocus, ObClient *old)
+static ObClient* focus_fallback_target(gboolean allow_refocus, ObClient *old)
 {
     GList *it;
     ObClient *target = NULL;
@@ -259,19 +273,27 @@ ObClient* focus_fallback_target(gboolean allow_refocus, ObClient *old)
 ObClient* focus_fallback(gboolean allow_refocus)
 {
     ObClient *new;
-    ObClient *old = focus_client;
+    ObClient *old;
 
-    /* unfocus any focused clients.. they can be focused by Pointer events
+    old = focus_client;
-       and such, and then when I try focus them, I won't get a FocusIn event
+    new = focus_fallback_target(allow_refocus, focus_client);
-       at all for them.
-    */
-    focus_nothing();
 
-    if ((new = focus_fallback_target(allow_refocus, old))) {
+    /* send focus somewhere if it is moving or if it was NULL before,
-        client_focus(new);
+       in which case it may not even be on the screen */
-        return new;
+    if (!old || new != old) {
-    } else
+        /* unfocus any focused clients.. they can be focused by Pointer events
-        return NULL;
+           and such, and then when we try focus them, we won't get a FocusIn
+           event at all for them. */
+        focus_nothing();
+
+        if (new) {
+            client_focus(new);
+            /* remember that we tried to send focus here */
+            focus_tried = new;
+        }
+    }
+
+    return new;
 }
 
 void focus_nothing()
@@ -283,6 +305,7 @@ void focus_nothing()
     }
 
     focus_client = NULL;
+    focus_tried = NULL; /* focus isn't "trying" to go anywhere now */
 
     /* when nothing will be focused, send focus to the backup target */
     XSetInputFocus(ob_display, screen_support_win, RevertToPointerRoot,
@@ -918,3 +941,25 @@ ObClient *focus_order_find_first(guint desktop)
     }
     return NULL;
 }
+
+static void focus_tried_destructor(ObClient *client, gpointer data)
+{
+    XEvent ce;
+
+    if (client == focus_tried) {
+        /* we were trying to focus this window but it's gone */
+
+        focus_tried = NULL;
+
+        ob_debug_type(OB_DEBUG_FOCUS, "Tried to focus window 0x%x and it "
+                      "is being unmanaged:\n");
+        if (XCheckIfEvent(ob_display, &ce, event_look_for_focusin_client,NULL))
+        {
+            XPutBackEvent(ob_display, &ce);
+            ob_debug_type(OB_DEBUG_FOCUS, "  but another FocusIn is coming\n");
+        } else {
+            ob_debug_type(OB_DEBUG_FOCUS, "  so falling back focus again.\n");
+            focus_fallback(TRUE);
+        }
+    }
+}

openbox/focus.h

@@ -45,9 +45,6 @@ void focus_set_client(struct _ObClient *client);
 /*! Focus nothing, but let keyboard events be caught. */
 void focus_nothing();
 
-struct _ObClient* focus_fallback_target(gboolean allow_refocus,
-                                        struct _ObClient *old);
-
 /*! Call this when you need to focus something! */
 struct _ObClient* focus_fallback(gboolean allow_refocus);
 

openbox/screen.c

@@ -494,7 +494,7 @@ void screen_set_desktop(guint num, gboolean dofocus)
        do this before hiding the windows so if helper windows are coming
        with us, they don't get hidden
     */
-    if (dofocus && (c = focus_fallback_target(TRUE, focus_client))) {
+    if (dofocus && (c = focus_fallback(TRUE))) {
         /* only do the flicker reducing stuff ahead of time if we are going
            to call xsetinputfocus on the window ourselves. otherwise there is
            no guarantee the window will actually take focus.. */
@@ -506,7 +506,6 @@ void screen_set_desktop(guint num, gboolean dofocus)
                server FocusIn event */
             frame_adjust_focus(c->frame, TRUE);
         }
-        client_focus(c);
     }
 
     /* hide windows from bottom to top */
@@ -939,7 +938,7 @@ void screen_show_desktop(gboolean show, ObClient *show_only)
     }
 
     if (show) {
-        /* focus desktop */
+        /* focus the desktop */
         for (it = focus_order; it; it = g_list_next(it)) {
             ObClient *c = it->data;
             if (c->type == OB_CLIENT_TYPE_DESKTOP &&
@@ -951,10 +950,16 @@ void screen_show_desktop(gboolean show, ObClient *show_only)
     else if (!show_only) {
         ObClient *c;
 
-        /* use NULL for the "old" argument because the desktop was focused
+        if ((c = focus_fallback(TRUE))) {
-           and we don't want to fallback to the desktop by default */
+            /* only do the flicker reducing stuff ahead of time if we are going
-        if ((c = focus_fallback_target(TRUE, NULL)))
+               to call xsetinputfocus on the window ourselves. otherwise there
-            client_focus(c);
+               is no guarantee the window will actually take focus.. */
+            if (c->can_focus) {
+                /* reduce flicker by hiliting now rather than waiting for the
+                   server FocusIn event */
+                frame_adjust_focus(c->frame, TRUE);
+            }
+        }
     }
 
     show = !!show; /* make it boolean */