After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 685362 - EvView inherits from GtkContainer, but the EvView struct from GtkLayout
EvView inherits from GtkContainer, but the EvView struct from GtkLayout
Status: RESOLVED FIXED
Product: evince
Classification: Core
Component: general
3.6.x
Other Linux
: Normal normal
: ---
Assigned To: Evince Maintainers
Evince Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-10-03 08:33 UTC by richard
Modified: 2013-01-05 12:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix the bug (806 bytes, patch)
2012-10-04 12:46 UTC, José Aliste
reviewed Details | Review

Description richard 2012-10-03 08:33:21 UTC
The recent documentation shows EvView is no longer a GtkLayout which means the GdkWindow for drawing is inaccessible.
(However http://git.gnome.org/browse/evince/tree/libview/ev-view-private.h?h=gnome-3-6 indicates that GtkLayout *is* the first component of EvView, so why gtk_layout_get_bin_window() fails is a mystery. The warning seen is 
(denemo:21030): Gtk-CRITICAL **: gtk_layout_get_bin_window: assertion
`GTK_IS_LAYOUT (layout)' failed
which agrees with the documentation).
Can you give an accessor for the GdkWindow for over-drawing the document please? And if possible a workaround/hack for patching the upcoming GNU/Denemo release.
Comment 1 José Aliste 2012-10-03 12:06:31 UTC
We have been deriving from GtkFixed since 2010... @Carlos, is that a bug that the EvView struct still has a GtkLayout  as first element?  At first, I thought you keep it there for ABI compability reasons, but I am nos sure. 


@Richard, gtk_layout_get_bin_window fails because typically gtk_ methods will check the GType of the object you are trying to apply the method to. In this case, you can no longer use the gtk_layout_get_bin_window. You should just use
gtk_widget_get_window (GTK_WIDGET (view)) to get a GdkWindow. 

Greetings

PS: Leaving this open until Carlos confirms whether the GtkLayout line in the ev-view-private.h is intentional ( which I presume so)
Comment 2 Carlos Garcia Campos 2012-10-03 12:19:56 UTC
No, we are not a GtkLayout nor a GtkFixed anymore but a GtkContainer, ev-view-private.h is wrong it should have GtkContainer and GtkContainerClass
Comment 3 richard 2012-10-03 12:42:38 UTC
I guess I can test GTK_IS_LAYOUT(eview) and if succeeds use gtk_layout_get_bin_window() else use gtk_widget_get_window()?
I presume this is needed, ie that gtk_widget_get_window() will not work for the earlier EvView?
Comment 4 José Aliste 2012-10-03 13:06:44 UTC
Looking at the commit log, it seems that the change from GtkLayout to GtkContainer is just when releasing Evince 3.0. (the GtkFixed thing was in between unstable versions). So you really can't use a runtime check, as EvView 3 works against gtk+3 and EvView 2.x against gtk-2.
Comment 5 richard 2012-10-03 14:04:19 UTC
So I could just test GTK_MAJOR_VERSION, but a run time check is surely not only possible, but desirable?
That is GTK_IS_LAYOUT(eview) is zero if eview is not a GtkLayout at runtime?
Comment 6 José Aliste 2012-10-03 14:21:43 UTC
Gtk2 and gtk3 are not compatible. I don't think it is possible to link an app against gtk3 and then use gtk2 library, or the contrary.
Comment 7 richard 2012-10-03 14:57:23 UTC
That's right, it will not dynamically load the wrong library, but the test GTK_IS_LAYOUT(eview) will work whichever library the app is linked to. Perhpas you mean it just feels wrong to determine something at runtime which you already know at compile time? It will waste a few cpu cycles I guess.
Comment 8 José Aliste 2012-10-03 15:01:44 UTC
Exactly...I think it would save you headaches just to port denemo to Gtk3 and then you don't need the GTK_IS_LAYOUT check as you also would be assuming you got evview3.  Even if GNOME 3 is not that popular, every linux distro carries gtk3 and evview3 nowadays. 

However, if you choose to keep compability for both Gtk3 and Gtk2, then it would be better to put the conditional code under #ifdefs.
Comment 9 richard 2012-10-03 16:17:05 UTC
Thank you for the clarification. Actually I use Debian stable, and it does not yet have gtk3, I have to ask others to test the GTK3 version of Denemo, which I am happy to report they have already done, and your suggestion of using gtk_widget_get_window() for the new EvView widget is working fine.
Comment 10 José Aliste 2012-10-04 12:46:55 UTC
Created attachment 225803 [details] [review]
Fix the bug
Comment 11 Carlos Garcia Campos 2012-10-06 08:40:38 UTC
Review of attachment 225803 [details] [review]:

::: libview/ev-view-private.h
@@ +111,3 @@
 struct _EvView {
+	GtkContainer parent;
+	gpointer  unused;    /* padding to keep ABI compatibility */

We should change the class struct too. I still wonder if we really need the padding, since this is a private header and it's not installed.
Comment 12 José Aliste 2012-10-06 13:47:47 UTC
Probably is not needed at all... I know nothing about ABI stability. And according to Christian, in the other bug where I attached the patch making the evView struct public, it shouldn't be needed.
Comment 13 Carlos Garcia Campos 2013-01-05 12:24:38 UTC
Fixed in git master