GNOME Bugzilla – Bug 339745
Return value of gtk_link_button_set_uri_hook not usable
Last modified: 2011-02-04 16:12:22 UTC
We don't know what to do with the function pointer gtk_link_button_set_uri_hook returns. It's a plain C function pointer. We don't get the user data. And the hook's destroy function has already been called. For all we know, the pointer is not valid anymore. So, should language bindings just ignore this? Should the API be extended to allow proper restoration of the previously installed hook?
These return values are definitely nonsensical and broken. And also the similar return values from gtk_about_dialog_set_url_hook() gtk_about_dialog_set_email_hook(). This is a particular problem for automatic binding with gobject-introspection, since there is no opportunity for human intervention to just ignore them. Annotating as: /* Return value: (type void): might work - or at least could be made to work in gobject-introspection. There is some question whether treating a function-pointer returning function as a void-returning function is OK in practice - it probably is, though it's certainly not C standard compliant. This might be an OK API break to make for GTK+ 3.0 - just remove these return values - since I see no way that an application could make use of them in a non-buggy fashion.
I would be open to the idea of just deprecating all these uri hook mechanisms and removing them in 3.0, in favour of gtk_show_uri()
For GtkLinkButton I would suggest to retain the ability to decide where links go. For example, a web browser wants to make sure that hyperlinks do not accidentally open in something else. And a documentation browser which uses link buttons wants to open documents in itself. As for GtkAboutDialog, it *would* likely be good enough, if somebody makes opening of URLs and email clients reliable outside of GNOME. At the moment, overriding the URI is a common trick to work around that. One idea would be to fallback to 'xdg-open' instead of 'evolution'.
Created attachment 163754 [details] [review] Deprecate uri hook mechanisms in gtkaboutdialog
Created attachment 170578 [details] [review] patch Here is a patch that changes gtk_link_button_set_uri_hook to return void, and removes the gtk_about_dialog_set_url/email_hook functions in favor of a GtkAboutDialog::activate-link signal.
an activate-link signal doesn't seem like a substitute for the global hooks... the point of the global hooks was that an app could set this up in one place, or in fact historically I guess the idea was that libgnome could set them up. Though I suppose a nicer solution is to allow some kind of module or other desktop-wide setup that doesn't rely on the app or another lib to do anything. But then you don't really need a signal either right?
(In reply to comment #6) > an activate-link signal doesn't seem like a substitute for the global hooks... > the point of the global hooks was that an app could set this up in one place, > or in fact historically I guess the idea was that libgnome could set them up. > > Though I suppose a nicer solution is to allow some kind of module or other > desktop-wide setup that doesn't rely on the app or another lib to do anything. > But then you don't really need a signal either right? The global configuration has moved below gtk_show_uri, into GIO/gvfs and its gconf/gsettings/shared-mime configuration. The signal is not really very needed. There's some fringe use cases, like a browser that wants to always show links 'in itself' instead of in the configured browser.
commit 701f48c8134d68e908020970bdd9fc28a93b06e5 Author: Matthias Clasen <mclasen@redhat.com> Date: Tue Sep 21 21:55:05 2010 -0400 Remove url hooks from GtkAboutDialog and GtkLinkButton With gtk_show_uri, global configurability of link activation has moved to GIO/gvfs. For local overrides, GtkLinkButton has the ::clicked signal, and GtkAboutDialog gets an ::activate-link signal. Bug 339745
Reopening because this isn't quite finished yet - the GtkAboutDialog documentation still mentions these functions.
True. Also, as discussed in the IRC, we could add the new signal in 2.23