GNOME Bugzilla – Bug 572756
Remove deprecated GTK+ symbols
Last modified: 2009-06-24 00:03:18 UTC
http://live.gnome.org/GnomeGoals/RemoveDeprecatedSymbols/GTK%2B GTK_OBJECT_FLOATING, gtk_action_connect_proxy, gtk_entry_set_editable, gtk_menu_item_remove_submenu, gtk_notebook_set_page, gtk_object_get_data, gtk_object_sink, gtk_option_menu_new, gtk_option_menu_set_history, gtk_option_menu_set_menu, gtk_recent_manager_get_for_screen, gtk_timeout_add, gtk_tool_item_set_tooltip, gtk_type_new, gtk_widget_get_action To potential patch contributors: It might make sense to break this into several smaller patches. :-P When patching also make sure that the gtk and glib versions in configure.in/.ac are high enough and take a look since which version the new non-deprecated functions are available. http://library.gnome.org is your friend here. :-) (Note: Some lines may be listed twice because they contain two deprecated symbols) $:andre\> grep -r GTK_OBJECT_FLOATING . ./plugins/gtk+/glade-gtk.c: if (GTK_OBJECT_FLOATING (child)) ./gladeui/glade-command.c: if (GTK_OBJECT_FLOATING (cdata->placeholder)) $:andre\> grep -r gtk_action_connect_proxy . ./src/glade-window.c: gtk_action_connect_proxy (action, GTK_WIDGET (priv->undo)); ./src/glade-window.c: gtk_action_connect_proxy (action, GTK_WIDGET (priv->redo)); $:andre\> grep -r gtk_entry_set_editable . ./plugins/gtk+/glade-cell-renderer-button.c: gtk_entry_set_editable (GTK_ENTRY (text_button->entry), priv->entry_editable); ./plugins/gtk+/glade-accels.c: gtk_entry_set_editable (GTK_ENTRY (eprop_accel->entry), FALSE); ./gladeui/glade-editor-property.c: gtk_entry_set_editable (GTK_ENTRY (eprop_flags->entry), FALSE); ./gladeui/glade-editor-property.c: gtk_entry_set_editable (GTK_ENTRY (eprop_color->entry), FALSE); ./gladeui/glade-editor-property.c: gtk_entry_set_editable (GTK_ENTRY (GTK_BIN (combo)->child), FALSE); ./gladeui/glade-editor-property.c: gtk_entry_set_editable (GTK_ENTRY (eprop_object->entry), FALSE); ./gladeui/glade-editor-property.c: gtk_entry_set_editable (GTK_ENTRY (eprop_objects->entry), FALSE); ./gladeui/glade-editor-property.c: gtk_entry_set_editable (GTK_ENTRY (eprop_adj->entry), FALSE); $:andre\> grep -r gtk_menu_item_remove_submenu . ./plugins/gtk+/glade-gtk.c: gtk_menu_item_remove_submenu (GTK_MENU_ITEM (object)); $:andre\> grep -r gtk_notebook_set_page . ./gladeui/glade-editor-property.c: gtk_notebook_set_page (GTK_NOTEBOOK (eprop_adj->notebook), 0); ./gladeui/glade-editor-property.c: gtk_notebook_set_page (GTK_NOTEBOOK (eprop_adj->notebook), 1); $:andre\> grep -r gtk_object_get_data . ./gladeui/glade-utils.c: ltext = (gchar *) gtk_object_get_data (GTK_OBJECT (listitem), $:andre\> grep -r gtk_object_sink . ./plugins/gtk+/glade-gtk.c: gtk_object_sink (GTK_OBJECT (child)); ./gladeui/glade-property-class.c: gtk_object_sink (GTK_OBJECT (widget)); $:andre\> grep -r gtk_option_menu_new . ./gladeui/glade-editor-property.c: eprop_enum->option_menu = gtk_option_menu_new (); $:andre\> grep -r gtk_option_menu_set_history . ./gladeui/glade-editor-property.c: gtk_option_menu_set_history (GTK_OPTION_MENU (eprop_enum->option_menu), $:andre\> grep -r gtk_option_menu_set_menu . ./gladeui/glade-editor-property.c: gtk_option_menu_set_menu (GTK_OPTION_MENU (eprop_enum->option_menu), menu); $:andre\> grep -r gtk_recent_manager_get_for_screen . ./src/glade-window.c: window->priv->recent_manager = gtk_recent_manager_get_for_screen (screen); ./src/glade-window.c: priv->recent_manager = gtk_recent_manager_get_for_screen (gtk_widget_get_screen (GTK_WIDGET (window))); $:andre\> grep -r gtk_timeout_add . ./gladeui/glade-utils.c: gtk_timeout_add (flash_length, (GtkFunction) remove_message_timeout, fi); $:andre\> grep -r gtk_tool_item_set_tooltip . ./src/glade-window.c: gtk_tool_item_set_tooltip (GTK_TOOL_ITEM (button), ./src/glade-window.c: gtk_tool_item_set_tooltip (GTK_TOOL_ITEM (button), $:andre\> grep -r gtk_type_new . ./gladeui/glade-clipboard-view.c: view = gtk_type_new (glade_clipboard_view_get_type ()); $:andre\> grep -r gtk_widget_get_action . ./src/glade-window.c: action = gtk_widget_get_action (item);
Created attachment 131055 [details] [review] partial fix - all deprecated gtk+ symbols that are replaced in glib Hello! Here is a patch that fixes GTK_FLOATING, gtk_object_sink, gtk_object_get_data and gtk_type_new More on the way.
Created attachment 131056 [details] [review] partial fix - all deprecated gtk+ symbols that are replaced in glib (all of them this time) That was bound to happen, I already broke my own categorisation rule. Replacement for 1st patch that also changes gtk_timeout_add.
Created attachment 131058 [details] [review] partial fix - all gtk+ symbols apart from a couple of awkward ones. replaces everything except those needing GtkActionable (need to bump GTK-version to 2.16) and GtkOptionMenu (which is slightly more in depth)
Created attachment 131061 [details] [review] partial fix part 3 - replace GtkOptionMenu with GtkComboBox Use a combo box for enum property editor. Tested and seems to work fine. I also cheated and this patch fixes some casts I forgot to change in the last one. I'll redo this and the last patch if needed, but I thought it might be better to minimise bugspam instead :) Last patch is to use GtkActionable - I need to build gtk-2.16 and all its deps to test this so it will take a while to cook. In the meantime, I've noticed a couple of other retro GTK calls: * gtk_combo_find and gtk_combo_func in gladeui/glade-utils.h, that operate on a GtkCombo and don't appear to be used by anything. * I got a bunch of "GtkSpinButton: setting an adjustment with non-zero page size is deprecated" warnings Shall I have ago at removing these too?
Created attachment 131129 [details] [review] partial fix - part 4. Okay here's the final patch, which bumps required GTK+ version to 2.16 and replaces gtk_widget_get_action and gtk_action_set_proxy with gtk_actionable equivalents.
Ok but, Ive said it before, and now have something to add, at the risk of sounding redundant: - Deprecated symbols included in the plugin are needed to support deprecated widgets (of course, you probably already know that) - Glade 3.6 depends on only GTK+ 2.14 intentionally (to allow 2.14 users to develop for 2.16 without upgrading their desktop, also an important feature). Cheers...
Those are fair points Tristan. Only the last patch requires GTK+ 2.16, are the other 3 still any use? None of them go anywhere near the deprecated widget support.
Sure I would be happy to apply a patch that refreshes symbols up to GTK+ 2.14, as specially in the GTK+ plugin and the Glade core. Note the libgnomeui plugin is just there for old project support and I expect it to die anyway.
So... Tristan, willing to apply the first three patches?
..and could the fourth patch be ifdef'ed?
First patch: diff --git a/gladeui/glade-property-class.c b/gladeui/glade-property-class.c index 72f966d..fbeaaed 100644 --- a/gladeui/glade-property-class.c +++ b/gladeui/glade-property-class.c @@ -611,7 +611,8 @@ glade_property_class_make_object_from_string (GladePropertyClass *property_class icon = gtk_widget_render_icon (widget, GTK_STOCK_MISSING_IMAGE, GTK_ICON_SIZE_MENU, NULL); - gtk_object_sink (GTK_OBJECT (widget)); + if (g_object_is_floating (G_OBJECT (widget))) + g_object_ref_sink (G_OBJECT (widget)); } I dont know why this old code does gtk_object_sink(), but it should probably just do gtk_widget_destroy() (since its used here only to create an icon... its probably leaked in this case). also: diff --git a/plugins/gtk+/glade-gtk.c b/plugins/gtk+/glade-gtk.c index 1c00ec3..733ae11 100644 --- a/plugins/gtk+/glade-gtk.c +++ b/plugins/gtk+/glade-gtk.c @@ -4075,8 +4075,8 @@ glade_gtk_notebook_add_child (GladeWidgetAdaptor *adaptor, /* Just destroy placeholders */ if (GLADE_IS_PLACEHOLDER (child)) { - if (GTK_OBJECT_FLOATING (child)) - gtk_object_sink (GTK_OBJECT (child)); + if (g_object_is_floating (G_OBJECT (child))) + g_object_ref_sink (G_OBJECT (child)); else g_object_unref (G_OBJECT (child)); } Same thing here, the old code is trying to free a floating placeholder that was never parented, this patch adds a ref and leeks the placeholder. The rest of the first patch (all deprecated gtk+ symbols that are replaced in glib path) is fine... The second patch, correct me if I'm wrong but: - gtk_entry_set_editable (GTK_ENTRY (eprop_adj->entry), FALSE); + gtk_editable_set_editable (GTK_ENTRY (eprop_adj->entry), FALSE); Wouldnt this require you to cast the entry as GTK_EDITABLE(entry) ? Also, can you explain this part of the patch ? diff --git a/src/glade-window.c b/src/glade-window.c index b735546..2dbfaed 100644 --- a/src/glade-window.c +++ b/src/glade-window.c @@ -514,7 +514,7 @@ window_screen_changed_cb (GtkWidget *widget, screen = gtk_widget_get_screen (widget); - window->priv->recent_manager = gtk_recent_manager_get_for_screen (screen); + window->priv->recent_manager = gtk_recent_manager_get_default (); The third patch, s/GtkOptionMenu/GtkComboBox looks all around good, maybe its best to branch off before introducing that change in 3.6 ? I wouldnt mind cramming the fix into stable personally, assuming it just works... The forth patch minus configure.ac and plus #if GTK_MINOR_VERSION >= etc etc could be ok, I want to keep a standard of supporting a version of GTK+ in the future than the version required by Glade, so inserting some #ifs for the GTK+ version could be good practice here.
Created attachment 135177 [details] [review] first patch redone Changed the first patch to destroy instead of sink and leak refs. I tried but I didn't manage to trigger the code at either of these points so it's untested. BTW, what's do you prefer me to do with the ChangeLog entries? They make the patches go stale quickly .. would you rather I posted a changelog summary in the comments here? Or left it for you to do? * gladeui/glade-clipboard-view.c, gladeui/glade-command.c, gladeui/glade-property-class.c, gladeui/glade-utils.c, plugins/gtk+/glade-gtk.c: Replace use of deprecated GTK+ function calls with replacements in GLib. Partially fixes bug #572756.
Created attachment 135178 [details] [review] 2nd patch redone Changed all the GTK_ENTRY () casts to GTK_EDITABLE () :) The business with gtk_recent_manager is that there doesn't seem to be a different manager for each screen now - hence the deprecation of get_default_for_screen. With this in mind the window_screen_changed_cb function can actually be removed completely, since its job was to use the GtkRecentManager for the correct screen. * src/glade-window.c, gladeui/glade-editor-property.c, plugins/gtk+/glade-accels.c, plugins/gtk+/glade-cell-renderer-button.c, plugins/gtk+/glade-gtk.c: Replace use of deprecated GTK+ functions. Partially fixes bug #572756.
Created attachment 135179 [details] [review] replacement for 3rd patch there was a nasty bug in this one :( calling gtk_tree_model_get without a terminating -1.
Created attachment 135180 [details] [review] and finally .. 4th patch redone as suggested
Commit first patch...
Commit second patch...
Commit third patch...
Ok, Im getting lazy but I'll commit it as such: #if (GTK_MAJOR_VERSION == 2) && (GTK_MINOR_VERSION < 16) deprecated code; #else new code; #endif Since the submitted version ensures that Glade wont link against GTK+ 3.0 with deprecated symbols removed. And with that Im closing this sucker.
W00t! Thanks a lot everybody for fixing this! Whitelisted the ifdefs so http://www.gnome.org/~fpeters/299.html should be correct in a few hours.
(In reply to comment #19) > Ok, Im getting lazy but I'll commit it as such: > > #if (GTK_MAJOR_VERSION == 2) && (GTK_MINOR_VERSION < 16) > deprecated code; > #else > new code; > #endif This is handled nicely by the GTK_CHECK_VERSION macro (note that deprecated and new code are then inverted): #if GTK_CHECK_VERSION (2, 16, 0) <new code>; #else <deprecated code>; #endif