add helper functions for manipulating the focus_order list.

move the focus popup into focus.c, out of action.c
allow cycling to iconic windows, which are kept at the bottom of the focus_order lists.

openbox/action.c

@@ -1,12 +1,10 @@
 #include "client.h"
-#include "grab.h"
 #include "focus.h"
 #include "moveresize.h"
 #include "menu.h"
 #include "prop.h"
 #include "stacking.h"
 #include "frame.h"
-#include "framerender.h"
 #include "screen.h"
 #include "action.h"
 #include "dispatch.h"
@@ -683,49 +681,11 @@ void action_showmenu(union ActionData *data)
     }
 }
 
-static void popup_cycle(Client *c, gboolean hide)
-{
-    XSetWindowAttributes attrib;
-    static Window coords = None;
-
-    if (coords == None) {
-        attrib.override_redirect = TRUE;
-        coords = XCreateWindow(ob_display, ob_root,
-                               0, 0, 1, 1, 0, render_depth, InputOutput,
-                               render_visual, CWOverrideRedirect, &attrib);
-        g_assert(coords != None);
-
-        grab_pointer(TRUE, None);
-
-        XMapWindow(ob_display, coords);
-    }
-
-    if (hide) {
-        XDestroyWindow(ob_display, coords);
-        coords = None;
-
-        grab_pointer(FALSE, None);
-    } else {
-        Rect *a;
-        Size s;
-
-        a = screen_area(c->desktop);
-
-        framerender_size_popup_label(c->title, &s);
-        XMoveResizeWindow(ob_display, coords,
-                          a->x + (a->width - s.width) / 2,
-                          a->y + (a->height - s.height) / 2,
-                          s.width, s.height);
-        framerender_popup_label(coords, &s, c->title);
-    }
-}
-
 void action_cycle_windows(union ActionData *data)
 {
     Client *c;
     
     c = focus_cycle(data->cycle.forward, data->cycle.linear, data->cycle.final,
                     data->cycle.cancel);
-    popup_cycle(c, !c || data->cycle.final || data->cycle.cancel);
 }
 

openbox/client.c

@@ -152,7 +152,6 @@ void client_manage(Window window)
     XWindowAttributes attrib;
     XSetWindowAttributes attrib_set;
 /*    XWMHints *wmhint; */
-    guint i;
 
     grab_server(TRUE);
 
@@ -222,13 +221,7 @@ void client_manage(Window window)
     g_hash_table_insert(client_map, &self->window, self);
 
     /* update the focus lists */
-    if (self->desktop == DESKTOP_ALL) {
+    focus_order_add_new(self);
-        for (i = 0; i < screen_num_desktops; ++i)
-            focus_order[i] = g_list_insert(focus_order[i], self, 1);
-    } else {
-        i = self->desktop;
-        focus_order[i] = g_list_insert(focus_order[i], self, 1);
-    }
 
     stacking_raise(self);
 
@@ -300,7 +293,6 @@ void client_unmanage_all()
 
 void client_unmanage(Client *self)
 {
-    guint i;
     int j;
     GSList *it;
 
@@ -322,13 +314,7 @@ void client_unmanage(Client *self)
     g_hash_table_remove(client_map, &self->window);
 
     /* update the focus lists */
-    if (self->desktop == DESKTOP_ALL) {
+    focus_order_remove(self);
-        for (i = 0; i < screen_num_desktops; ++i)
-            focus_order[i] = g_list_remove(focus_order[i], self);
-    } else {
-        i = self->desktop;
-        focus_order[i] = g_list_remove(focus_order[i], self);
-    }
 
     /* once the client is out of the list, update the struts to remove it's
        influence */
@@ -1750,6 +1736,9 @@ void client_iconify(Client *self, gboolean iconic, gboolean curdesk)
 	/* we unmap the client itself so that we can get MapRequest events,
 	   and because the ICCCM tells us to! */
 	XUnmapWindow(ob_display, self->window);
+
+        /* update the focus lists.. iconic windows go to the bottom */
+        focus_order_to_bottom(self);
     } else {
 	if (curdesk)
 	    client_set_desktop(self, screen_desktop, FALSE);
@@ -1918,7 +1907,7 @@ void client_kill(Client *self)
 
 void client_set_desktop(Client *self, guint target, gboolean donthide)
 {
-    guint old, i;
+    guint old;
 
     if (target == self->desktop) return;
   
@@ -1926,6 +1915,9 @@ void client_set_desktop(Client *self, guint target, gboolean donthide)
 
     g_assert(target < screen_num_desktops || target == DESKTOP_ALL);
 
+    /* remove from the old desktop(s) */
+    focus_order_remove(self);
+
     old = self->desktop;
     self->desktop = target;
     PROP_SET32(self->window, net_wm_desktop, cardinal, target);
@@ -1939,25 +1931,11 @@ void client_set_desktop(Client *self, guint target, gboolean donthide)
         stacking_raise(self);
     screen_update_struts();
 
-    /* update the focus lists */
+    /* add to the new desktop(s) */
-    if (old == DESKTOP_ALL) {
+    if (config_focus_new)
-        for (i = 0; i < screen_num_desktops; ++i)
+        focus_order_to_top(self);
-            focus_order[i] = g_list_remove(focus_order[i], self);
+    else
-    } else
+        focus_order_to_bottom(self);
-        focus_order[old] = g_list_remove(focus_order[old], self);
-    if (target == DESKTOP_ALL) {
-        for (i = 0; i < screen_num_desktops; ++i) {
-            if (config_focus_new)
-                focus_order[i] = g_list_prepend(focus_order[i], self);
-            else
-                focus_order[i] = g_list_append(focus_order[i], self);
-        }
-    } else {
-        if (config_focus_new)
-            focus_order[target] = g_list_prepend(focus_order[target], self);
-        else
-            focus_order[target] = g_list_append(focus_order[target], self);
-    }
 
     dispatch_client(Event_Client_Desktop, self, target, old);
 }
@@ -2144,38 +2122,23 @@ Client *client_focus_target(Client *self)
     return self;
 }
 
-gboolean client_focusable(Client *self)
-{
-    /* won't try focus if the client doesn't want it, or if the window isn't
-       visible on the screen */
-    return self->frame->visible &&
-        (self->can_focus || self->focus_notify);
-}
-
 gboolean client_focus(Client *self)
 {
     XEvent ev;
-    guint i;
 
     /* choose the correct target */
     self = client_focus_target(self);
 
     if (self->desktop != DESKTOP_ALL && self->desktop != screen_desktop) {
         /* update the focus lists */
-        if (self->desktop == DESKTOP_ALL) {
+        focus_order_to_top(self);
-            for (i = 0; i < screen_num_desktops; ++i) {
-                focus_order[i] = g_list_remove(focus_order[i], self);
-                focus_order[i] = g_list_prepend(focus_order[i], self);
-            }
-        } else {
-            i = self->desktop;
-            focus_order[i] = g_list_remove(focus_order[i], self);
-            focus_order[i] = g_list_prepend(focus_order[i], self);
-        }
         return FALSE;
     }
 
-    if (!client_focusable(self))
+    if (!((self->can_focus || self->focus_notify) &&
+          (self->desktop == screen_desktop ||
+           self->desktop == DESKTOP_ALL) &&
+          !self->iconic))
 	return FALSE;
 
     /* do a check to see if the window has already been unmapped or destroyed

openbox/client.h

@@ -417,9 +417,6 @@ void client_set_state(Client *self, Atom action, long data1, long data2);
    Client passed to it or another Client if appropriate. */
 Client *client_focus_target(Client *self);
 
-/* Returns if a client can be focused or not */
-gboolean client_focusable(Client *self);
-
 /*! Attempt to focus the client window */
 gboolean client_focus(Client *self);
 

openbox/focus.c

@@ -1,5 +1,7 @@
 #include "event.h"
 #include "openbox.h"
+#include "grab.h"
+#include "framerender.h"
 #include "client.h"
 #include "config.h"
 #include "frame.h"
@@ -132,12 +134,10 @@ static Client *find_transient_recursive(Client *c, Client *top, Client *skip)
     Client *ret;
 
     for (it = c->transients; it; it = it->next) {
-        g_message("looking");
         if (it->data == top) return NULL;
         ret = find_transient_recursive(it->data, top, skip);
         if (ret && ret != skip && client_normal(ret)) return ret;
         if (it->data != skip && client_normal(it->data)) return it->data;
-        g_message("not found");
     }
     return NULL;
 }
@@ -213,6 +213,43 @@ void focus_fallback(FallbackType type)
     focus_set_client(NULL);
 }
 
+static void popup_cycle(Client *c, gboolean show)
+{
+    XSetWindowAttributes attrib;
+    static Window coords = None;
+
+    if (coords == None) {
+        attrib.override_redirect = TRUE;
+        coords = XCreateWindow(ob_display, ob_root,
+                               0, 0, 1, 1, 0, render_depth, InputOutput,
+                               render_visual, CWOverrideRedirect, &attrib);
+        g_assert(coords != None);
+
+        grab_pointer(TRUE, None);
+
+        XMapWindow(ob_display, coords);
+    }
+
+    if (!show) {
+        XDestroyWindow(ob_display, coords);
+        coords = None;
+
+        grab_pointer(FALSE, None);
+    } else {
+        Rect *a;
+        Size s;
+
+        a = screen_area(c->desktop);
+
+        framerender_size_popup_label(c->title, &s);
+        XMoveResizeWindow(ob_display, coords,
+                          a->x + (a->width - s.width) / 2,
+                          a->y + (a->height - s.height) / 2,
+                          s.width, s.height);
+        framerender_popup_label(coords, &s, c->title);
+    }
+}
+
 Client *focus_cycle(gboolean forward, gboolean linear, gboolean done,
                     gboolean cancel)
 {
@@ -231,6 +268,8 @@ Client *focus_cycle(gboolean forward, gboolean linear, gboolean done,
         goto done_cycle;
     } else if (done) {
         if (focus_cycle_target) {
+            if (focus_cycle_target->iconic)
+                client_iconify(focus_cycle_target, FALSE, FALSE);
             client_focus(focus_cycle_target);
             stacking_raise(focus_cycle_target);
         }
@@ -250,23 +289,23 @@ Client *focus_cycle(gboolean forward, gboolean linear, gboolean done,
     do {
         if (forward) {
             it = it->next;
-            if (it == NULL) it = list;
+            if (it == NULL) it = g_list_first(list);
         } else {
             it = it->prev;
             if (it == NULL) it = g_list_last(list);
         }
         ft = client_focus_target(it->data);
-        if (ft == it->data && client_normal(ft) && client_focusable(ft)) {
+        if (ft == it->data && client_normal(ft) &&
+            (ft->can_focus || ft->focus_notify) &&
+            (ft->desktop == screen_desktop || ft->desktop == DESKTOP_ALL)) {
             if (focus_cycle_target)
                 frame_adjust_focus(focus_cycle_target->frame, FALSE);
-            else if (focus_client)
-                frame_adjust_focus(focus_client->frame, FALSE);
             focus_cycle_target = ft;
             frame_adjust_focus(focus_cycle_target->frame, TRUE);
+            popup_cycle(ft, TRUE);
             return ft;
         }
     } while (it != start);
-    return NULL;
 
 done_cycle:
     t = NULL;
@@ -274,5 +313,95 @@ done_cycle:
     focus_cycle_target = NULL;
     g_list_free(order);
     order = NULL;
+    popup_cycle(ft, FALSE);
     return NULL;
 }
+
+void focus_order_add_new(Client *c)
+{
+    guint d, i;
+
+    if (c->iconic)
+        focus_order_to_top(c);
+    else {
+        d = c->desktop;
+        if (d == DESKTOP_ALL) {
+            for (i = 0; i < screen_num_desktops; ++i) {
+                if (focus_order[i] && ((Client*)focus_order[i]->data)->iconic)
+                    focus_order[i] = g_list_insert(focus_order[i], c, 0);
+                else
+                    focus_order[i] = g_list_insert(focus_order[i], c, 1);
+            }
+        } else
+             if (focus_order[d] && ((Client*)focus_order[d]->data)->iconic)
+                focus_order[d] = g_list_insert(focus_order[d], c, 0);
+            else
+                focus_order[d] = g_list_insert(focus_order[d], c, 1);
+    }
+}
+
+void focus_order_remove(Client *c)
+{
+    guint d, i;
+
+    d = c->desktop;
+    if (d == DESKTOP_ALL) {
+        for (i = 0; i < screen_num_desktops; ++i)
+            focus_order[i] = g_list_remove(focus_order[i], c);
+    } else
+        focus_order[d] = g_list_remove(focus_order[d], c);
+}
+
+static void to_top(Client *c, guint d)
+{
+    focus_order[d] = g_list_remove(focus_order[d], c);
+    if (!c->iconic) {
+        focus_order[d] = g_list_prepend(focus_order[d], c);
+    } else {
+        GList *it;
+
+        /* insert before first iconic window */
+        for (it = focus_order[d];
+             it && !((Client*)it->data)->iconic; it = it->next);
+        g_list_insert_before(focus_order[d], it, c);
+    }
+}
+
+void focus_order_to_top(Client *c)
+{
+    guint d, i;
+
+    d = c->desktop;
+    if (d == DESKTOP_ALL) {
+        for (i = 0; i < screen_num_desktops; ++i)
+            to_top(c, i);
+    } else
+        to_top(c, d);
+}
+
+static void to_bottom(Client *c, guint d)
+{
+    focus_order[d] = g_list_remove(focus_order[d], c);
+    if (c->iconic) {
+        focus_order[d] = g_list_append(focus_order[d], c);
+    } else {
+        GList *it;
+
+        /* insert before first iconic window */
+        for (it = focus_order[d];
+             it && !((Client*)it->data)->iconic; it = it->next);
+        g_list_insert_before(focus_order[d], it, c);
+    }
+}
+
+void focus_order_to_bottom(Client *c)
+{
+    guint d, i;
+
+    d = c->desktop;
+    if (d == DESKTOP_ALL) {
+        for (i = 0; i < screen_num_desktops; ++i)
+            to_bottom(c, i);
+    } else
+        to_bottom(c, d);
+}

openbox/focus.h

@@ -36,4 +36,17 @@ void focus_fallback(FallbackType type);
 struct Client *focus_cycle(gboolean forward, gboolean linear, gboolean done,
                            gboolean cancel);
 
+/*! Add a new client into the focus order */
+void focus_order_add_new(struct Client *c);
+
+/*! Remove a client from the focus order */
+void focus_order_remove(struct Client *c);
+
+/*! Move a client to the top of the focus order */
+void focus_order_to_top(struct Client *c);
+
+/*! Move a client to the bottom of the focus order (keeps iconic windows at the
+  very bottom always though). */
+void focus_order_to_bottom(struct Client *c);
+
 #endif