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 783483 - Remove explicit destroy of label/entry children
Remove explicit destroy of label/entry children
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: st
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2017-06-06 15:38 UTC by Cosimo Cecchi
Modified: 2017-06-06 19:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Remove dangerous explicit destroy (1.17 KB, patch)
2017-06-06 15:38 UTC, Cosimo Cecchi
committed Details | Review
Remove an explicit destroy to the ClutterText actor (1.08 KB, patch)
2017-06-06 15:38 UTC, Cosimo Cecchi
committed Details | Review

Description Cosimo Cecchi 2017-06-06 15:38:42 UTC
See attached patches.
Comment 1 Cosimo Cecchi 2017-06-06 15:38:46 UTC
Created attachment 353264 [details] [review]
Remove dangerous explicit destroy

There's no need to explicitly destroy the ClutterText actor inside the
label; doing so is actually harmful, as it will break the normal
reference cycle between container and children.
As StLabel doesn't hold any extra reference to the ClutterText actor and
just uses clutter_actor_add_actor() to add it to itself, let the normal
container dispose cycle run to dispose of the reference.
Comment 2 Cosimo Cecchi 2017-06-06 15:38:50 UTC
Created attachment 353265 [details] [review]
Remove an explicit destroy to the ClutterText actor

This is the same as the previous commit, but for StEntry.
We don't have any need to explicitly destroy this actor in our dispose
implementation, and doing so breaks the assumption that we can access
the clutter_text from within destroy.
Comment 3 Florian Müllner 2017-06-06 19:16:59 UTC
Review of attachment 353264 [details] [review]:

LGTM
Comment 4 Florian Müllner 2017-06-06 19:17:09 UTC
Review of attachment 353265 [details] [review]:

LGTM
Comment 5 Cosimo Cecchi 2017-06-06 19:40:07 UTC
Attachment 353264 [details] pushed as ad2cb22 - Remove dangerous explicit destroy
Attachment 353265 [details] pushed as e2838a7 - Remove an explicit destroy to the ClutterText actor