nice code cleanup that's been needed for a long time. add parents list to client, which you can iterate instead of going thru the group.

openbox/client.c

@@ -93,6 +93,7 @@ static GSList *client_search_all_top_parents_internal(ObClient *self,
                                                       ObStackingLayer layer);
 static void client_call_notifies(ObClient *self, GSList *list);
 
+
 void client_startup(gboolean reconfig)
 {
     if (reconfig) return;
@@ -162,37 +163,6 @@ void client_set_list()
     stacking_set_list();
 }
 
-/*
-  void client_foreach_transient(ObClient *self, ObClientForeachFunc func, gpointer data)
-  {
-  GSList *it;
-
-  for (it = self->transients; it; it = g_slist_next(it)) {
-  if (!func(it->data, data)) return;
-  client_foreach_transient(it->data, func, data);
-  }
-  }
-
-  void client_foreach_ancestor(ObClient *self, ObClientForeachFunc func, gpointer data)
-  {
-  if (self->transient_for) {
-  if (self->transient_for != OB_TRAN_GROUP) {
-  if (!func(self->transient_for, data)) return;
-  client_foreach_ancestor(self->transient_for, func, data);
-  } else {
-  GSList *it;
-
-  for (it = self->group->members; it; it = g_slist_next(it))
-  if (it->data != self &&
-  !((ObClient*)it->data)->transient_for) {
-  if (!func(it->data, data)) return;
-  client_foreach_ancestor(it->data, func, data);
-  }
-  }
-  }
-  }
-*/
-
 void client_manage_all()
 {
     guint i, j, nchild;
@@ -682,22 +652,16 @@ void client_unmanage(ObClient *self)
     client_call_notifies(self, client_destroy_notifies);
 
     /* tell our parent(s) that we're gone */
-    if (self->transient_for == OB_TRAN_GROUP) { /* transient of group */
+    for (it = self->parents; it; it = g_slist_next(it))
-        for (it = self->group->members; it; it = g_slist_next(it))
+        ((ObClient*)it->data)->transients =
-            if (it->data != self)
+            g_slist_remove(((ObClient*)it->data)->transients,self);
-                ((ObClient*)it->data)->transients =
-                    g_slist_remove(((ObClient*)it->data)->transients,self);
-    } else if (self->transient_for) {        /* transient of window */
-        self->transient_for->transients =
-            g_slist_remove(self->transient_for->transients, self);
-    }
 
     /* tell our transients that we're gone */
     for (it = self->transients; it; it = g_slist_next(it)) {
-        if (((ObClient*)it->data)->transient_for != OB_TRAN_GROUP) {
+        ((ObClient*)it->data)->parents =
-            ((ObClient*)it->data)->transient_for = NULL;
+            g_slist_remove(((ObClient*)it->data)->parents, self);
-            client_calc_layer(it->data);
+        /* we could be keeping our children in a higher layer */
-        }
+        client_calc_layer(it->data);
     }
 
     /* remove from its group */
@@ -1141,38 +1105,30 @@ static void client_get_desktop(ObClient *self)
         gboolean trdesk = FALSE;
 
         if (self->transient_for) {
-            if (self->transient_for != OB_TRAN_GROUP) {
+            /* if they are all on one desktop, then open it on the
-                if (self->transient_for->desktop != DESKTOP_ALL) {
+               same desktop */
-                    self->desktop = self->transient_for->desktop;
+            GSList *it;
-                    trdesk = TRUE;
+            gboolean first = TRUE;
-                }
+            guint all = screen_num_desktops; /* not a valid value */
-            } else {
-                /* if all the group is on one desktop, then open it on the
-                   same desktop */
-                GSList *it;
-                gboolean first = TRUE;
-                guint all = screen_num_desktops; /* not a valid value */
 
-                for (it = self->group->members; it; it = g_slist_next(it)) {
+            for (it = self->parents; it; it = g_slist_next(it)) {
-                    ObClient *c = it->data;
+                ObClient *c = it->data;
 
-                    if (c->desktop == DESKTOP_ALL) continue;
+                if (c->desktop == DESKTOP_ALL) continue;
 
-                    if (c != self) {
+                if (first) {
-                        if (first) {
+                    all = c->desktop;
-                            all = c->desktop;
+                    first = FALSE;
-                            first = FALSE;
-                        }
-                        else if (all != c->desktop)
-                            all = screen_num_desktops; /* make it invalid */
-                    }
-                }
-                if (all != screen_num_desktops) {
-                    self->desktop = all;
-                    trdesk = TRUE;
                 }
+                else if (all != c->desktop)
+                    all = screen_num_desktops; /* make it invalid */
+            }
+            if (all != screen_num_desktops) {
+                self->desktop = all;
+                trdesk = TRUE;
             }
         }
+
         if (!trdesk) {
             /* try get from the startup-notification protocol */
             if (sn_get_desktop(self->startup_id, &self->desktop)) {
@@ -1259,23 +1215,9 @@ void client_update_transient_for(ObClient *self)
                 target = NULL;
             }
 
-            /* THIS IS SO ANNOYING ! ! ! ! Let me explain.... have a seat..
+            /* Setting the transient_for to Root is actually illegal, however
-
-               Setting the transient_for to Root is actually illegal, however
                applications from time have done this to specify transient for
-               their group.
+               their group */
-
-               Now you can do that by being a TYPE_DIALOG and not setting
-               the transient_for hint at all on your window. But people still
-               use Root, and Kwin is very strange in this regard.
-
-               KWin 3.0 will not consider windows with transient_for set to
-               Root as transient for their group *UNLESS* they are also modal.
-               In that case, it will make them transient for the group. This
-               leads to all sorts of weird behavior from KDE apps which are
-               only tested in KWin. I'd like to follow their behavior just to
-               make this work right with KDE stuff, but that seems wrong.
-            */
             if (!target && self->group) {
                 /* not transient to a client, see if it is transient for a
                    group */
@@ -1323,13 +1265,14 @@ static void client_update_transient_tree(ObClient *self,
     */
     if (oldparent != newparent &&
         newparent != NULL && newparent != OB_TRAN_GROUP &&
-        newgroup != NULL && newgroup == oldgroup)
+        newgroup != NULL && newgroup == oldgroup && client_normal(newparent))
     {
         ObClient *look = newparent;
         do {
             self->transients = g_slist_remove(self->transients, look);
+            look->parents = g_slist_remove(look->parents, self);
             look = look->transient_for;
-        } while (look != NULL && look != OB_TRAN_GROUP);
+        } while (look != NULL && look != OB_TRAN_GROUP && client_normal(look));
     }
             
 
@@ -1344,8 +1287,10 @@ static void client_update_transient_tree(ObClient *self,
         for (it = self->transients; it; it = next) {
             next = g_slist_next(it);
             c = it->data;
-            if (c->group == oldgroup)
+            if (c->group == oldgroup && client_normal(self)) {
                 self->transients = g_slist_delete_link(self->transients, it);
+                c->parents = g_slist_remove(c->parents, self);
+            }
         }
     }
 
@@ -1355,19 +1300,25 @@ static void client_update_transient_tree(ObClient *self,
     if (oldparent == OB_TRAN_GROUP && (oldgroup != newgroup ||
                                        oldparent != newparent))
     {
-        for (it = oldgroup->members; it; it = g_slist_next(it)) {
+        for (it = self->parents; it; it = next) {
+            next = g_slist_next(it);
             c = it->data;
-            if (c != self && (!c->transient_for ||
+            if ((!c->transient_for || c->transient_for != OB_TRAN_GROUP) &&
-                              c->transient_for != OB_TRAN_GROUP))
+                client_normal(c))
+            {
                 c->transients = g_slist_remove(c->transients, self);
+                self->parents = g_slist_delete_link(self->parents, it);
+            }
         }
     }
     /* If we used to be transient for a single window and we are no longer
        transient for it, then we need to remove ourself from its children */
     else if (oldparent != NULL && oldparent != OB_TRAN_GROUP &&
-             oldparent != newparent)
+             oldparent != newparent && client_normal(oldparent))
+    {
         oldparent->transients = g_slist_remove(oldparent->transients, self);
-
+        self->parents = g_slist_remove(self->parents, oldparent);
+    }
 
     /** Re-add the client to the transient tree wherever it has changed **/
 
@@ -1378,9 +1329,14 @@ static void client_update_transient_tree(ObClient *self,
     {
         for (it = oldgroup->members; it; it = g_slist_next(it)) {
             c = it->data;
-            if (c != self && (!c->transient_for ||
+            if (c != self &&
-                              c->transient_for != OB_TRAN_GROUP))
+                (!c->transient_for ||
+                 c->transient_for != OB_TRAN_GROUP) &&
+                client_normal(c))
+            {
                 c->transients = g_slist_prepend(c->transients, self);
+                self->parents = g_slist_prepend(self->parents, c);
+            }
         }
     }
     /* If we are now transient for a single window which we weren't before,
@@ -1392,8 +1348,12 @@ static void client_update_transient_tree(ObClient *self,
     else if (newparent != NULL && newparent != OB_TRAN_GROUP &&
              newparent != oldparent &&
              /* don't make ourself its child if it is already our child */
-             !client_is_direct_child(self, newparent))
+             !client_is_direct_child(self, newparent) &&
+             client_normal(newparent))
+    {
         newparent->transients = g_slist_prepend(newparent->transients, self);
+        self->parents = g_slist_prepend(self->parents, newparent);
+    }
 
     /* If the group changed then we need to add any new group transient
        windows to our children. But if we're transient for the group, then
@@ -1412,9 +1372,11 @@ static void client_update_transient_tree(ObClient *self,
             c = it->data;
             if (c != self && c->transient_for == OB_TRAN_GROUP &&
                 /* Don't make it our child if it is already our parent */
-                !client_is_direct_child(c, self))
+                !client_is_direct_child(c, self) &&
+                client_normal(self))
             {
                 self->transients = g_slist_prepend(self->transients, c);
+                c->parents = g_slist_prepend(c->parents, self);
             }
         }
     }
@@ -2368,28 +2330,23 @@ ObClient *client_search_focus_tree(ObClient *self)
 
 ObClient *client_search_focus_tree_full(ObClient *self)
 {
-    if (self->transient_for) {
+    if (self->parents) {
-        if (self->transient_for != OB_TRAN_GROUP) {
+        GSList *it;
-            return client_search_focus_tree_full(self->transient_for);
-        } else {
-            GSList *it;
-        
-            for (it = self->group->members; it; it = g_slist_next(it)) {
-                if (it->data != self) {
-                    ObClient *c = it->data;
 
-                    if (client_focused(c)) return c;
+        for (it = self->parents; it; it = g_slist_next(it)) {
-                    if ((c = client_search_focus_tree(it->data))) return c;
+            ObClient *c = it->data;
-                }
+            if ((c = client_search_focus_tree_full(it->data))) return c;
-            }
         }
-    }
 
-    /* this function checks the whole tree, the client_search_focus_tree
+        return NULL;
-       does not, so we need to check this window */
+    }
-    if (client_focused(self))
+    else {
-        return self;
+        /* this function checks the whole tree, the client_search_focus_tree
-    return client_search_focus_tree(self);
+           does not, so we need to check this window */
+        if (client_focused(self))
+            return self;
+        return client_search_focus_tree(self);
+    }
 }
 
 ObClient *client_search_focus_group_full(ObClient *self)
@@ -2410,21 +2367,7 @@ ObClient *client_search_focus_group_full(ObClient *self)
 
 gboolean client_has_parent(ObClient *self)
 {
-    if (self->transient_for) {
+    return self->parents != NULL;
-        if (self->transient_for != OB_TRAN_GROUP) {
-            if (client_normal(self->transient_for))
-                return TRUE;
-        }
-        else if (self->group) {
-            GSList *it;
-
-            for (it = self->group->members; it; it = g_slist_next(it)) {
-                if (it->data != self && client_normal(it->data))
-                    return TRUE;
-            }
-        }
-    }
-    return FALSE;
 }
 
 static ObStackingLayer calc_layer(ObClient *self)
@@ -3098,7 +3041,7 @@ void client_iconify(ObClient *self, gboolean iconic, gboolean curdesk,
 {
     if (self->functions & OB_CLIENT_FUNC_ICONIFY || !iconic) {
         /* move up the transient chain as far as possible first */
-        self = client_search_top_normal_parent(self);
+        self = client_search_top_direct_parent(self);
         client_iconify_recursive(self, iconic, curdesk, hide_animation);
     }
 }
@@ -3274,15 +3217,13 @@ void client_set_desktop_recursive(ObClient *self,
 
 void client_set_desktop(ObClient *self, guint target, gboolean donthide)
 {
-    self = client_search_top_normal_parent(self);
+    self = client_search_top_direct_parent(self);
     client_set_desktop_recursive(self, target, donthide);
 }
 
 gboolean client_is_direct_child(ObClient *parent, ObClient *child)
 {
-    while (child != parent &&
+    while (child != parent && (child = client_direct_parent(child)));
-           child->transient_for && child->transient_for != OB_TRAN_GROUP)
-        child = child->transient_for;
     return child == parent;
 }
 
@@ -3700,20 +3641,12 @@ static ObClientIcon* client_icon_recursive(ObClient *self, gint w, gint h)
 
     if (!self->nicons) {
         ObClientIcon *parent = NULL;
+        GSList *it;
 
-        if (self->transient_for) {
+        for (it = self->parents; it; it = g_slist_next(it)) {
-            if (self->transient_for != OB_TRAN_GROUP)
+            ObClient *c = it->data;
-                parent = client_icon_recursive(self->transient_for, w, h);
+            if ((parent = client_icon_recursive(c, w, h)))
-            else {
+                break;
-                GSList *it;
-                for (it = self->group->members; it; it = g_slist_next(it)) {
-                    ObClient *c = it->data;
-                    if (c != self && !c->transient_for) {
-                        if ((parent = client_icon_recursive(c, w, h)))
-                            break;
-                    }
-                }
-            }
         }
         
         return parent;
@@ -3784,11 +3717,17 @@ guint client_monitor(ObClient *self)
     return screen_find_monitor(&self->frame->area);
 }
 
-ObClient *client_search_top_normal_parent(ObClient *self)
+ObClient *client_direct_parent(ObClient *self)
 {
-    while (self->transient_for && self->transient_for != OB_TRAN_GROUP &&
+    if (!self->parents) return NULL;
-           client_normal(self->transient_for))
+    if (self->transient_for == OB_TRAN_GROUP) return NULL;
-        self = self->transient_for;
+    return self->parents->data;
+}                        
+
+ObClient *client_search_top_direct_parent(ObClient *self)
+{
+    ObClient *p;
+    while ((p = client_direct_parent(self))) self = p;
     return self;
 }
 
@@ -3796,34 +3735,18 @@ static GSList *client_search_all_top_parents_internal(ObClient *self,
                                                       gboolean bylayer,
                                                       ObStackingLayer layer)
 {
-    GSList *ret = NULL;
+    GSList *ret;
+    ObClient *p;
     
     /* move up the direct transient chain as far as possible */
-    while (self->transient_for && self->transient_for != OB_TRAN_GROUP &&
+    while ((p = client_direct_parent(self)) &&
-           (!bylayer || self->transient_for->layer == layer) &&
+           (!bylayer || p->layer == layer))
-           client_normal(self->transient_for))
+        self = p;
-        self = self->transient_for;
 
-    if (!self->transient_for)
+    if (!self->parents)
-        ret = g_slist_prepend(ret, self);
+        ret = g_slist_prepend(NULL, self);
-    else {
+    else
-            GSList *it;
+        ret = g_slist_copy(self->parents);
-
-            g_assert(self->group);
-
-            for (it = self->group->members; it; it = g_slist_next(it)) {
-                ObClient *c = it->data;
-
-                if (!c->transient_for && client_normal(c) &&
-                    (!bylayer || c->layer == layer))
-                {
-                    ret = g_slist_prepend(ret, c);
-                }
-            }
-
-            if (ret == NULL) /* no group parents */
-                ret = g_slist_prepend(ret, self);
-    }
 
     return ret;
 }
@@ -3840,46 +3763,20 @@ GSList *client_search_all_top_parents_layer(ObClient *self)
 
 ObClient *client_search_focus_parent(ObClient *self)
 {
-    if (self->transient_for) {
+    GSList *it;
-        if (self->transient_for != OB_TRAN_GROUP) {
-            if (client_focused(self->transient_for))
-                return self->transient_for;
-        } else {
-            GSList *it;
 
-            for (it = self->group->members; it; it = g_slist_next(it)) {
+    for (it = self->parents; it; it = g_slist_next(it))
-                ObClient *c = it->data;
+        if (client_focused(it->data)) return it->data;
-
-                /* checking transient_for prevents infinate loops! */
-                if (c != self && !c->transient_for)
-                    if (client_focused(c))
-                        return c;
-            }
-        }
-    }
 
     return NULL;
 }
 
 ObClient *client_search_parent(ObClient *self, ObClient *search)
 {
-    if (self->transient_for) {
+    GSList *it;
-        if (self->transient_for != OB_TRAN_GROUP) {
-            if (self->transient_for == search)
-                return search;
-        } else {
-            GSList *it;
 
-            for (it = self->group->members; it; it = g_slist_next(it)) {
+    for (it = self->parents; it; it = g_slist_next(it))
-                ObClient *c = it->data;
+        if (it->data == search) return search;
-
-                /* checking transient_for prevents infinate loops! */
-                if (c != self && !c->transient_for)
-                    if (c == search)
-                        return search;
-            }
-        }
-    }
 
     return NULL;
 }

openbox/client.h

@@ -103,6 +103,8 @@ struct _ObClient
       case.
      */
     ObClient *transient_for;
+    /*! The client which are parents of this client */
+    GSList *parents;
     /*! The clients which are transients (children) of this client */
     GSList *transients;
     /*! The desktop on which the window resides (0xffffffff for all
@@ -659,10 +661,15 @@ GSList *client_search_all_top_parents(ObClient *self);
 */
 GSList *client_search_all_top_parents_layer(ObClient *self);
 
+/*! Returns the client's parent when it is transient for a direct window
+  rather than a group. If it has no parents, or is transient for the
+  group, this returns null */
+ObClient *client_direct_parent(ObClient *self);
+
 /*! Returns a window's top level parent. This only counts direct parents,
   not groups if it is transient for its group.
 */
-ObClient *client_search_top_normal_parent(ObClient *self);
+ObClient *client_search_top_direct_parent(ObClient *self);
 
 /*! Is one client a direct child of another (i.e. not through the group.) */
 gboolean client_is_direct_child(ObClient *parent, ObClient *child);

openbox/focus_cycle_popup.c

@@ -211,31 +211,26 @@ static void popup_setup(ObFocusCyclePopup *p, gboolean create_targets,
 static gchar *popup_get_name(ObClient *c)
 {
     ObClient *p;
-    gchar *title = NULL;
+    gchar *title;
     const gchar *desk = NULL;
     gchar *ret;
 
-    /* find our highest direct parent, including non-normal windows */
+    /* find our highest direct parent */
-    for (p = c; p->transient_for && p->transient_for != OB_TRAN_GROUP;
+    p = client_search_top_direct_parent(c);
-         p = p->transient_for);
 
     if (c->desktop != DESKTOP_ALL && c->desktop != screen_desktop)
         desk = screen_desktop_names[c->desktop];
 
-    /* use the transient's parent's title/icon if we don't have one */
+    title = c->iconic ? c->icon_title : c->title;
-    if (p != c && !strcmp("", (c->iconic ? c->icon_title : c->title)))
-        title = g_strdup(p->iconic ? p->icon_title : p->title);
 
-    if (title == NULL)
+    /* use the transient's parent's title/icon if we don't have one */
-        title = g_strdup(c->iconic ? c->icon_title : c->title);
+    if (p != c && title[0] == '\0')
+        title = p->iconic ? p->icon_title : p->title;
 
     if (desk)
         ret = g_strdup_printf("%s [%s]", title, desk);
-    else {
+    else
-        ret = title;
+        ret = g_strdup(title);
-        title = NULL;
-    }
-    g_free(title);
 
     return ret;
 }

openbox/place.c

@@ -62,6 +62,7 @@ static Rect **pick_head(ObClient *c)
     guint *choice;
     guint i;
     gint px, py;
+    ObClient *p;
 
     area = g_new(Rect*, screen_num_monitors);
     choice = g_new(guint, screen_num_monitors);
@@ -69,12 +70,10 @@ static Rect **pick_head(ObClient *c)
         choice[i] = screen_num_monitors; /* make them all invalid to start */
 
     /* try direct parent first */
-    if (c->transient_for && c->transient_for != OB_TRAN_GROUP &&
+    if ((p = client_direct_parent(c))) {
-        client_normal(c->transient_for))
+        add_choice(choice, client_monitor(p));
-    {
-        add_choice(choice, client_monitor(c->transient_for));
         ob_debug("placement adding choice %d for parent\n",
-                 client_monitor(c->transient_for));
+                 client_monitor(p));
     }
 
     /* more than one window in its group (more than just this window) */
@@ -446,42 +445,24 @@ static gboolean place_per_app_setting(ObClient *client, gint *x, gint *y,
 
 static gboolean place_transient_splash(ObClient *client, gint *x, gint *y)
 {
-    if (client->transient_for && client->type == OB_CLIENT_TYPE_DIALOG) {
+    if (client->type == OB_CLIENT_TYPE_DIALOG) {
-        if (client->transient_for != OB_TRAN_GROUP &&
+        GSList *it;
-            client_normal(client->transient_for) &&
+        gboolean first = TRUE;
-            !client->iconic)
+        gint l, r, t, b;
-        {
+        for (it = client->parents; it; it = g_slist_next(it)) {
-            ObClient *c = client;
+            ObClient *m = it->data;
-            ObClient *p = client->transient_for;
+            if (!m->iconic) {
-
+                if (first) {
-            if (client_normal(p)) {
+                    l = RECT_LEFT(m->frame->area);
-                *x = (p->frame->area.width - c->frame->area.width) / 2 +
+                    t = RECT_TOP(m->frame->area);
-                    p->frame->area.x;
+                    r = RECT_RIGHT(m->frame->area);
-                *y = (p->frame->area.height - c->frame->area.height) / 2 +
+                    b = RECT_BOTTOM(m->frame->area);
-                    p->frame->area.y;
+                    first = FALSE;
-                return TRUE;
+                } else {
-            }
+                    l = MIN(l, RECT_LEFT(m->frame->area));
-        } else {
+                    t = MIN(t, RECT_TOP(m->frame->area));
-            GSList *it;
+                    r = MAX(r, RECT_RIGHT(m->frame->area));
-            gboolean first = TRUE;
+                    b = MAX(b, RECT_BOTTOM(m->frame->area));
-            gint l, r, t, b;
-            for (it = client->group->members; it; it = g_slist_next(it)) {
-                ObClient *m = it->data;
-                if (!(m == client || m->transient_for) && client_normal(m) &&
-                    !m->iconic)
-                {
-                    if (first) {
-                        l = RECT_LEFT(m->frame->area);
-                        t = RECT_TOP(m->frame->area);
-                        r = RECT_RIGHT(m->frame->area);
-                        b = RECT_BOTTOM(m->frame->area);
-                        first = FALSE;
-                    } else {
-                        l = MIN(l, RECT_LEFT(m->frame->area));
-                        t = MIN(t, RECT_TOP(m->frame->area));
-                        r = MAX(r, RECT_RIGHT(m->frame->area));
-                        b = MAX(b, RECT_BOTTOM(m->frame->area));
-                    }
                 }
             }
             if (!first) {
@@ -492,8 +473,8 @@ static gboolean place_transient_splash(ObClient *client, gint *x, gint *y)
         }
     }
 
-    if ((client->transient && client->type == OB_CLIENT_TYPE_DIALOG)
+    if (client->type == OB_CLIENT_TYPE_DIALOG ||
-        || client->type == OB_CLIENT_TYPE_SPLASH)
+        client->type == OB_CLIENT_TYPE_SPLASH)
     {
         Rect **areas;
 

openbox/stacking.c

@@ -169,7 +169,7 @@ static void restack_windows(ObClient *selected, gboolean raise)
     GList *modals = NULL;
     GList *trans = NULL;
 
-    if (!raise && selected->transient_for) {
+    if (!raise && selected->parents) {
         GSList *top, *top_it;
         GSList *top_reorder = NULL;
         
@@ -371,7 +371,7 @@ static GList *find_highest_relative(ObClient *client)
 {    
     GList *ret = NULL;
 
-    if (client->transient_for) {
+    if (client->parents) {
         GList *it;
         GSList *top;