use the same decision code to focus new windows as for focus cycling or focus fallback. yay consistency.

6 files changed, 125 insertions, 136 deletions

diff --git a/openbox/client.c b/openbox/client.c
index 29b27d1d..c6945d23 100644
--- a/openbox/client.c
+++ b/openbox/client.c
@@ -314,20 +314,13 @@ void client_manage(Window window)
     /* focus the new window? */
     if (ob_state() != OB_STATE_STARTING &&
         (!self->session || self->session->focused) &&
-        !self->iconic &&
         /* this means focus=true for window is same as config_focus_new=true */
         ((config_focus_new || (settings && settings->focus == 1)) ||
          client_search_focus_tree_full(self)) &&
         /* this checks for focus=false for the window */
         (!settings || settings->focus != 0) &&
-        /* note the check against type Normal/Dialog/Utility,
-           not client_normal(self), which would also include other types.
-           in this case we want more strict rules for focus */
-        (self->type == OB_CLIENT_TYPE_NORMAL ||
-         self->type == OB_CLIENT_TYPE_UTILITY ||
-         self->type == OB_CLIENT_TYPE_DIALOG))
+        focus_valid_target(self, FALSE, TRUE, FALSE, FALSE))
     {
-        /* XXX use focus_cycle_valid_target instead... */
         activate = TRUE;
     }
 
diff --git a/openbox/focus.c b/openbox/focus.c
index 6d66382a..f34021a4 100644
--- a/openbox/focus.c
+++ b/openbox/focus.c
@@ -23,6 +23,7 @@
 #include "grab.h"
 #include "client.h"
 #include "config.h"
+#include "group.h"
 #include "focus_cycle.h"
 #include "screen.h"
 #include "prop.h"
@@ -126,7 +127,7 @@ static ObClient* focus_fallback_target(gboolean allow_refocus,
            backup fallback though)
         */
         if ((allow_omnipresent || c->desktop == screen_desktop) &&
-            focus_cycle_target_valid(c, FALSE, FALSE, FALSE, FALSE) &&
+            focus_valid_target(c, FALSE, FALSE, FALSE, FALSE) &&
             (allow_refocus || client_focus_target(c) != old) &&
             client_focus(c))
         {
@@ -145,7 +146,7 @@ static ObClient* focus_fallback_target(gboolean allow_refocus,
            a splashscreen or a desktop window (save the desktop as a
            backup fallback though)
         */
-        if (focus_cycle_target_valid(c, FALSE, FALSE, FALSE, TRUE) &&
+        if (focus_valid_target(c, FALSE, FALSE, FALSE, TRUE) &&
             (allow_refocus || client_focus_target(c) != old) &&
             client_focus(c))
         {
@@ -267,3 +268,92 @@ ObClient *focus_order_find_first(guint desktop)
     }
     return NULL;
 }
+
+/*! Returns if a focus target has valid group siblings that can be cycled
+  to in its place */
+static gboolean focus_target_has_siblings(ObClient *ft,
+                                          gboolean iconic_windows,
+                                          gboolean all_desktops)
+                                                         
+{
+    GSList *it;
+
+    if (!ft->group) return FALSE;
+
+    for (it = ft->group->members; it; it = g_slist_next(it)) {
+        ObClient *c = it->data;
+        /* check that it's not a helper window to avoid infinite recursion */
+        if (c != ft && c->type == OB_CLIENT_TYPE_NORMAL &&
+            focus_valid_target(c, iconic_windows, all_desktops, FALSE, FALSE))
+        {
+            return TRUE;
+        }
+    }
+    return FALSE;
+}
+
+gboolean focus_valid_target(ObClient *ft,
+                            gboolean iconic_windows,
+                            gboolean all_desktops,
+                            gboolean dock_windows,
+                            gboolean desktop_windows)
+{
+    gboolean ok = FALSE;
+
+    /* it's on this desktop unless you want all desktops.
+
+       do this check first because it will usually filter out the most
+       windows */
+    ok = (all_desktops || ft->desktop == screen_desktop ||
+          ft->desktop == DESKTOP_ALL);
+
+    /* the window can receive focus somehow */
+    ok = ok && (ft->can_focus || ft->focus_notify);
+
+    /* the window is not iconic, or we're allowed to go to iconic ones */
+    ok = ok && (iconic_windows || !ft->iconic);
+
+    /* it's the right type of window */
+    if (dock_windows || desktop_windows)
+        ok = ok && ((dock_windows && ft->type == OB_CLIENT_TYPE_DOCK) ||
+                    (desktop_windows && ft->type == OB_CLIENT_TYPE_DESKTOP));
+    /* modal windows are important and can always get focus if they are
+       visible and stuff, so don't change 'ok' based on their type */ 
+    else if (!ft->modal)
+        /* normal non-helper windows are valid targets */
+        ok = ok &&
+            ((client_normal(ft) && !client_helper(ft))
+             ||
+             /* helper windows are valid targets if... */
+             (client_helper(ft) &&
+              /* ...a window in its group already has focus ... */
+              ((focus_client && ft->group == focus_client->group) ||
+               /* ... or if there are no other windows in its group 
+                  that can be cycled to instead */
+               !focus_target_has_siblings(ft, iconic_windows, all_desktops))));
+
+    /* it's not set to skip the taskbar (unless it is a type that would be
+       expected to set this hint, or modal) */
+    ok = ok && ((ft->type == OB_CLIENT_TYPE_DOCK ||
+                 ft->type == OB_CLIENT_TYPE_DESKTOP ||
+                 ft->type == OB_CLIENT_TYPE_TOOLBAR ||
+                 ft->type == OB_CLIENT_TYPE_MENU ||
+                 ft->type == OB_CLIENT_TYPE_UTILITY) ||
+                ft->modal ||
+                !ft->skip_taskbar);
+
+    /* it's not going to just send focus off somewhere else (modal window),
+       unless that modal window is not one of our valid targets, then let
+       you choose this window and bring the modal one here */
+    {
+        ObClient *cft = client_focus_target(ft);
+        ok = ok && (ft == cft || !focus_valid_target(cft,
+                                                     iconic_windows,
+                                                     all_desktops,
+                                                     dock_windows,
+                                                     desktop_windows));
+    }
+
+    return ok;
+}
+
diff --git a/openbox/focus.h b/openbox/focus.h
index f54da05c..f5033aee 100644
--- a/openbox/focus.h
+++ b/openbox/focus.h
@@ -63,4 +63,10 @@ void focus_order_to_bottom(struct _ObClient *c);
 
 struct _ObClient *focus_order_find_first(guint desktop);
 
+gboolean focus_valid_target(struct _ObClient *ft,
+                            gboolean iconic_windows,
+                            gboolean all_desktops,
+                            gboolean dock_windows,
+                            gboolean desktop_windows);
+
 #endif
diff --git a/openbox/focus_cycle.c b/openbox/focus_cycle.c
index bd8a0846..a5ac4e68 100644
--- a/openbox/focus_cycle.c
+++ b/openbox/focus_cycle.c
@@ -26,7 +26,6 @@
 #include "screen.h"
 #include "openbox.h"
 #include "debug.h"
-#include "group.h"
 
 #include <X11/Xlib.h>
 #include <glib.h>
@@ -37,13 +36,10 @@ static gboolean focus_cycle_all_desktops;
 static gboolean focus_cycle_dock_windows;
 static gboolean focus_cycle_desktop_windows;
 
-static gboolean  focus_target_has_siblings  (ObClient *ft,
-                                             gboolean iconic_windows,
-                                             gboolean all_desktops);
-static ObClient *focus_find_directional    (ObClient *c,
-                                            ObDirection dir,
-                                            gboolean dock_windows,
-                                            gboolean desktop_windows);
+static ObClient *focus_find_directional(ObClient *c,
+                                        ObDirection dir,
+                                        gboolean dock_windows,
+                                        gboolean desktop_windows);
 
 void focus_cycle_startup(gboolean reconfig)
 {
@@ -60,106 +56,17 @@ void focus_cycle_stop(ObClient *ifclient)
     /* stop focus cycling if the given client is a valid focus target,
        and so the cycling is being disrupted */
     if (focus_cycle_target && ifclient &&
-        focus_cycle_target_valid(ifclient,
-                                 focus_cycle_iconic_windows,
-                                 focus_cycle_all_desktops,
-                                 focus_cycle_dock_windows,
-                                 focus_cycle_desktop_windows))
+        focus_valid_target(ifclient,
+                           focus_cycle_iconic_windows,
+                           focus_cycle_all_desktops,
+                           focus_cycle_dock_windows,
+                           focus_cycle_desktop_windows))
     {
         focus_cycle(TRUE, TRUE, TRUE, TRUE, TRUE, TRUE, TRUE, TRUE, TRUE);
         focus_directional_cycle(0, TRUE, TRUE, TRUE, TRUE, TRUE, TRUE);
     }
 }
 
-/*! Returns if a focus target has valid group siblings that can be cycled
-  to in its place */
-static gboolean focus_target_has_siblings(ObClient *ft,
-                                          gboolean iconic_windows,
-                                          gboolean all_desktops)
-                                                         
-{
-    GSList *it;
-
-    if (!ft->group) return FALSE;
-
-    for (it = ft->group->members; it; it = g_slist_next(it)) {
-        ObClient *c = it->data;
-        /* check that it's not a helper window to avoid infinite recursion */
-        if (c != ft && c->type == OB_CLIENT_TYPE_NORMAL &&
-            focus_cycle_target_valid(c, iconic_windows, all_desktops, FALSE,
-                                     FALSE))
-        {
-            return TRUE;
-        }
-    }
-    return FALSE;
-}
-
-gboolean focus_cycle_target_valid(ObClient *ft,
-                                  gboolean iconic_windows,
-                                  gboolean all_desktops,
-                                  gboolean dock_windows,
-                                  gboolean desktop_windows)
-{
-    gboolean ok = FALSE;
-
-    /* it's on this desktop unless you want all desktops.
-
-       do this check first because it will usually filter out the most
-       windows */
-    ok = (all_desktops || ft->desktop == screen_desktop ||
-          ft->desktop == DESKTOP_ALL);
-
-    /* the window can receive focus somehow */
-    ok = ok && (ft->can_focus || ft->focus_notify);
-
-    /* the window is not iconic, or we're allowed to go to iconic ones */
-    ok = ok && (iconic_windows || !ft->iconic);
-
-    /* it's the right type of window */
-    if (dock_windows || desktop_windows)
-        ok = ok && ((dock_windows && ft->type == OB_CLIENT_TYPE_DOCK) ||
-                    (desktop_windows && ft->type == OB_CLIENT_TYPE_DESKTOP));
-    /* modal windows are important and can always get focus if they are
-       visible and stuff, so don't change 'ok' based on their type */ 
-    else if (!ft->modal)
-        /* normal non-helper windows are valid targets */
-        ok = ok &&
-            ((client_normal(ft) && !client_helper(ft))
-             ||
-             /* helper windows are valid targets if... */
-             (client_helper(ft) &&
-              /* ...a window in its group already has focus ... */
-              ((focus_client && ft->group == focus_client->group) ||
-               /* ... or if there are no other windows in its group 
-                  that can be cycled to instead */
-               !focus_target_has_siblings(ft, iconic_windows, all_desktops))));
-
-    /* it's not set to skip the taskbar (unless it is a type that would be
-       expected to set this hint, or modal) */
-    ok = ok && ((ft->type == OB_CLIENT_TYPE_DOCK ||
-                 ft->type == OB_CLIENT_TYPE_DESKTOP ||
-                 ft->type == OB_CLIENT_TYPE_TOOLBAR ||
-                 ft->type == OB_CLIENT_TYPE_MENU ||
-                 ft->type == OB_CLIENT_TYPE_UTILITY) ||
-                ft->modal ||
-                !ft->skip_taskbar);
-
-    /* it's not going to just send focus off somewhere else (modal window),
-       unless that modal window is not one of our valid targets, then let
-       you choose this window and bring the modal one here */
-    {
-        ObClient *cft = client_focus_target(ft);
-        ok = ok && (ft == cft || !focus_cycle_target_valid(cft,
-                                                           iconic_windows,
-                                                           all_desktops,
-                                                           dock_windows,
-                                                           desktop_windows));
-    }
-
-    return ok;
-}
-
 void focus_cycle(gboolean forward, gboolean all_desktops,
                  gboolean dock_windows, gboolean desktop_windows,
                  gboolean linear, gboolean interactive,
@@ -211,11 +118,11 @@ void focus_cycle(gboolean forward, gboolean all_desktops,
             if (it == NULL) it = g_list_last(list);
         }
         ft = it->data;
-        if (focus_cycle_target_valid(ft,
-                                     focus_cycle_iconic_windows,
-                                     focus_cycle_all_desktops,
-                                     focus_cycle_dock_windows,
-                                     focus_cycle_desktop_windows))
+        if (focus_valid_target(ft,
+                               focus_cycle_iconic_windows,
+                               focus_cycle_all_desktops,
+                               focus_cycle_dock_windows,
+                               focus_cycle_desktop_windows))
         {
             if (interactive) {
                 if (ft != focus_cycle_target) { /* prevents flicker */
@@ -283,8 +190,8 @@ static ObClient *focus_find_directional(ObClient *c, ObDirection dir,
         /* the currently selected window isn't interesting */
         if (cur == c)
             continue;
-        if (!focus_cycle_target_valid(it->data, FALSE, FALSE, dock_windows,
-                                      desktop_windows))
+        if (!focus_valid_target(it->data, FALSE, FALSE, dock_windows,
+                                desktop_windows))
             continue;
 
         /* find the centre coords of this window, from the
@@ -385,11 +292,11 @@ void focus_directional_cycle(ObDirection dir, gboolean dock_windows,
         GList *it;
 
         for (it = focus_order; it; it = g_list_next(it))
-            if (focus_cycle_target_valid(it->data,
-                                         focus_cycle_iconic_windows,
-                                         focus_cycle_all_desktops,
-                                         focus_cycle_dock_windows,
-                                         focus_cycle_desktop_windows))
+            if (focus_valid_target(it->data,
+                                   focus_cycle_iconic_windows,
+                                   focus_cycle_all_desktops,
+                                   focus_cycle_dock_windows,
+                                   focus_cycle_desktop_windows))
                 ft = it->data;
     }
         
diff --git a/openbox/focus_cycle.h b/openbox/focus_cycle.h
index afdbdc79..2e32805f 100644
--- a/openbox/focus_cycle.h
+++ b/openbox/focus_cycle.h
@@ -44,10 +44,4 @@ void focus_directional_cycle(ObDirection dir, gboolean dock_windows,
 
 void focus_cycle_stop(struct _ObClient *ifclient);
 
-gboolean  focus_cycle_target_valid(struct _ObClient *ft,
-                                   gboolean iconic_windows,
-                                   gboolean all_desktops,
-                                   gboolean dock_windows,
-                                   gboolean desktop_windows);
-
 #endif
diff --git a/openbox/focus_cycle_popup.c b/openbox/focus_cycle_popup.c
index 3ec14a5c..581fa215 100644
--- a/openbox/focus_cycle_popup.c
+++ b/openbox/focus_cycle_popup.c
@@ -22,7 +22,6 @@
 #include "client.h"
 #include "screen.h"
 #include "focus.h"
-#include "focus_cycle.h"
 #include "openbox.h"
 #include "window.h"
 #include "event.h"
@@ -175,11 +174,11 @@ static void popup_setup(ObFocusCyclePopup *p, gboolean create_targets,
     for (it = g_list_last(focus_order); it; it = g_list_previous(it)) {
         ObClient *ft = it->data;
 
-        if (focus_cycle_target_valid(ft,
-                                     iconic_windows,
-                                     all_desktops,
-                                     dock_windows,
-                                     desktop_windows))
+        if (focus_valid_target(ft,
+                               iconic_windows,
+                               all_desktops,
+                               dock_windows,
+                               desktop_windows))
         {
             gchar *text = popup_get_name(ft);