After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 778617 - GtkListBox: placeholder is not removed properly
GtkListBox: placeholder is not removed properly
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
3.89.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks: 778535
 
 
Reported: 2017-02-14 17:28 UTC by Kjell Ahlstedt
Modified: 2017-04-26 10:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Text case (1.55 KB, text/x-csrc)
2017-02-14 17:28 UTC, Kjell Ahlstedt
  Details
patch: listbox: Remove placeholder when destroying (2.96 KB, patch)
2017-02-28 15:50 UTC, Kjell Ahlstedt
none Details | Review
listbox: Properly remove placeholders (1.21 KB, patch)
2017-04-24 17:17 UTC, Timm Bäder
none Details | Review

Description Kjell Ahlstedt 2017-02-14 17:28:29 UTC
Created attachment 345744 [details]
Text case

When the C++ wrapper of a gtk+ widget is deleted in gtkmm,
g_object_run_dispose() is called, to dispose of the gtk+ object.
This works well in most cases, but not for a GtkListBox with a placeholder
widget. The result differs, depending on the order of deletion and it's also
different in gtk+3 and gtk+4.

1. gtk+3

1.1 First dispose of the listbox: Nothing unexpected happens.
    Then dispose of the placeholder: Segfault

1.2 First dispose of the placeholder:
      Gtk-WARNING **: Tried to remove non-child 0x55e8f7dcf3f0
    Then dispose of the listbox: Nothing unexpected happens.

2. gtk+4

2.1 First dispose of the listbox:
      Gtk-WARNING **: Finalizing GtkListBox 0x55fd39864210, but it still has children left:
      Gtk-WARNING **:    - GtkLabel 0x55fd39710390
    Then dispose of the placeholder: Segfault

2.2 First dispose of the placeholder:
      Gtk-WARNING **: Tried to remove non-child 0x55aa9936e390
    Then dispose of the listbox:
      Gtk-WARNING **: Finalizing GtkListBox 0x55aa994c2240, but it still has children left:
      Gtk-WARNING **:    - GtkLabel 0x55aa9936e390

I think a problem with the placeholder is that it does not quite fit into either
of the child groups "internal" or "non-internal" (or external) children.
The documentation of gtk_contatiner_foreach() says internal children are
"precisely those child widgets that were added to the container by the
application with explicit add() calls."
A placeholder was added by the application, but not with an add() call.

When the listbox is disposed of, gtk_list_box_forall() is called with
include_internals == FALSE and callback == gtk_widget_destroy.
The placeholder is not destroyed, as it's considered an internal child.

When the placeholder (in the test case a GtkLabel) is disposed of,
gtk_list_box_remove() is called. The placeholder is not removed, as it's not
considered a child.
Comment 1 Kjell Ahlstedt 2017-02-28 15:50:16 UTC
Created attachment 346912 [details] [review]
patch: listbox: Remove placeholder when destroying

This patch applies to the master branch. I don't know if it also applies to the
gtk-3-22 branch.

The patch adds gtk_list_box_destroy(), where a placeholder widget, if any, is
unparented. I don't know if destroy is the best place for this code.
Other container widgets with internal children unparent such children either
in the destroy, dispose or finalize function.

In comment 0: s/Text case/Test case/
Comment 2 Timm Bäder 2017-04-24 17:16:19 UTC
> The patch adds gtk_list_box_destroy(), where a placeholder widget, if any, is
> unparented. I don't know if destroy is the best place for this code.
> Other container widgets with internal children unparent such children either
> in the destroy, dispose or finalize function.

You are right that internal widgets should be unparented in destroy, at least according to the GtkContainer documentation, but it doesn't seem like the placeholder is really an internal child to me.

I prepared another patch for this that just changed the placeholder to a non-internal child. As far as I understand the GtkContainer documentation, it's not an internal child since it has been added by the user. But I don't know if that is usable for gtk-3-22.

I'm just going to attach that patch as well.
Comment 3 Timm Bäder 2017-04-24 17:17:09 UTC
Created attachment 350315 [details] [review]
listbox: Properly remove placeholders
Comment 4 Kjell Ahlstedt 2017-04-25 17:42:08 UTC
Yes, your patch is a good fix. I've tested it in the master branch and in the
gtk-3-22 branch, also with the test case in the original gtkmm bug 778535.