GNOME Bugzilla – Bug 576091
GtkTooltip destroy the custom widget
Last modified: 2009-05-25 07:49:40 UTC
Please describe the problem: when finalized a tooltip destroy its window and so all the widgets inside. Which I think is hazardous since the tooltip does not create the custom widget it should not assume its destruction since the user can still want to keep it. Steps to reproduce: 1. Create a tooltip with custom widget 2. Show the tooltip and hide it 3. Retry to show the tooltip Actual results: The custom widget was destroyed : the apps SegV Expected results: before finalize, the tooltip should remove and unref the custom widget Does this happen every time? yes Other information: following patch : in order to keep my widget, before destroying the window I remove the custom widget from the tooltip. Here is the diff : (in function gtk_tooltip_finalize ) 195,211d194 < /* FIXEME : in some case the user can want to keep its custom widget < since we don't create it, we should not assume its destruction < we remove it from the tooltip so, if the user does not need it anymore, < it will be finalize (ref_cout == 0), otherwise it will be kept unless < the user unref it. < */ < if (tooltip->custom_widget) < { < GtkWidget *custom = tooltip->custom_widget; < /* Note: We must reset tooltip->custom_widget first, < * since gtk_container_remove() will recurse into < * gtk_tooltip_set_custom() < */ < tooltip->custom_widget = NULL; < gtk_container_remove (GTK_CONTAINER (tooltip->box), custom); < g_object_unref (custom); < } This code is actually a duplication of what gtk_tooltip_set_custom does when setting a new custom widget. This code works, I do not see any memory leak in my test.
Can you generate your patch with patch -up and put it as an attachment? Otherwise it is hard to follow it.
Created attachment 131046 [details] [review] patch-up file This is the patch up file that include the correction to remove the custom widget before destroying the tooltip window I made a function to avoid code duplication in gtk_tooltip_set_custom and gtk_tooltip_finalize : void gtk_tooltip_remove_custom (GtkTooltip *tooltip) (see at end of file)
Makes sense to me, but we also need to clarify the documentation of gtk_tooltip_set_custom. Also, please provide a patch instead of a full source file.
Created attachment 133995 [details] [review] Remove custom widget before destroy the tooltip Could people review and commit it. My work is blocked by this bug.
Patch looks ok to me, but as I said above, please clarify the documentation of gtk_tooltip_set_custom() - it needs to mention that custom_widget may be NULL, and that custom_widget does not get destroyed when the tooltip goes away. Since this is a slight change of behaviour, it is probably worth a note in README.in. Please add a 'Release notes for 2.18' section for this.
(In reply to comment #5) > Patch looks ok to me, but as I said above, please clarify the documentation of > gtk_tooltip_set_custom() - it needs to mention that custom_widget may be NULL, > and that custom_widget does not get destroyed when the tooltip goes away. > > Since this is a slight change of behaviour, it is probably worth a note in > README.in. Please add a 'Release notes for 2.18' section for this. > I will update this patch. But I'm not a native speaker, so please review the doc and feel free to revise it.
Created attachment 134383 [details] [review] patch
Guys, are there any objections? Can anyone commit it?
Looks ok to me.
Matthias, my understanding for 'accepted-commit_now' is to let me commit. If I'm wrong, feel free to back it out. Thanks ;)
You need to close the bug and mark the patch as committed after pushing it. Doing it now.