GNOME Bugzilla – Bug 585299
Crash clicking spin button below layout style for GtkDialog
Last modified: 2010-12-18 10:35:19 UTC
To reproduce: 1. Open Glade 2. Accept default options 3. Create GtkDialog 4. Select action-area in dialog 5. Click up arrow on spin button below "layout style" Outcome: Segfault every time Expected outcome: This spin button has no label but for a standard horizontal button box it would be "Spacing".
Stack trace from valgrind: ==18513== Process terminating with default action of signal 11 (SIGSEGV) ==18513== Access not within mapped region at address 0x18 ==18513== at 0x40951BC: glade_command_set_property_value (glade-command.c:734) ==18513== by 0x4083816: glade_editor_property_commit_common (glade-editor-property.c:94) ==18513== by 0x48CD1DB: g_cclosure_marshal_VOID__POINTER (gmarshal.c:601) ==18513== by 0x48BD748: g_type_class_meta_marshal (gclosure.c:878) ==18513== by 0x48BF0C1: g_closure_invoke (gclosure.c:767) ==18513== by 0x48D579A: signal_emit_unlocked_R (gsignal.c:3285) ==18513== by 0x48D6E8C: g_signal_emit_valist (gsignal.c:2980) ==18513== by 0x48D7345: g_signal_emit (gsignal.c:3037) ==18513== by 0x407D027: glade_editor_property_commit (glade-editor-property.c:3563) ==18513== by 0x4081F52: glade_editor_property_commit_no_callback (glade-editor-property.c:118) ==18513== by 0x4083371: glade_eprop_numeric_changed (glade-editor-property.c:734) ==18513== by 0x48CD11B: g_cclosure_marshal_VOID__VOID (gmarshal.c:77)
Ubuntu bug link: https://bugs.launchpad.net/ubuntu/+source/glade-3/+bug/372058
does this bug still reproduce?
Yes this bug is still reproducible. I did some investigation on the matter. Commenting line glade_widget_remove_property (actionarea_widget, "spacing"); in the file glade-gtk.c is enough to make it go away. For some strange reason, remove_property is not working as intended. According to comments in the file, these properties should not be settable by the action_area, but by the dialog itself. It should be "semantically" replaced by a property in the dialog called "button-spacing", but I don't see where this is settable. It looks like it is more fundamental problem. Maybe, the simplest solution on the short term is to just remove the remove_property call. Is there anything wrong with this?
(In reply to comment #4) > Yes this bug is still reproducible. I did some investigation on the matter. > Commenting line glade_widget_remove_property (actionarea_widget, "spacing"); in > the file glade-gtk.c is enough to make it go away. > > For some strange reason, remove_property is not working as intended. According > to comments in the file, these properties should not be settable by the > action_area, but by the dialog itself. It should be "semantically" replaced by > a property in the dialog called "button-spacing", but I don't see where this is > settable. No it should not be settable, note the comment that they are GtkDialog "style properties", style properties are controlled by the theme, not by the user. > It looks like it is more fundamental problem. Maybe, the simplest solution on > the short term is to just remove the remove_property call. Is there anything > wrong with this? Yes I'd rather have the bug fixed properly than regress into letting users configure properties manually which are in fact controlled by the theme. Editors need to check that all it's properties are available on the widget instance and if not they need to hide certain properties.
Also GladeEditorProperty should not crash if its not backed with a widget.
Adding if (eprop->property == NULL) return; to glade_editor_property_commit_common in glade-editor-property.c fixes the crash. But the bug that makes the property to be incorrectly removed is still there. I really have no idea on why the property is not removed.
Its because GladeEditor caches the editors by class (they are not re-created every time that the editor is displayed). So in GladeEditable->load() it would be important for classes to hide eprop's for properties that dont exist on the instance. (probably only the glade-editor-table.c class needs to check this).
Hmmm... Is the bug easily fixable then?
yes glade_editor_table_load() for (l in table->properties) { if (widget has property) show() else hide() }
It doesn't seems so easy. The first column of the editor is created by glade_editor_table_load which know which widget is been is currently been displayed. The other column of the editor is created by append_items that only knows about the adaptor. Is there a way to get the widget from the adaptor?
glade-editor-table.c:225 calls glade_editor_property_load_by_widget(). thats where the widget data is loaded to the eprop and the eprop goes and connects to the property. in this code you need to check if the GladeWidget "instance" has a GladeProperty "instance" the editor will always have an instance of an eprop for every propertyclass that is on a widget adaptor.
I think it is not enough. Even jumping the for in glade-editor-table.c:222, the second column of the table is still created.
Hmmm, now I understand... the property can be hidden with a gtk_widget_hide. All that is missing now is a way to know if a widget has, indeed, such property.
Created attachment 176642 [details] [review] Hides properties that should be hidden
The (previously) attached patch fixes the problem. Some warnings do appear, I can't what is causing them.
Created attachment 176643 [details] [review] Hides properties that should be hidden The attached patch fixes the bug. Its an small improvement over the last one following Tristan directions on how to fix the bug correctly. Please commit it.
Applied, thanks :)