After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 572756 - Remove deprecated GTK+ symbols
Remove deprecated GTK+ symbols
Status: RESOLVED FIXED
Product: glade
Classification: Applications
Component: general
git master
Other Linux
: High critical
: ---
Assigned To: Glade 3 Maintainers
Glade 3 Maintainers
Depends on:
Blocks: 585692
 
 
Reported: 2009-02-22 17:04 UTC by André Klapper
Modified: 2009-06-24 00:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
partial fix - all deprecated gtk+ symbols that are replaced in glib (3.14 KB, patch)
2009-03-20 23:46 UTC, Sam Thursfield
none Details | Review
partial fix - all deprecated gtk+ symbols that are replaced in glib (all of them this time) (3.52 KB, patch)
2009-03-20 23:53 UTC, Sam Thursfield
needs-work Details | Review
partial fix - all gtk+ symbols apart from a couple of awkward ones. (7.67 KB, patch)
2009-03-21 00:19 UTC, Sam Thursfield
needs-work Details | Review
partial fix part 3 - replace GtkOptionMenu with GtkComboBox (7.59 KB, patch)
2009-03-21 01:29 UTC, Sam Thursfield
none Details | Review
partial fix - part 4. (2.05 KB, patch)
2009-03-22 17:00 UTC, Sam Thursfield
reviewed Details | Review
first patch redone (2.97 KB, patch)
2009-05-22 14:50 UTC, Sam Thursfield
committed Details | Review
2nd patch redone (8.28 KB, patch)
2009-05-22 15:18 UTC, Sam Thursfield
committed Details | Review
replacement for 3rd patch (4.67 KB, patch)
2009-05-22 15:32 UTC, Sam Thursfield
committed Details | Review
and finally .. 4th patch redone as suggested (1.86 KB, patch)
2009-05-22 15:57 UTC, Sam Thursfield
rejected Details | Review

Description André Klapper 2009-02-22 17:04:19 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);
Comment 1 Sam Thursfield 2009-03-20 23:46:53 UTC
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.
Comment 2 Sam Thursfield 2009-03-20 23:53:13 UTC
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.
Comment 3 Sam Thursfield 2009-03-21 00:19:49 UTC
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)
Comment 4 Sam Thursfield 2009-03-21 01:29:03 UTC
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?
Comment 5 Sam Thursfield 2009-03-22 17:00:03 UTC
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.
Comment 6 Tristan Van Berkom 2009-03-22 21:31:37 UTC
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...
Comment 7 Sam Thursfield 2009-03-23 22:47:46 UTC
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.
Comment 8 Tristan Van Berkom 2009-03-23 22:56:44 UTC
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.
Comment 9 André Klapper 2009-04-26 16:42:47 UTC
So... Tristan, willing to apply the first three patches?
Comment 10 André Klapper 2009-04-28 08:54:33 UTC
..and could the fourth patch be ifdef'ed?
Comment 11 Tristan Van Berkom 2009-04-28 16:14:47 UTC
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.

Comment 12 Sam Thursfield 2009-05-22 14:50:31 UTC
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.
Comment 13 Sam Thursfield 2009-05-22 15:18:17 UTC
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.
Comment 14 Sam Thursfield 2009-05-22 15:32:53 UTC
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.
Comment 15 Sam Thursfield 2009-05-22 15:57:39 UTC
Created attachment 135180 [details] [review]
and finally .. 4th patch redone as suggested
Comment 16 Tristan Van Berkom 2009-06-20 16:52:40 UTC
Commit first patch...
Comment 17 Tristan Van Berkom 2009-06-20 17:01:42 UTC
Commit second patch...
Comment 18 Tristan Van Berkom 2009-06-20 17:07:21 UTC
Commit third patch...
Comment 19 Tristan Van Berkom 2009-06-20 17:23:05 UTC
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.
Comment 20 André Klapper 2009-06-20 19:57:32 UTC
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.
Comment 21 Luis Menina 2009-06-24 00:03:18 UTC
(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