GNOME Bugzilla – Bug 700629
GtkComboBoxText <items> from GtkBuilder input is broken
Last modified: 2013-06-23 20:29:36 UTC
Created attachment 244703 [details] GtkBuilder file showing breakage The attached GtkBuilder file shows a GtkComboBox which sets the <items>, the combo seems to load the 2 items, and then fails to render them in the drop down menu. It would seem that the data is correctly in place, but for some reason the cell renderer is missing. To reproduce, fire up: glade-previewer -f combo-box-text-bug.ui (the attached file)
seems to work ok here ?
(In reply to comment #1) > seems to work ok here ? Strange. I've updated GTK+ master and with today's master I still see no 'item1' or 'item2' rendered by the combo box (problem remains). I'm half tempted to point at the theme setup, since I'm using stock GTK+ from a relocated jhbuild and no theme (and I suspect you are running with Adwaita)... however it doesn't really make much sense that this would be theme related since I use plenty of combo boxes which show up fine, only that the items are added dynamically and not declared with <items> in GtkBuilder.
I am seeing what looks to be the same thing with combos in evolution preferences on rawhide with a gtk snapshot from today. Can reproduce the supplied example breakage.
The evolution issue which lead me here https://bugzilla.gnome.org/show_bug.cgi?id=700629
Bisected to this commit e9f182e37a7f6e2dc339054841a3c9f930f573ed Author: Matthias Clasen <mclasen@redhat.com> Date: Sun Apr 28 21:43:49 2013 -0400 Fix a few memory leaks wrt to translations Just reverting on top of current master fixes the rendering of the evolution composer preferences comboboxes
*** Bug 701083 has been marked as a duplicate of this bug. ***
I ran into the same problem today and debugged it further, and I believe I understand what's going on here. The issue is that _gtk_builder_parser_translate() can now return the same pointer that is passed in, and that trips up the callers. from gtk/gtkcomboboxtext.c: translated = _gtk_builder_parser_translate (data->domain, data->context, data->string->str); g_string_set_size (data->string, 0); g_string_append (data->string, translated); Depending on the locale, in some cases the returned translated == data->string->str. In that case the code above does the following: 1) translated = a pointer to GString's internal character array 2) reset the size of GString to 0, which invalidates the internal character array 3) call append(), passing in the pointer to the now invalidated internal array I think the best way here would be go back to returning a strdup'ed string from _gtk_builder_parser_translate() and just have callers free it. Less suprises that way. I can do the patch later today if it makes sense.
(In reply to comment #7) > I ran into the same problem today and debugged it further, and I believe I > understand what's going on here. > > The issue is that _gtk_builder_parser_translate() can now return the same > pointer that is passed in, and that trips up the callers. > > from gtk/gtkcomboboxtext.c: > translated = _gtk_builder_parser_translate (data->domain, > data->context, > data->string->str); > g_string_set_size (data->string, 0); > g_string_append (data->string, translated); > [...] > > I can do the patch later today if it makes sense. Nice catch. As _gtk_builder_parser_translate() is an internal function I guess it doesn't matter if we change it's behaviour (and grep tells me there are only a hand full of usages of that function). I think I agree that reverting the behaviour would be safer (and more readable), since the original change was to avoid memory leaks, the patch should of course ensure that we don't have leaks around calls to _gtk_builder_parser_translate() (bug 699016).
Created attachment 246130 [details] [review] Memory allocation fixes for GtkBuilder translations This is another attempt to squash some memory leaks with GtkBuilder translations and at the same time fix regressions from the previous attempt. Commit e9f182e37a7f6e2dc339054841a3c9f930f573ed changed _gtk_builder_parser_translate() to avoid strdup() and just pass on the gettext() return value. Depending on if the translation was found, the return value would be either a pointer to a statically allocated string, or just the msgid parameter passed back. That led to some confusion with callers deallocating the msgid before using the return value -- sometimes it would work, sometimes it would also deallocate the returned value; depending on the locale. With LANG=C, GtkComboBoxText <items> from GtkBuilder started showing up as empty; with some other locales that had translations it would work. This commit goes back to the less suprising behaviour where _gtk_builder_parser_translate dup's the string and callers are responsible for freeing it.
What we came up with on irc is to replace g_string_set_size (parser_data->string, 0); g_string_append (parser_data->string, translated); with g_string_assign (parser_data->string, translated); instead. g_string_assign is self-assignment-safe.
Created attachment 246247 [details] [review] Fix string allocation handling with GtkBuilder translations Use g_string_assign to avoid issues with assigning GString's internal buffer back to the same string. This can happen when no translations are available and _gtk_builder_parser_translate returns back the same pointer that was passed in. This fixes a regression from commit e9f182e37a7f6e2dc339054841a3c9f930f that caused GtkComboBoxText <items> from GtkBuilder to show up empty if no translations are available.
Review of attachment 246247 [details] [review]: thanks, looks good
Thanks! Attachment 246247 [details] pushed as a374501 - Fix string allocation handling with GtkBuilder translations
*** Bug 701933 has been marked as a duplicate of this bug. ***