Created attachment 143319 [details] [review]
Use accessor functions instead direct access

GTK+ 2.17.10 is now the required version

I've used all the GTK+ 2.17.11 api available, still missing:

GTK_WIDGET_SET_FLAGS (widget, GTK_REALIZED);
GTK_WIDGET_REALIZED ()

Created attachment 143368 [details] [review]
Use accessor functions instead direct access.

Sorry, here the correct patch

Thanks for working on this.  I haven't had a close look at the patch yet.  My main concern is that I don't want to increase the gtk+ version requirement to 2.17.x.  It's important that it remains possible to build rhythmbox on recent desktop distribution releases.

Looks like we can commit a large portion of this if we increase the gtk+ version requirement to 2.14, which isn't a problem at all.

For much of the rest, it looks like we could provide a set of backwards compatibility #defines, like "#define gtk_widget_is_drawable(x) GTK_WIDGET_DRAWABLE(x)", in a header with appropriate version checks.

Indeed, we should coordinate us to have a common file.

gnome-games already has one: http://git.gnome.org/cgit/gnome-games/tree/libgames-support/games-gtk-compat.h.

After talking with a gnome-games devel I think we should use that file as a "main repo" to all the funcions we need, what do you think?

I've just increased the gtk+ version requirement to 2.14, and I'll look at committing parts of this soon.  As far as I can tell, the GtkAdjustment, GtkSelectionData, and gtk_dialog_get_content_area changes can go in as-is.

Created attachment 150566 [details] [review]
Use accessor functions instead direct accessv2

Ok, here a new patch;

I've created a gseal-gtk-compat.h file to functions that need a GTK+ version >= 2.14.0

Still missing (no GTK+ api):
GTK_WIDGET_SET_FLAGS (widget, GTK_REALIZED);
GTK_WIDGET_REALIZED ()

Review of attachment 150566 [details] [review]:

Aside from a couple of things, this looks good.  Thanks for working on it.

::: plugins/visualizer/rb-vis-widget.c
@@ +58,3 @@
 {
+	gtk_widget_set_can_focus (GTK_WIDGET (rbvw), TRUE);
+	gtk_widget_set_double_buffered (GTK_WIDGET (rbvw), TRUE);

shouldn't this be gtk_widget_set_double_buffered (GTK_WIDGET (rbvw), FALSE); ?

::: shell/rb-shell-preferences.c
@@ +202,3 @@
 			   shell_preferences->priv->notebook);
 
+	gtk_container_set_border_width (GTK_CONTAINER (content_area), 5);

I don't think this one should have been changed - the original line was setting the border with on shell_preferences, not the content area.

hrm, splinter didn't format that the way I'd hoped it would..

the original line for this:
+    gtk_widget_set_double_buffered (GTK_WIDGET (rbvw), TRUE);
was:
	GTK_WIDGET_UNSET_FLAGS (GTK_WIDGET (rbvw), GTK_DOUBLE_BUFFERED);

Review of attachment 150566 [details] [review]:

I haven't actually tried building this with gtk+ <2.18 yet, but I'm about to.

::: lib/gseal-gtk-compat.h
@@ +36,3 @@
+#define gtk_widget_set_can_default(widget, TRUE)                (GTK_WIDGET_SET_FLAGS (widget, GTK_CAN_DEFAULT))
+#define gtk_widget_set_can_focus(widget, TRUE)                  (GTK_WIDGET_SET_FLAGS (widget, GTK_CAN_FOCUS))
+#define gtk_widget_set_double_buffered (widget, FALSE)          (GTK_WIDGET_UNSET_FLAGS (widget, GTK_DOUBLE_BUFFERED))

These cause compiler errors - (do { ... } while (...)) results in "expected expression before 'do'".

The c preprocessor doesn't check that the arguments match here, it'll use these expansions for anything that looks like gtk_widget_set_can_focus(x, y), whether y is TRUE or FALSE or ICECREAM.

@@ +37,3 @@
+#define gtk_widget_set_can_focus(widget, TRUE)                  (GTK_WIDGET_SET_FLAGS (widget, GTK_CAN_FOCUS))
+#define gtk_widget_set_double_buffered (widget, FALSE)          (GTK_WIDGET_UNSET_FLAGS (widget, GTK_DOUBLE_BUFFERED))
+#define gtk_widget_set_window (widget, window)                  ((widget)->window=window)

It'll also replace any instance of 'window' in the macro expansion with the macro argument.  The argument needs to be renamed to _window for this to work.

A few things were missing from gseal-gtk-compat.h:

gtk_cell_renderer_get_padding
gtk_cell_renderer_set_padding
gtk_cell_renderer_get_alignment

and a few more files needed to include it, but with those changes, it builds with gtk 2.16.  I'm pretty sure 2.14 will be OK too.

I've committed my updated version.  Since there's still more to do (GTK_WIDGET_REALIZED etc.) I'm leaving the bug open.

Hello Jonathan, thank you for fix my patch and commit it, I was a bit busy these days ;)

As soon as exist the GTK+ api to complete fix this bug I'll attach a new patch

Happy year!

Created attachment 156761 [details] [review]
Use accessor functions instead direct access. Final patch

Here the final patch to fix this bug.

I'd recommend you to add 

CFLAGS+="-DGSEAL_ENABLE"

to you configure.ac, so this bug won't be reopended

Committed, thanks.

Created attachment 157719 [details] [review]
compilation with GSEAL is broken for me without this patch

Please review it carefully, I wrote all this patch by guessing, I might have used wrong accessors...

Comment on attachment 157719 [details] [review]
compilation with GSEAL is broken for me without this patch

Looks correct to me

Comment on attachment 157719 [details] [review]
compilation with GSEAL is broken for me without this patch

Pushed as 55777fcb6ec68fd6e257425dfe66023225e9d440