GNOME Bugzilla – Bug 572325
Remove deprecated GTK+ symbols
Last modified: 2010-09-29 14:59:28 UTC
http://live.gnome.org/GnomeGoals/RemoveDeprecatedSymbols/GTK%2B GTK_CHECK_CAST, GTK_CHECK_CLASS_CAST, GTK_CHECK_CLASS_TYPE, GTK_CHECK_GET_CLASS, GTK_CHECK_TYPE, GtkItemFactoryCallback, GtkItemFactoryEntry, GtkType, gdk_pixbuf_render_to_drawable, gdk_pixbuf_render_to_drawable_alpha, gtk_box_pack_start_defaults, gtk_file_chooser_dialog_new_with_backend, gtk_item_factory_create_items, gtk_item_factory_get_widget, gtk_item_factory_get_widget_by_action, gtk_item_factory_new, gtk_item_factory_set_translate_func, gtk_status_icon_set_tooltip, gtk_timeout_add, gtk_timeout_remove, gtk_tooltips_force_window, gtk_tooltips_new, gtk_widget_draw, gtk_widget_set_usize (some might be in libslab as this was a grep in jhbuild) It might make sense to break this into several smaller patches. :-P Some stuff is really only about replacing the calls, some is more complex. Also make sure that the gtk and glib versions in configure.in are high enough and take a look since which version the new non-deprecated functions are available. library.gnome.org is your friend. :-)
Created attachment 129061 [details] [review] Fixes gdk_* stuff This patch replaces the deprecated GDK symbols. This only fixes a part of this bug.
Created attachment 129090 [details] [review] Fixes gtk_timeout_* additional patch that fixes some gtk_timeout_* occurrences. Still more work to be done
Don't forget to check whether the gtk/glib dependencies need to be bumped.
Created attachment 129098 [details] [review] Replaces more deprecated gtk stuff Replaces: - gtk_box_pack_start_defaults - gtk_widget_set_usize - GTK_CHECK_* - GtkType Sorry about the missing dependencies bumps. I'll take a look at what is needed soon.
It looks to me like it should be very easy to get rid of eel-gtk-macros.h completely, thereby dropping lots of boilerplate nobody uses anyway.
Hmm, maybe I am missing something but I don't see anything that requires bumping the glib/gtk dependencies. Current dependencies: gtk+-2.0 >= 2.13.1 glib-2.0 >= 2.17.4 The remaining deprecated gtk symbols for gtk 2.14 are: capplets/appearance/gedit-message-area.c:167: GtkTooltips *tooltips; capplets/appearance/gedit-message-area.c:175: tooltips = gtk_tooltips_new (); capplets/appearance/gedit-message-area.c:178: gtk_tooltips_force_window (tooltips); capplets/appearance/appearance-desktop.c:628: gtk_file_chooser_dialog_new_with_backend (_("Add Wallpaper"), capplets/keyboard/gnome-keyboard-properties-xkblt.c:249: gtk_widget_draw (tree_view, NULL); typing-break/drwright.c:64: GtkItemFactory *popup_factory; typing-break/drwright.c:124:#define GIF_CB(x) ((GtkItemFactoryCallback)(x)) typing-break/drwright.c:126:static GtkItemFactoryEntry popup_items[] = { typing-break/drwright.c:539: item = gtk_item_factory_get_widget_by_action (dr->popup_factory, typing-break/drwright.c:623: menu = gtk_item_factory_get_widget (dr->popup_factory, ""); typing-break/drwright.c:789: dr->popup_factory = gtk_item_factory_new (GTK_TYPE_MENU, typing-break/drwright.c:792: gtk_item_factory_set_translate_func (dr->popup_factory, typing-break/drwright.c:797: gtk_item_factory_create_items (dr->popup_factory, typing-break/drwright.c:802: /*item = gtk_item_factory_get_widget_by_action (dr->popup_factory, POPUP_ITEM_ENABLED); typing-break/drwright.c:805: item = gtk_item_factory_get_widget_by_action (dr->popup_factory, POPUP_ITEM_BREAK);
I committed the first two patches: http://svn.gnome.org/viewvc/gnome-control-center?view=revision&revision=9283 Is it okay to commit the non-eel-gtk-macros.h parts of the last patch and reattach the eel-gtk-macros.h-stuff in a separate patch? I am not really sure what to do about gedit_message_area.c: style_set(). The function looks suspicious wrt indentation. I am also not really sure how I should test that code. Perhaps it is better that you fix that part of the code? Wrt GtkItemFactory use in typing-break the documentation suggests using GtkUIManager instead. Do you want me to do that?
(In reply to comment #7) > I committed the first two patches: Thanks. > Is it okay to commit the non-eel-gtk-macros.h parts of the last patch and > reattach the eel-gtk-macros.h-stuff in a separate patch? Sure. > I am not really sure what to do about gedit_message_area.c: style_set(). The > function looks suspicious wrt indentation. I am also not really sure how I > should test that code. Perhaps it is better that you fix that part of the code? Ideally, it would be fixed in gedit first, and we'd just steal their fixes. ;-) > Wrt GtkItemFactory use in typing-break the documentation suggests using > GtkUIManager instead. Do you want me to do that? TBH, I haven't looked at it myself, yet, but if that's what the documentation says, knock yourself out.
Created attachment 129356 [details] [review] Replace GtkType GType Reattaching parts of the last patch that had comments. Marking reviewed as before. The other parts of the patch are committed as: http://svn.gnome.org/viewvc/gnome-control-center?view=revision&revision=9284
Feel free to commit this as well for now. We can always clean this up later.
Thanks. Commited as: http://svn.gnome.org/viewvc/gnome-control-center?view=revision&revision=9287
Created attachment 129384 [details] [review] Use GtkUIManager to replace GtkItemFactory Patch is tested and works. You will probably want to review this one a bit. I am unsure about whether I did it correctly wrt translations. Also unsure about accelerators this popup menu. Are they needed or even possible?
Gedit already fixed/changed their code back in September 2007: http://svn.gnome.org/viewvc/gedit?view=revision&revision=5868 Perhaps it is would be good to resync with them?
+static const GtkActionEntry actions[] = { + {"Preferences", GTK_STOCK_PREFERENCES, N_("_Preferences"), NULL, NULL, G_CALLBACK (popup_preferences_cb)}, + {"About", GTK_STOCK_ABOUT, N_("_About"), NULL, NULL, G_CALLBACK (popup_about_cb)}, You can save the translators a bit of work and leave out the label where you use stock items. +const char ui_description[] = + "<ui>" + " <popup name='Pop'>" + " <menuitem action='Preferences'/>" + " <menuitem action='About'/>" + " <separator/>" + " <menuitem action='TakeABreak'/>" + " </popup>" + "</ui>"; Since this is only used once, I'd turn it into a local variable. The rest looks pretty solid to me. Any specific issues with the accelerators? (In reply to comment #13) > Gedit already fixed/changed their code back in September 2007: > http://svn.gnome.org/viewvc/gedit?view=revision&revision=5868 > > Perhaps it is would be good to resync with them? Yeah. Should be as simple as a cp.
Created attachment 129449 [details] [review] Updated patch Updated patch as per comments. There is no real problem with accelerators except for the fact that they are not there :) If this is intended then all is good I think.
If by "aren't there" you mean they aren't used in the popup menu, then yes, I guess that's intentional.
Commited as: http://svn.gnome.org/viewvc/gnome-control-center?view=revision&revision=9289 Remaining: capplets/appearance/gedit-message-area.c:167: GtkTooltips *tooltips; capplets/appearance/gedit-message-area.c:175: tooltips = gtk_tooltips_new (); capplets/appearance/gedit-message-area.c:178: gtk_tooltips_force_window (tooltips); capplets/appearance/appearance-desktop.c:628: gtk_file_chooser_dialog_new_with_backend (_("Add Wallpaper"), capplets/keyboard/gnome-keyboard-properties-xkblt.c:249: gtk_widget_draw (tree_view, NULL);
Cool. I love you guys. :-)
Reverted the patch since it broke string freeze and I did not get the needed approval.
Resetting patch status (None->AcceptedNow->Committed->Reverted->None->CommitAfterFreeze) though "Commit after branching" would make more sense :-P
let me know when gnome-control-center has branched and I will commit again. Thanks.
+ const char ui_description[] = This should be "static const char ui_description[]" .
Created attachment 133317 [details] [review] Replace GtkItemFactory with GtkUIManager Thanks Christian. Patch is updated
Could this get in before 2.27.1?
ping.
Patch committed to master
I reopen the bug because still there are deprecated symbols in the code. See http://www.gnome.org/~fpeters/299.html for a complete list
Remaining: capplets/appearance/gedit-message-area.c:167: GtkTooltips *tooltips; capplets/appearance/gedit-message-area.c:175: tooltips = gtk_tooltips_new (); capplets/appearance/gedit-message-area.c:178: gtk_tooltips_force_window (tooltips); This is stuff copied from gedit. Gedit already fixed/changed their code back in September 2007: http://svn.gnome.org/viewvc/gedit?view=revision&revision=5868 capplets/appearance/appearance-desktop.c:628: gtk_file_chooser_dialog_new_with_backend (_("Add Wallpaper"), No real idea about this one. capplets/keyboard/gnome-keyboard-properties-xkblt.c:249: gtk_widget_draw (tree_view, NULL); Patch coming up
Created attachment 138134 [details] [review] Replace deprecated gtk_widget_draw Patch is completely untested! See http://git.gnome.org/cgit/gtk+/tree/gtk/gtkwidget.c#n3637 for the contents of gtk_widget_draw I basically just replaced it with the code path for passing a NULL area.
Created attachment 138210 [details] [review] Replace deprecated gtk_file_chooser_dialog_new_with_backend Patch is completely untested! I googled around a bit for the gtk_file_chooser_dialog_new_with_backend issue. Since gtk+ now uses gio directly it should work to just use gtk_file_chooser_dialog_new instead. Bug about the deprecation: http://bugzilla.gnome.org/show_bug.cgi?id=545976 With the last two patches all that is left is the gedit-message-area stuff. I don't plan to look at that as I am not really sure what the code is about. See comment #14 for details about it.
Created attachment 138329 [details] [review] Resync gedit-message-area.[ch] with upstream Oh well. I did it anyway. It is a simple resync as there was no g-c-c specific patches applied to the file. Again untested. I can't seem to get g-c-c to build currently. With these patches the bug is fixed.
Created attachment 138542 [details] [review] Replace deprecated gtk_widget_draw with gtk_widget_queue_draw New patch. Tested this time.
Created attachment 138545 [details] [review] Updated: Replace deprecated gtk_file_chooser_dialog_new_with_backend Updated patch. Tested. All patches should be ready for review/commit now.
Committed to master: http://git.gnome.org/cgit/gnome-control-center/commit/?id=8ffc430cb8d033d74c1e904d78d59f8af902ccf0 http://git.gnome.org/cgit/gnome-control-center/commit/?id=f315f3451acb0f7a55ddb9b97ca22b003c4710dd http://git.gnome.org/cgit/gnome-control-center/commit/?id=e01c1e2a64b836892bfc7a9d6852fc9b76b7ee8e
Great. Only thing left currently are these two functions: ./typing-break/drwright.c: gtk_status_icon_set_tooltip (dr->icon, ./typing-break/drwright.c: gtk_status_icon_set_tooltip (dr->icon,
Created attachment 138743 [details] [review] Replace gtk_status_icon_set_tooltip Missed those. The patch replaces them with gtk_status_icon_set_text. This requires gtk 2.16.0 so the patch also bumps the requirement in configure.in
My copy of the GTK+ docs says "Since: 2.14"...?
Correction. Seems it got in a little later. Please change to check for 2.15.0 instead and commit.
Online docs says "2.16" http://library.gnome.org/devel/gtk/unstable/GtkStatusIcon.html#gtk-status-icon-set-tooltip-text I double checked and it was indeed included in 2.15.0. Patch committed with that change: http://git.gnome.org/cgit/gnome-control-center/commit/?id=3d3638cdd5441d9721713b6fb16c1c337ef43716
Reopening: there are new deprecated symbols See http://people.gnome.org/~fpeters/299.html
Created attachment 158301 [details] [review] Remove deprecated GTK+ symbols This patch removed all the deprecated GTK+ symbols. I'm not very sure about the gtk_button_released (widget); > g_signal_emit_by_name (widget, "button-release-event", 0); substitution, check that you are not affected by bug #610515
Review of attachment 158301 [details] [review]: The libslab part needs to go into gnome-main-menu/libslab instead, anyway. We just sync from there. Rest looks fine.
Comment on attachment 158301 [details] [review] Remove deprecated GTK+ symbols commit 825434bbcd53285cc2f6101309fad3869b041b12 For the liblab part, I've filled bug #615671
Also, seems that the proper solution here is depend on libslab as an standalone library. Take a look here: http://git.gnome.org/browse/libslab/tree/TODO-standalone-library.txt
(In reply to comment #45) > seems the proper solution here is depend on libslab as an standalone library. That has been discussed before and is planned IIRC.
Created attachment 162116 [details] [review] Trivial patch ./capplets/common/theme-thumbnail.c: gdk_pixmap_unref (pixmap); ./capplets/common/theme-thumbnail.c: gdk_pixmap_unref (pixmap); ./capplets/common/theme-thumbnail.c: gdk_pixmap_unref (pixmap); were introduced when fixing bug 597888.
Comment on attachment 162116 [details] [review] Trivial patch http://git.gnome.org/browse/gnome-control-center/commit/?id=186a999d6e82eea5b908d3a040871e02ff470c36
Capplets aren't built. The code is still there whilst panels get ported, so closing.