GNOME Bugzilla – Bug 669116
GtkNotebook's child-notify::position not always emitted as expected
Last modified: 2012-02-16 10:33:31 UTC
Created attachment 206523 [details] test case Unless I'm missing the obvious, I'd say that "child-notify" is not working at all.
Wrong conclusion. If you connect to child-notify on the other button, you will see it getting emitted. Its just that gtknotebook is not very good about emitting child-notify on other children that are affected by reordering.
Just spent some minutes with this, and yes, I obviously connected the signal handler to the wrong button. However, child-notify in GtkNotebook is not very useful to keep track of the position of the notebook children soundly, apparently it's not even emitted when manually reordering the tabs.
Created attachment 206995 [details] [review] GtkNotebook: emit child-notify::position a few more times When moving a page around, all children changing their position need to be notified. There are still other places where proper notification is missing (drag 'n drop, etc.)
Review of attachment 206995 [details] [review]: ::: gtk/gtknotebook.c @@ +8020,3 @@ + list = g_list_nth (priv->children, MIN (old_pos, position)); + for (i = ABS (old_pos - position) + 1; i > 0; i--, list = list->next) + gtk_widget_child_notify (((GtkNotebookPage *) list->data)->child, "position"); I think I would find this easier to follow as a straight loop over the children, like for (list = priv->children, i = 0; list; list = list->next, i++) { if (MIN (old_pos, position) <= i && i <= MAX (old_pos, position)) gtk_widget_child_notify (...) }
Created attachment 207237 [details] [review] GtkNotebook: emit child-notify::position a few more times When moving a page around, all children changing their position need to be notified. There are still other places where proper notification is missing (drag 'n drop, etc.)
Review of attachment 207237 [details] [review]: Thanks, looks good
Comment on attachment 207237 [details] [review] GtkNotebook: emit child-notify::position a few more times Attachment 207237 [details] pushed as e2339f5 - GtkNotebook: emit child-notify::position a few more times
Created attachment 207258 [details] [review] GtkNotebook: emit child-notify::position on page add/removal For each page added/removed, notify all the other children changing position.
Created attachment 207528 [details] [review] GtkNotebook: emit child-notify::position on drag 'n drop reorder
I think that should do it to close this bug, unless I'm missing other entry points to reordering..
Review of attachment 207258 [details] [review]: Looks right
Review of attachment 207528 [details] [review]: Looks right, too
Thank you, Matthias. Pushing and closing the bug. Attachment 207258 [details] pushed as cb775a6 - GtkNotebook: emit child-notify::position on page add/removal Attachment 207528 [details] pushed as 347328a - GtkNotebook: emit child-notify::position on drag 'n drop reorder
Review of attachment 207237 [details] [review]: ::: gtk/gtknotebook.c @@ +8018,3 @@ gtk_notebook_child_reordered (notebook, page); + + for (list = priv->children, i = 0; list; list = list->next) I forgot to increase the counter here, my bad. I'm fixing this in master.
The following fix has been pushed: ccf7867 GtkNotebook: fix one child-notify emission
Created attachment 207741 [details] [review] GtkNotebook: fix one child-notify emission Forgot to increase the counter in the for loop, doing it now.
The following fix has been pushed: cfe65a0 GtkNotebook: and another fix
Created attachment 207742 [details] [review] GtkNotebook: and another fix