Moving to the right component. Sorry for the spam.

the bug may could be solved if "state_changed_tag" var will moved from _WnckTask
to _WnckWindowPrivate. the signal will be connected in wnck_tasklist_update_lists 
before the window is excluded with wnck_tasklist_include_window. if the state
changes the function wnck_task_state_changed will still called (and the window
with the removed skip_taskbar property will be included again)

window.c:

gulong
wnck_window_get_state_changed_tag (WnckWindow *window)
{
  g_return_val_if_fail (WNCK_IS_WINDOW (window), 0);

  return window->priv->state_changed_tag;
}

void 
wnck_window_set_state_changed_tag (WnckWindow *window, gulong tag)
{
  g_return_val_if_fail (WNCK_IS_WINDOW (window), NULL);

  window->priv->state_changed_tag=tag;
}

tasklist.c:

static void
wnck_tasklist_update_lists (WnckTasklist *tasklist)
{
....
....

l = windows = wnck_screen_get_windows (tasklist->priv->screen);
  while (l != NULL)
    {
      win = WNCK_WINDOW (l->data);
      gulong tag=wnck_window_get_state_changed_tag(win);

       if (tag == 0) {

	     tag = g_signal_connect (G_OBJECT (win), "state_changed",
              G_CALLBACK (wnck_task_state_changed), tasklist);

              wnck_window_set_state_changed_tag(win, tag);

	    }


      if (wnck_tasklist_include_window (tasklist, win))
...
...

removing the signal can be done in .. ähh, i must go ;)

Created attachment 47879 [details] [review]
patch to patch bug

the (new) idea is to add a list to taskbar struct with all windows with
_NET_WM_STATE_SKIP_TASKBAR property set. windows are not included in taskbar,
but connected to "state_changed". only windows from type "normal" are
recognized. maybe windows from type "utility" should be recognized too.

Um, so, looking over the code it seems like there is already code there to
handle a change from _NET_WM_STATE_SKIP_TASKBAR -- though perhaps a Metacity bug
that I just submitted a patch for was interfering.  A couple questions:

1) Can you submit a simple testcase program that can be used to trigger the bug?

2) What is the _NET_WM_WINDOW_TYPE of the window that you are setting and
unsetting _NET_WM_STATE_SKIP_TASKBAR on?

1)

#include <gtk/gtk.h>
#include <gdk/gdkx.h>
#include <string.h>
#include <stdlib.h>

// to add property: ./skip 100663306 add
//remove ./skip 100663306 remove
//number is window id, got with xwininfo -int


void skip (Window target, gboolean add)
{

  XEvent xev;

  printf ("target win: %d\n", target);

  if (add)
    printf ("add\n");
  else
    printf ("remove\n");
  
  xev.xclient.type = ClientMessage;
  xev.xclient.serial = 0;
  xev.xclient.send_event = True;
  xev.xclient.window = target;
  xev.xclient.message_type = XInternAtom (GDK_DISPLAY(), "_NET_WM_STATE", False);
  xev.xclient.format = 32;
  xev.xclient.data.l[0] = add ? 1 : 0;
  xev.xclient.data.l[1] = XInternAtom (GDK_DISPLAY(),
"_NET_WM_STATE_SKIP_TASKBAR", False);
  xev.xclient.data.l[2] = 0;
  xev.xclient.data.l[3] = 0;
  xev.xclient.data.l[4] = 0;
  
  XSendEvent (GDK_DISPLAY(), GDK_ROOT_WINDOW(), False,
     SubstructureRedirectMask | SubstructureNotifyMask, &xev);
  
  XSync (GDK_DISPLAY(), False);
}


int main( int   argc, char *argv[] )
{

  gtk_init (&argc, &argv);

  if (!strcmp (argv[2], "add"))
    skip (atoi (argv[1]), TRUE);
  else
    skip (atoi (argv[1]), FALSE);


  return 0;
}

add property will work (remove from list), remove property will not update (add
to list) tasklist (move the window around to update)

2) _NET_WM_WINDOW_TYPE_NORMAL and maybe _NET_WM_WINDOW_TYPE_UTILITY

Created attachment 49366 [details] [review]
Add a window_needs_tracking signal

Oh, right; we have code for it and a state_changed signal gets emitted but
there's no task corresponding to the window in the taskbar so the signal is
ignored.  It looks like you figured that out and showed it with your patch but I
somehow missed it.

Anyway, I'd rather avoid making this specific to certain window types.  Also,
it'd be nice to continue ignoring signals for windows that aren't in the
tasklist to avoid extra work (it might be nice to have the functions in window.c
ignore the extra work as well if the window isn't being tracked, though I don't
know if we need to worry about that enhancement right now).  I'm not sure the
right approach exactly, but I attached a patch above that solves this by adding
a new signal.  I'll need to get one of the other maintainers to comment though,
because that patch has something that bothers me a little--it effectively
makes/requires window.c know whether the window will be tracked or not while
currently we had this separation where only tasklist.c would know.  That's
probably not a big deal if the rule is always based on skip_taskbar but might
become an issue if other reasons are included.  *shrug*

Havoc, Mark, Vincent: comments?

Comment on attachment 49366 [details] [review]
Add a window_needs_tracking signal

I think it would be cleaner  if the tasklist just tracked all windows and
noticed when the skip_taskbar flag changed

will this be fixed in gnome 2.12 ? imho is a not perfect patch better than leave
the bug. (or is it already fixed somewhere deep inside gnome.cvs ?)

Man, I so totally suck.  Both Jochen and Jaap pinged me in email and I told them
I'd get back to it and it's still here.  Anyway, I did take a brief look at it
about the time of RC1, and the only other way besides Jochen's that I could see
doing it would (if I'm remembering correctly) involve creating an actual
WnckTask for each window that has SKIP_TASKBAR set.  That seems kind of wasteful
since it wouldn't be used.  So we should probably go with Jochen's way--besides,
we can fix it up later if we want to do it some other route and we should get
this bug fixed.

Jochen: Can you clean up your patch to follow the coding style elsewhere in the
code?  (IIRC, you didn't follow the same indentation and had braces that weren't
on their own line--watch out for tabs (don't use 'em) and spaces before
parentheses when calling functions too).

Created attachment 52829 [details] [review]
cleaned patch

Created attachment 52916 [details] [review]
Cleaned up patch

Your patches still had some spacing issues (space before and after operators,
making sure if/else blocks align properly), but since I want to get this in
soon I just went and fixed those up.  I made one non-cosmetic change related to
the question you asked before about which windows to include--I think if they
would be included by wnck_tasklist_include_window were skip_taskbar not set,
then it ought to be included in the skipped list when skip_taskbar is set.  So,
I just broke up wnck_tasklist_include_window into two functions so we could
make use of the rest of the wnck_tasklist_include_window() functionality there.

Elijah: the patch makes sense. I only have two comments:

 static void
+wnck_tasklist_free_skipped_windows (WnckTasklist  *tasklist)
[...]
+  g_list_free (tasklist->priv->skipped_windows);
+  tasklist->priv->skipped_windows = NULL;
+}

I'd do the "tasklist->priv->skipped_windows = NULL;" not in this function, but
in the caller of this function. But since it's called twice, it make sense to do
it here so we don't forget. Not a big deal anyway :-)



@@ -1782,6 +1831,18 @@ wnck_tasklist_update_lists (WnckTasklist
 	  
 	  class_group_task->windows = g_list_prepend (class_group_task->windows,
win_task);
 	}
+      else if (tasklist_include_window_ignoring_skip_taskbar (tasklist, win))
[...]

So, it looks like this is what will happen:

  if (wnck_tasklist_include_window (tasklist, win)) {
  } else if (tasklist_include_window_ignoring_skip_taskbar (tasklist, win)) {

It means that tasklist_include_window_ignoring_skip_taskbar() will be called
twice if we're not in the first case. Wouldn't it make sense to just do:

  if (tasklist_include_window_ignoring_skip_taskbar (tasklist, win)) {
    if (wnck_window_get_state (win) & WNCK_WINDOW_STATE_SKIP_TASKLIST) {
    } else {
    }
  }

Hrm. It's probably less readable, though. You can commit in its current form if
you prefer :-)

Comment on attachment 52916 [details] [review]
Cleaned up patch

Elijah pinged me on this and it looks fine to me.

In general I'm OK with you guys (Elijah, Vincent) modifying libwnck without
consulting me btw, though I'm happy to help if there are questions.

Committed.