GNOME Bugzilla – Bug 724407
Fix critical warning when using GtkMenuButton with popover
Last modified: 2014-02-19 23:02:46 UTC
Either the attached patch or popover_set_relative_to should be changed and annotated to accept null
Created attachment 269192 [details] [review] patch
Created attachment 269551 [details] [review] popover: Accept NULL relative_to widgets And document the fact that the popover will get destroyed if a NULL relative-to is given on a parented popover, if no extra references are kept. For gtk_popover_new*(), a NULL relative-to will leave the widget as a floating object, to be sunk by a later call to gtk_widget_set_relative_to().
Being the popover lifetime so attached to its relative-to widget, I was at first hesitant about allowing none there, but seeing how it's being practically used on GtkMenuButton, it wouldn't have such a different semantics from gtk_menu_attach/detach in the end, just that it's comprised in a single call here. And this looks like it could be a common usage pattern, so I guess it's best to just document the oddities and at least allow widgets to do this.
Review of attachment 269551 [details] [review]: other than that, I agree with this patch ::: gtk/gtkpopover.c @@ +1632,3 @@ + * widget, so if @relative_to is set to %NULL on an attached @popover, it + * will be detached from its previous widget, and consequently destroyed + * unless extra references are kept. Might be good to have a similar note for gtk_popover_new, explaining that the returned reference is already owned by @relative_to ?
Review of attachment 269551 [details] [review]: ::: gtk/gtkpopover.c @@ +1632,3 @@ + * widget, so if @relative_to is set to %NULL on an attached @popover, it + * will be detached from its previous widget, and consequently destroyed + * unless extra references are kept. that's quite unusual in gtk, maybe it is more clear to drop the relative_to arg in new() ? It is slightly less convenient in C, but I think it would less "special"
Comment on attachment 269551 [details] [review] popover: Accept NULL relative_to widgets Attachment 269551 [details] pushed as 63bb834 - popover: Accept NULL relative_to widgets