GNOME Bugzilla – Bug 644736
gdk_window_set_user_data is unwrappable by GI and is a poor name for what it actually does
Last modified: 2011-08-14 12:08:18 UTC
As a note, I will be following this up with a patch later today: gdk_window_set_user_data is a holdover from some of the first GTK releases. It has been deprecated for all uses except for setting the widget which owns the window for forwarding events. There are a number of reasons to deprecate this in favour of a better API. First set_user_data takes a gpointer which is not wrappable via GI since we have no type data to work off of. With the advent of offscreen windows it would be nice to be able to prototype offscreen effects using a scripting language. The type issue could be fixed via an annotation but that still leaves us with an obscure method that is hard to understand even after reading the documentation. I feel if we are going to alter the source, might as well put it a little extra work and fix the API. The proposal in bug #644729 is to create a gdk_window_set_owner_widget_libgtk_only method which contains the internals of gdk_window_set_user_data. So we don't break ABI we would keep gdk_window_set_user_data as a deprecated method which just calls the new method. For wrapping purposes we would then create a gtk_widget_own_window (suggestions on a better name are welcome) which can take a GtkWidget as a parameter and would simply cast to GObject and call gdk_window_set_owner_widget_libgtk_only internally.
Created attachment 183360 [details] [review] deprecate gdk_window_set_user_data in favor of gtk_widget_own_window
Patch isn't complete because I would like feedback before doing any more work. Right now setting the window owner is implemented but we also need to unset and get the window owner. The unset is a bit murky since creating a gtk_widget_unown_window(GtkWidget, GdkWindow) brings with it some operational concerns like what to do if the owner (user_data) is not the same as the GtkWidget that is unsetting it. The easier option is to add a gdk_window_unset_owner API but it is kind of weird to call gtk_widget_own_window and then subsequently call gdk_window_unset_owner. I think this is the best option though. The other option is to make the new gdk_window_set_widget_owner public so we can unset by sending in a NULL. As for gdk_window_get_user_data, this is a simple fix of deprecating that method and adding a gdk_window_get_widget_owner but since this is used extensively inside of Gtk+ I wanted to get others opinions before I dove in and changed the world.
A couple of notes. First I think this is a good idea from a language binding point of view and I would like it to get in. * gdk_window_get_user_data should be deprecated as well by adding some sort of getter which allows you to get the widget disguised as a GObject. * I can't really think of any ways an application developer would like to call gdk_window_set_widget_owner() instead of relying that it's called in GtkWidget::unrealize. It needs to be documented properly * Migration guide, examples, tests and demos needs to be updated if this is accepted.
The reason for exposing unsetting the owner is that my widget subclass may have a pointer to a GdkWindow that the default unrealize won't unset and that I have to unset myself by overriding unrealize.
+ * Deprecated: 3.2. Use #GtkWidget::own_window instead. That should be gtk_widget_own_window() Mr. Python-User. :) If we did a change like this, it wouldn't need to touch GDK at all as we could just use g_object_set_data (window, "gtk-owner-widget", widget); from GTK (and fall back to gdk_window_get_user_data() in the getter). I'm also not sure about the term "own", though I can't suggest anything better atm.
(In reply to comment #5) > + * Deprecated: 3.2. Use #GtkWidget::own_window instead. > That should be gtk_widget_own_window() Mr. Python-User. :) That is copy and paste from the same source file I think. I thought it weird too since that is C++ syntax but perhaps the doc tools prefer that now? > If we did a change like this, it wouldn't need to touch GDK at all as we could > just use g_object_set_data (window, "gtk-owner-widget", widget); from GTK (and > fall back to gdk_window_get_user_data() in the getter). does 'gtk-owner-widget' map to user_data or is there a conditional that checks both? As far as I could see in the code user_data is still the struct member that hold the owner widget. > I'm also not sure about the term "own", though I can't suggest anything better > at Any suggestions on the getting and unsetting side. Is it ok to have the getter and unsetter be on the GDK window and not add API to the Gtk side (well the getter wouldn't be on the GtkWidget so the question is more for the unsetter). As I said in an earlier comment unsetting from GtkWidget brings with it issues such as what happens if we aren't the current owner of the GdkWindow?
Yeah, the problem in this case is that we need to attach API to GDK from GTK. I don't know if we have a precedent for how to handle such cases, so no idea really. In bindings, you ideally want to provide API such as window.set_widget(), but that function should live in gtk, not gdk. I suspect in C we'd do something like gtk_gdk_window_set_widget(), kinda like we do gdk_cairo_set_source_pixbuf(). Maybe we should have something like gtk_widget_attach_window() and gtk_widget_detach_window() instead to provide a different way of thinking about this? (ie not "set the widget for a window" but "make a widget use a window")
I don't think gdk_window_set_user_data is any worse than g_object_set_data, really.
(In reply to comment #8) > I don't think gdk_window_set_user_data is any worse than g_object_set_data, > really. I think it's not so much the name of the function as the fact that it does something magical - g_object_set_data is literally just setting data. gdk_window_set_user_data changes how events are processed. I don't have any better name suggestions than John's gtk_widget_own_window(); Benjamin's approach of fixing the inversion might be nice too. But it'd at least be worth fixing up the internals to change the variable from "user_data" to "gtkwidget" or something. And I think it does call for a new function too.
I've been going through what we need to support in PyGObject to have full widget subclassing. This is urgent due to the impending API/ABI freeze on Monday. The simple fix is to annotate gdk_window_set_user_data to accept GObjects since we can't correctly marshal untyped gpointers (they would end up being the python object wrapper for the GdkWindow not the GdkWindow itself). We need to know what Gtk will accept as an API fix here.
Decided to go with the simple annotation fix as there is no sense in complicating the code here.