GNOME Bugzilla – Bug 423201
gtk_combo_box_entry_active_changed does not transform values
Last modified: 2010-10-15 21:01:00 UTC
If you create a GtkComboBoxEntry with a text column that is not a G_TYPE_STRING, the application SEGVs when the active iter changes. There is no warning about the incompatible column type, and no checking of the type. In gtk_combo_box_entry_active_changed(), the code simply assumes that it got a string back from the model, and passes that directly to the Entry. This violates the Principle of Least Surprise. You can use a GtkCellRendererText to display a column of nearly any type, and the value will be coerced into a string by GValue's transformations. A developer may reasonably expect that GtkComboBoxEntry would do the same, especially since there is no warning or critical assertion about the type mismatch. A trivial patch to gtkcomboboxentry.c fixes this. The question is, am i right in thinking that the combobox should do this coercion? Or should the user simply use a G_TYPE_STRING column for the ComboBoxEntry's "text-column"? The bug is marked as affecting 2.10, but i have verified that it affects both FC2's 2.4.x and trunk@HEAD.
Created attachment 85364 [details] Simple test case This is a very simple test case that demonstrates the bug. Simply select something from the dropdown, and you get a SIGSEGV in strcmp() from gtk_combo_box_entry_active_changed().
Created attachment 85365 [details] [review] Patch to coerce values to strings Straightforward patch to use the object property system to trigger GValue transformation of the model's value, instead of assuming that it is a G_TYPE_STRING. Patterned after similar code in gtktreeviewcolumn.c.
Muppet, please add the changelog. Thanks :)
Like this? Or do you want that in the patch, as well? 2007-03-27 muppet <scott@asofyet.org> * gtk/gtkcomboboxentry.c: (gtk_combo_box_entry_active_changed): Use property system to coerce model data to G_TYPE_STRING. Bug #423201.
avoiding a segmentation fault should be a reason enough to apply this patch; either this or a g_critical() is added and the fact that no coercion is done should be written in the documentation.
Created attachment 109970 [details] [review] Updated Muppet's patch Updated Muppet's patch to head. It still works as advertised. No crashes and non-string columns are coerced.
Is there any reason why this can't be committed?
Created attachment 172426 [details] [review] Use property system to coerce model data to G_TYPE_STRING Updated patch against master.
Review of attachment 172426 [details] [review]: Looks fine to me.