On Sun, 14 Jan 2007, Dominik Vogt wrote:
On Sun, Jan 14, 2007 at 04:42:23PM +0100, Viktor Griph wrote:
This loop is on line 2288 in icons.c:
for (ofw = NULL; fw != ofw && IS_ICONIFIED_BY_PARENT(fw); )
{
t = get_transientfor_fvwmwindow(fw);
if (t != NULL)
{
ofw = fw;
fw = t;
}
}
I'm not sure exactly what it is supposed to do, but if a non-transient
window is iconified by IconifyWindowGroups it will loop forever.
This loop looks for the top window of the transient tree.
Actually, your fix broke that logic because it climbs only one
level up the tree. The right fix is to break the loop if
get_transientfor_fvwmwindow returns NULL. The comparison
(ofw == fw) is there to catch windows that are their own
transients.
My fix did still climb the tree. What it did was to always assign ofw=fw,
and as long as t wasn't null assign fw = t. Which mean that only if t was
the same as fw or t was NULL would ofw == fw, and the loop would end.
It changed the value of ofw after the loop, but it wasn't used anyway.
However I'm still not convinced that this is the right fix. That code was
written without the IconifyWindowGroups option in mind, shouldn't it
really try to climb to the head of the windowgroup as well? Maybe it is
ennoug to use the mark_transient_subtree result. That function doesn't
care about if it's called with the head of a tree or not.
The only effect the loop has is that deiconifying raises, not the window
selected for deiconify, but the root of the transient tree. Isn't it
better to raise the window selected, and let the
styles StackTransientParent and RaisTransient control which windows
actually are raised.
/Viktor
