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 625300 - Buttons order in dialogs broken in gtk+3
Buttons order in dialogs broken in gtk+3
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
2.90.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2010-07-26 11:09 UTC by Carlos Garcia Campos
Modified: 2010-08-05 07:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add _gtk_box_get_children() internal function (2.75 KB, patch)
2010-07-26 13:27 UTC, Carlos Garcia Campos
accepted-commit_now Details | Review

Description Carlos Garcia Campos 2010-07-26 11:09:42 UTC
This commit 

http://git.gnome.org/browse/gtk+/commit/?id=a46c1eba6413b676c15103b38a85ba686c230161 

broke the button order in dialogs. Using gtk_container_get_children() is not equivalent to use the children list of GtkBox, because gtk_container_get_children uses GtkContainerClass::forall which is implemented by GtkBox returning first the children packed at start and then the one packed at end. Also, gtk_container_get_children() returns a new list that is not freed. 

Assuming we can't change the behaviour of forall in GtkBox, we could just add a new accessor to GtkBox to return the children list as a const GList * which is exactly what we were doing and we avoid iterating over the list and allocating a new one.
Comment 1 Emmanuele Bassi (:ebassi) 2010-07-26 11:36:24 UTC
something like:

  G_CONST_RETURN GList *gtk_box_peek_children (GtkBox *box);

should be enough.
Comment 2 Emmanuele Bassi (:ebassi) 2010-07-26 11:39:04 UTC
it could even be an internal function only, now that I think of it.
Comment 3 Emmanuele Bassi (:ebassi) 2010-07-26 11:41:18 UTC
one problem with the "return an internal pointer": GtkBox stores a list of GtkBoxChild, which is a private structure. we cannot really return that any more - so we need to build a GList from the internal one.
Comment 4 Carlos Garcia Campos 2010-07-26 12:10:57 UTC
so, GList *_gtk_box_get_children (GtkBox *box); then? an internal function returning a new list. I'll fix the leaks too.
Comment 5 Carlos Garcia Campos 2010-07-26 13:27:40 UTC
Created attachment 166582 [details] [review]
Add _gtk_box_get_children() internal function

It seems leaks were already fixed in git master, so it just adds _gtk_box_get_children() internal function
Comment 6 Matthias Clasen 2010-08-05 01:13:28 UTC
Review of attachment 166582 [details] [review]:

Ugh, indeed.

This patch is a step in the right direction, but gtkstatusbar.c:has_extra_children needs one more fix:
It has an early exit in the loop, which leaks the list. Can you fix that while you commit this ?
Comment 7 Carlos Garcia Campos 2010-08-05 07:53:03 UTC
Pushed to git master 76267f9d0621c07b13e56f6ffd93e942111dfbcf