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 605776 - Use accessor functions instead direct access
Use accessor functions instead direct access
Status: RESOLVED FIXED
Product: evince
Classification: Core
Component: general
2.31.x
Other All
: Normal normal
: ---
Assigned To: Evince Maintainers
Evince Maintainers
Depends on: 618271
Blocks: 585391
 
 
Reported: 2009-12-31 01:26 UTC by Juanjo Marín
Modified: 2010-06-01 07:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Do not use GTK_WIDGET_*SET_FLAGS (widget, GTK_HAS_FOCUS) (2.40 KB, patch)
2010-04-29 03:41 UTC, Javier Jardón (IRC: jjardon)
none Details | Review
Do not use GTK_WIDGET_*SET_FLAGS (widget, GTK_HAS_FOCUS).v2 (2.93 KB, patch)
2010-05-10 00:18 UTC, Javier Jardón (IRC: jjardon)
none Details | Review

Description Juanjo Marín 2009-12-31 01:26:14 UTC
GTK+ currently exposes a huge amount of internal implementation details which blocks cleaning up of internal code, something that is much needed. This is not only for code beautification, in fact, the reason is that it’s a necessity in order to maintain the toolkit.

The objective is substitute all direct access to the object fields for accessor functions.

After we have done this we will have an API consisting only of functions which means that any compatibility glue code we need to write can be done without the risk of variables being accessed directly and circumvent the compatibility layer. This is in the fundamentals of OOP practices.

This is an important Goal to GTK+ 3 transition.

AFAIK, evince doesn't compile with the -DGSEAL_ENABLE flag

More info: http://live.gnome.org/GnomeGoals/UseGseal
Comment 1 André Klapper 2010-01-02 19:11:25 UTC
> AFAIK, evince doesn't compile with the -DGSEAL_ENABLE flag

Oh PLEASE actually TEST issues first before "guessing" bugs.
This is a bug tracker about real bugs and not about "might be" bugs...
Comment 2 André Klapper 2010-01-02 19:47:08 UTC
Confirming.
Comment 3 Juanjo Marín 2010-01-03 11:03:22 UTC
André,

Sorry for my poor quality bug report. Of course it was not guessing, but I though of it as a reminding for talking with the maintainer on the IRC. I understand that "AFAIK" is misleading, but it was the first time I use I compiled Evince adding a flag and I wasn't sure if what I did was right.

My Apologise for distrubing your job as bugmaster.
Comment 4 Juanjo Marín 2010-01-03 11:11:03 UTC
Compiled with this line on .jhbuildrc:

os.environ['CFLAGS'] = '-DGSEAL_ENABLE'

The message error:

ev-pixbuf-cache.c: In function ‘get_selection_colors’:
ev-pixbuf-cache.c:482: error: ‘GtkObject’ has no member named ‘flags’
ev-pixbuf-cache.c:483: error: ‘GtkWidget’ has no member named ‘style’
ev-pixbuf-cache.c:484: error: ‘GtkWidget’ has no member named ‘style’
ev-pixbuf-cache.c:486: error: ‘GtkWidget’ has no member named ‘style’
ev-pixbuf-cache.c:487: error: ‘GtkWidget’ has no member named ‘style’
make[3]: *** [libevview_la-ev-pixbuf-cache.lo] Error 1
make[3]: Leaving directory `/home/jjmarin/src/gnome/svn/evince/libview'
make[2]: *** [all] Error 2
make[2]: Leaving directory `/home/jjmarin/src/gnome/svn/evince/libview'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/jjmarin/src/gnome/svn/evince'
make: *** [all] Error 2
*** Error during phase build of evince: ########## Error running make   *** [1/1]
Comment 5 Carlos Garcia Campos 2010-03-31 19:11:18 UTC
libdocument and shell build now with GSEAL, libview uses GTK_WIDGET_SET_FLAGS (widget, GTK_HAS_FOCUS); which odesn't have an accessor yet.
Comment 6 Javier Jardón (IRC: jjardon) 2010-04-11 16:40:22 UTC
Hello Carlos,

Where do you need to use GTK_WIDGET_SET_FLAGS (widget, GTK_HAS_FOCUS);?

Note that you should never need to use this (see https://bugzilla.gnome.org/show_bug.cgi?id=593601#c4)
Comment 7 Carlos Garcia Campos 2010-04-12 12:51:41 UTC
 * For interactive goto page in libview/ev-view-presentation.c
 * For annotation popup windows in libview/ev-annotation-window.c
Comment 8 Tobias Mueller 2010-04-14 01:07:27 UTC
Javier, do you have any idea how to address the use cases Carlos just mentioned?
Comment 9 Javier Jardón (IRC: jjardon) 2010-04-14 01:58:18 UTC
Yeah ;), new GTK+ API will arrive soon to solve this. See bug #593671
Comment 10 Javier Jardón (IRC: jjardon) 2010-04-29 03:41:28 UTC
Created attachment 159847 [details] [review]
Do not use GTK_WIDGET_*SET_FLAGS (widget, GTK_HAS_FOCUS)

Little patch to fix the use of GTK_WIDGET_*SET_FLAGS (widget, GTK_HAS_FOCUS) in libview/ev-view-presentation.c and in libview/ev-annotation-window.c
Comment 11 Carlos Garcia Campos 2010-04-29 07:54:46 UTC
Hey Javier, thanks for the patch, but we'll better wait for a new GTK+ release with the accessor instead of adding more #if
Comment 12 André Klapper 2010-05-08 16:15:35 UTC
Other issues left to fix:
* cut-n-paste/gimpcellrenderertoggle/gimpcellrenderertoggle.c
* cut-n-paste/toolbar-editor/egg-editable-toolbar.c
* cut-n-paste/toolbar-editor/egg-toolbar-editor.c
Comment 13 Carlos Garcia Campos 2010-05-08 17:33:57 UTC
I'm finishing the libview part and I've found some issues :-( I'll take a look at cut-n-paste as soon as libview is working.
Comment 14 Javier Jardón (IRC: jjardon) 2010-05-10 00:18:40 UTC
Created attachment 160676 [details] [review]
Do not use GTK_WIDGET_*SET_FLAGS (widget, GTK_HAS_FOCUS).v2

Updated patch against current master. GTK+ required version bumped to 2.21.0
Comment 15 Carlos Garcia Campos 2010-05-10 13:33:09 UTC
Thanks for the patch Javier, I already had those changes in my local patch indeed. I've just pushed a commit to fix most of the GSEAL issues in libview. Only GtkWindow::group is still used because there ins't an accessor yet, see bug #618271.
Comment 16 Carlos Garcia Campos 2010-05-10 17:47:32 UTC
(In reply to comment #12)
> Other issues left to fix:
> * cut-n-paste/gimpcellrenderertoggle/gimpcellrenderertoggle.c
> * cut-n-paste/toolbar-editor/egg-editable-toolbar.c
> * cut-n-paste/toolbar-editor/egg-toolbar-editor.c

cut-n-paste code is fixed in git master now too.
Comment 17 André Klapper 2010-05-27 15:35:42 UTC
So what is left to do on git master is

make[4]: Entering directory `/evince/cut-n-paste/toolbar-editor'
egg-editable-toolbar.c: In function ‘toolbar_drag_data_received_cb’:
egg-editable-toolbar.c:697: error: ‘GdkDragContext’ has no member named ‘action’
egg-editable-toolbar.c:701: error: ‘GdkDragContext’ has no member named ‘action’
egg-editable-toolbar.c: In function ‘toolbar_drag_motion_cb’:
egg-editable-toolbar.c:760: error: ‘GdkDragContext’ has no member named ‘suggested_action’

make[3]: Entering directory `/evince/libview'
ev-view.c: In function ‘ev_view_drag_motion’:
ev-view.c:3476: error: ‘GdkDragContext’ has no member named ‘suggested_action’
ev-view-presentation.c: In function ‘ev_view_presentation_goto_window_create’:
ev-view-presentation.c:635: error: ‘GtkWindow’ has no member named ‘group’
ev-view-presentation.c:636: error: ‘GtkWindow’ has no member named ‘group’
ev-view-presentation.c:638: error: ‘GtkWindow’ has no member named ‘group’
ev-view-presentation.c:639: error: ‘GtkWindow’ has no member named ‘group’
ev-view-presentation.c:648: error: ‘GtkWindow’ has no member named ‘group’
ev-view-presentation.c:649: error: ‘GtkWindow’ has no member named ‘group’
Comment 18 Carlos Garcia Campos 2010-05-27 15:41:44 UTC
(In reply to comment #17)
> So what is left to do on git master is
> 
> make[4]: Entering directory `/evince/cut-n-paste/toolbar-editor'
> egg-editable-toolbar.c: In function ‘toolbar_drag_data_received_cb’:
> egg-editable-toolbar.c:697: error: ‘GdkDragContext’ has no member named
> ‘action’
> egg-editable-toolbar.c:701: error: ‘GdkDragContext’ has no member named
> ‘action’
> egg-editable-toolbar.c: In function ‘toolbar_drag_motion_cb’:
> egg-editable-toolbar.c:760: error: ‘GdkDragContext’ has no member named
> ‘suggested_action’

hmm, no, I thought I had already fixed this, I'll look at it again. 

> make[3]: Entering directory `/evince/libview'
> ev-view.c: In function ‘ev_view_drag_motion’:
> ev-view.c:3476: error: ‘GdkDragContext’ has no member named ‘suggested_action’
> ev-view-presentation.c: In function ‘ev_view_presentation_goto_window_create’:
> ev-view-presentation.c:635: error: ‘GtkWindow’ has no member named ‘group’
> ev-view-presentation.c:636: error: ‘GtkWindow’ has no member named ‘group’
> ev-view-presentation.c:638: error: ‘GtkWindow’ has no member named ‘group’
> ev-view-presentation.c:639: error: ‘GtkWindow’ has no member named ‘group’
> ev-view-presentation.c:648: error: ‘GtkWindow’ has no member named ‘group’
> ev-view-presentation.c:649: error: ‘GtkWindow’ has no member named ‘group’

Yes, I'm just waiting for a new gtk+ release with gtk_window_has_group()

Thanks!
Comment 19 Carlos Garcia Campos 2010-05-27 16:17:46 UTC
(In reply to comment #18)
> (In reply to comment #17)
> > So what is left to do on git master is
> > 
> > make[4]: Entering directory `/evince/cut-n-paste/toolbar-editor'
> > egg-editable-toolbar.c: In function ‘toolbar_drag_data_received_cb’:
> > egg-editable-toolbar.c:697: error: ‘GdkDragContext’ has no member named
> > ‘action’
> > egg-editable-toolbar.c:701: error: ‘GdkDragContext’ has no member named
> > ‘action’
> > egg-editable-toolbar.c: In function ‘toolbar_drag_motion_cb’:
> > egg-editable-toolbar.c:760: error: ‘GdkDragContext’ has no member named
> > ‘suggested_action’
> 
> hmm, no, I thought I had already fixed this, I'll look at it again. 

aha, this is gdk, not gtk, which has been recently sealed, we need a new gtk+ release for these too.
Comment 20 Carlos Garcia Campos 2010-05-31 17:33:05 UTC
Fixed in git master
Comment 21 Tobias Mueller 2010-05-31 22:59:27 UTC
Carlos, do you know which commit made the fix? :)
Comment 22 Carlos Garcia Campos 2010-06-01 07:41:06 UTC
Many, the last one is this: 

http://git.gnome.org/browse/evince/commit/?id=ccf1a3cbc3fc2caa4a82713c1cff7a875de292f3

if that is what you are asking for.