GNOME Bugzilla – Bug 532636
Segfault loading message dialog
Last modified: 2008-09-16 18:32:18 UTC
loading a glade file with a message dialog with glade 3.4.x causes a crash when realizing the embedded message dialog. 0x00007faf28155a55 in ?? () from /usr/lib/libgtk-x11-2.0.so.0 (gdb) where
+ Trace 197462
Created attachment 110793 [details] This message dialog works well
My trace differs from yours, but anyway, seems like "image" property is the problem. Initially it's an object outside the project, so it cannot be saved or loaded until we set it to project's image widget. Currently the "image" property: - is initialized to itself on the message dialog creation; - is saved as an empty value if have not been modified (empty value is invalid); - can be set to any project's widget, causing glade to fail on the next load of the project.
Created attachment 110924 [details] [review] possible fix Seems like glade is not supposed to handle "image" property, so it should be disabled. I've tested this patch with glade3 svn r1823 of gnome-2-22 branch.
Possibly using the "ignore" property for this would work better, and also allow us to set the image property to a custom widget to be embedded in the dialog. there is however a remaining problem, images should not be saved to the glade file when they are merely NULL strings...
I couldn't find any example of usage of the "image" property. I cannot see any possibility of using this property in glade, because a message dialog reparents a widget, so it becomes a child.
So how do you propose to use a custom image for the message dialog without using the "image" property ?
I misinformed you that the widget is reparented on set_image call. In fact it is just added to the container and should not have a parent. Do you have an idea how to use a custom image for the message dialog _using_ the "image" property?
Yes, its not trivial but were gonna have to do it eventually anyway. First thing needs to be done, you have to be allowed to have toplevel non-GtkWindow objects, this is currently unsupported (and by unsupported I mean, there is basically some "if" statments that deny the possibility, other than that enabling them might introduce some unexpected misc bugs). Then we need to enhance the objects selection dialog a bit, so that we can deny use of objects that are already parented in this type of case. Then the question remains, should the message dialog actually parent the image in the glade runtime ? or just leave that to a possible preview feature (that we'll probably need to display menus created by UIManager anyway), either we deal with the possible side effects of parenting a toplevel project object into another one via its property - or we just display a custom icon in the message dialog signifying there is a widget specified for that place.
Created attachment 111685 [details] [review] BFPatch There are still some minor problems with changing of contruction-only properties. Comments about variables/functions names and messages are strongly appreciated. Is there any debugging/logging functions in glade? I've found that I spent a lot of time writing code for debugging output. Why don't you use gboolean as a result of glade_widget_adaptor->set_property?
I tried to make the patch as clean as possible, but unfortunately I've found some useless diff: - I've already sent a patch for src/glade-window.c to another bug; - diff for gladeui/glade-editor-property.c is dummy.
Created attachment 115713 [details] [review] Patch v2 Here is a new patch. Short description: like a previous one it adds support for parentless widgets and implements "image" property of GtkMessageDialog; the difference is that this patch really works.
I know I forgot to mention this at any point, but usually we ask you to provide ChangeLog entries with your patches, so please do so in the future (the other patches already attached I think its not so important I can write it up), but here I think it will help, for instance looking at this patch I have no idea why its modifying glade-command.c. Anyway, this is a big patch, and another one I think is really important, but I'll need comments in the changelog about apis that are added/changed, and a little bit of detail about what it does under the hood, basically, just a good normal detailed changelog will let me properly review the patch. Thanks.
I hope this will be enough: * gladeui/glade-app.c Creating toplevel widget through unified glade-palette interface. Removed error message when pasting non-toplevel widgets without a parent * gladeui/glade-command.h A new function for getting depth of command recursion * gladeui/glade-command.c A new function for getting depth of command recursion. A return value of glade_command_set_property_execute function is valid and is respected by glade_command_set_properties_list. indicate success/failure of their execution. Glade-command can be unified to null. Setting property command is always executed as a group, so if there is any recursive command, they will be added to that group. Removed parentless widget message level lowered from critical to message. Widget is treated as toplevel only if it has no parent. Removed an assertion from glade_command_create that doesn't allow creation of non-GtkWindow parentless widgets * gladeui/glade-editor-property.c Object selection dialog will be optionally filled by parentless non-GtkWindow widgets only Unparenting root widgets before setting another property to them * gladeui/glade-inspector.c Popup for clicking even on empty part of widget list * gladeui/glade-palette.h: A unified function for creating root widgets * gladeui/glade-palette.c A unified function for creating root widgets A new button for creating root widgets * gladeui/glade-placeholder.[ch]: glade_placeholder_get_project has been made public for using in glade-popup * gladeui/glade-popup.c: A new function glade_popup_simple_pop for creating a context menu on an empty space of glade-inspector New context menu items for adding widgets * gladeui/glade-popup.h: A new function glade_popup_simple_pop for creating a context menu on an empty space of glade-inspector * gladeui/glade-project.c Unifying command even if there's redo items. Unifying atomic commands only. Unifying to null * gladeui/glade-property-class.[ch] A new field for making properties that points to parentless widgets * gladeui/glade-property.h Added a return value to glade_property_set* functions to indicate success/failure that is used in glade-command * gladeui/glade-property.c Ignoring parentless_widget properties while duplicating properties. Additional check while adding/removing property reference Added a return value to glade_property_set* functions to indicate success/failure that is used in glade-command. Determining that property is changed using glade-proproperty method instead of direct comparing GValue. Loading properties through glade-widget-adaptor interface instead of getting them directly. remove_object method now unsets referencing property instead of setting it. Removed dummy duplicated setting of property while unsetting referencing property. * gladeui/glade-property.h Added a return value to glade_property_set* functions to indicate success/failure that is used in glade-command. * gladeui/glade-widget.c Removed setting widget properties to template/default values while building a new object, because they will be set later in constructor. Reloading properties after duplicating a widget. A new function for removing parent reference, that was made by setting parentless_widget property to the widget. Saving and loading parentless_widget properties while rebuilding, because they cannot be duplicated. Corrected destroying of an old widget while rebuilding. Seems like it's not fully correct still. * gladeui/glade-widget.h A new function for removing parent reference, that was made by setting parentless_widget property to the widget. * gladeui/glade-xml-utils.h A new tag "parentless-widget" as a property attribute * plugins/gtk+/glade-gtk.c, plugins/gtk+/gtk+.xml.in: Removed an old hack for "image" property of GtkMessageDialog. A new implementation of "image" property using parentless_widget kind of property. Added an ability of working with parentless widgets using "remove parent" and "add parent" items of context menu
Ok I read it all and reviewed the whole patch, I committed the whole thing except changed one thing; in the case when we set a property to an object that has other references set, we lookup the reference via gladewidget but call glade_command only from glade-editor-property.c, this way I was able to group the two commands in the object dialog code. quite honestly, this is just about exactly how I wanted it done, thanks :) There are a few things I will want to address but the basic functionality is there (i.e. I'll want to add context menus to the palette, we never have that selector button showing in glade so thats a problem... also need to revamp the objects dialog to select object class specific and display it properly, and warn if you are going to "steal" an image from another dialog... the list will go on and on ... )