GNOME Bugzilla – Bug 584847
Get rid of libsexy, use new GtkLabel link support
Last modified: 2009-06-22 16:37:33 UTC
Now that GtkLabel link support has been added to GTK+ (9dbb304), it would be nice for policykit-gnome to switch to it. Trivial patch attached, but untested as I don't have gtk 2.17.1 installed here
Created attachment 135961 [details] [review] Get rid of libsexy, use new GtkLabel link support
<snip> -static void -vendor_url_activated (SexyUrlLabel *url_label, char *url, gpointer user_data) +static gboolean +vendor_url_activated (GtkLabel *url_label, gpointer user_data) { + gchar *url; + + gtk_label_get_current_uri (GTK_LABEL (url_label)); if (url != NULL) - gtk_show_uri (NULL, url, GDK_CURRENT_TIME, NULL); + { + gtk_show_uri (NULL, url, GDK_CURRENT_TIME, NULL); + g_free (url); + return TRUE; + } + return FALSE; } </snip> There's a bug : + gtk_label_get_current_uri (GTK_LABEL (url_label)); should be + url = gtk_label_get_current_uri (GTK_LABEL (url_label)); Moreover the url should not be freed, as specified in http://library.gnome.org/devel/gtk/unstable/GtkLabel.html#gtk-label-get-current-uri The whole thing would be nicer this way: static gboolean vendor_url_activated (GtkLabel *url_label, gpointer user_data) { gchar *url; url = gtk_label_get_current_uri (GTK_LABEL (url_label)); if (url == NULL) return FALSE; return gtk_show_uri (NULL, url, GDK_CURRENT_TIME, NULL); } In another part of the patch, we see : - } + l which is certainly an error. Then, the signal connection part. http://library.gnome.org/devel/gtk/unstable/GtkLabel.html#GtkLabel-activate-link specifies that "Applications may connect to [the activate-link signal] to override the default behaviour, which is to call gtk_show_uri()". But what we want *is* the default behavior, so it seems neither the callback not the signal connection are useful. I haven't checked the rest of the patch, but it already needs some work. Feel free to submit an improved version.
Created attachment 136471 [details] [review] Get rid of libsexy, use new GtkLabel link support v2 New patch. Thank you Luis for your review!
Created attachment 136472 [details] [review] Get rid of libsexy, use new GtkLabel link support v3 I've forgotten the part about the signals in the last patch Thank you again Luis.
Looks good. The only concern I have is that your patch touches a #if 0'ed area. So with the blessing of the maintainer, it would be nicer to remove the dead code (the callback and the signal connection). Could the maintainer please give an advice here ?
Committed something based on the patch in comment 5. Thanks!
Hello David, I think that you mean comment 4 ? ;)