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 722938 - Adapt a11y code for newest gtk
Adapt a11y code for newest gtk
Status: RESOLVED FIXED
Product: evolution
Classification: Applications
Component: general
3.11.x (obsolete)
Other Linux
: Normal normal
: ---
Assigned To: Evolution Shell Maintainers Team
Evolution QA team
Depends on:
Blocks:
 
 
Reported: 2014-01-24 22:09 UTC by Mike Gorse
Modified: 2014-10-13 17:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch. (40.27 KB, patch)
2014-01-24 22:23 UTC, Mike Gorse
needs-work Details | Review
Revised patch. (44.42 KB, patch)
2014-01-27 21:51 UTC, Mike Gorse
reviewed Details | Review
Revised patch, with a few more fixes. (44.42 KB, patch)
2014-02-02 19:37 UTC, Mike Gorse
needs-work Details | Review
Revised patch. (44.42 KB, patch)
2014-03-12 03:32 UTC, Mike Gorse
committed Details | Review

Description Mike Gorse 2014-01-24 22:09:07 UTC
Gtk's atk implementation was refactored a while back, and AtkObjectFactories are no longer used to create accessibles for gtk widgets, so fetching the AtkObjectFactory associated with the parent class and deriving from it no longer works.

https://bugzilla.gnome.org/show_bug.cgi?id=669441 is similar but for gtkhtml.
Comment 1 Mike Gorse 2014-01-24 22:23:18 UTC
Created attachment 267163 [details] [review]
Proposed patch.

Note that this ups the required gtk+ version to 3.8, since GtkWidgetAccessible and its related classes only became public in 3.8. It is possible to refactor in a way that will work with older gtk versions, by adding a custom get_accessible implementation for our classes, but doing this is messy (ie, requires cutting and pasting a bunch of code from GtkWidgetAccessible).
Comment 2 Milan Crha 2014-01-27 16:27:42 UTC
Thanks for a bug rpeort and patch. It, unfortunately, produces compiler warnings. Please fix them first. Those I see are:

gal-a11y-e-tree.c:35:13: warning: 'priv_offset' defined but not used [-Wunused-variable]
 static gint priv_offset;

ea-calendar.c:39:1042: warning: 'ea_cal_view_factory_get_type' defined but not used [-Wunused-function]
 EA_FACTORY (EA_TYPE_CAL_VIEW, ea_cal_view, ea_cal_view_new)

ea-calendar.c:40:1042: warning: 'ea_day_view_factory_get_type' defined but not used [-Wunused-function]
 EA_FACTORY (EA_TYPE_DAY_VIEW, ea_day_view, ea_day_view_new)

ea-calendar.c:44:1049: warning: 'ea_week_view_factory_get_type' defined but not used [-Wunused-function]
 EA_FACTORY (EA_TYPE_WEEK_VIEW, ea_week_view, ea_week_view_new)

ea-week-view-main-item.c:187:81: warning: redundant redeclaration of 'ea_week_view_main_item_class_init' [-Wredundant-decls]
 G_DEFINE_TYPE_WITH_CODE (EaWeekViewMainItem, ea_week_view_main_item, GAIL_TYPE_CANVAS_ITEM,

ea-week-view-main-item.c:35:13: note: previous declaration of 'ea_week_view_main_item_class_init' was here
 static void ea_week_view_main_item_class_init
Comment 3 Mike Gorse 2014-01-27 21:51:37 UTC
Created attachment 267351 [details] [review]
Revised patch.

Fix compiler warnings; remove some now unused variables and definitions.
Comment 4 Milan Crha 2014-01-28 12:10:27 UTC
Thanks for the update, this is much better. I'm currently getting runtime warnings when I'm closing evolution in a Calendar view, as they name also:
> Gtk-CRITICAL **: gtk_widget_get_accessible: assertion
> 'GTK_IS_WIDGET (widget)' failed
and they were not there before, then I believe they are brought in by the commit. What I've done:
a) from terminal: evolution -c calendar
b) in opened in a day view; find a week with at least one event and select it
   in the left-bottom calendar, together with one week before at and one after
   the event
c) close evolution by 'X', of File->Quit

There are many (new) runtime warnings on the close, not only the cited one above.
Comment 5 Mike Gorse 2014-02-02 19:37:10 UTC
Created attachment 267863 [details] [review]
Revised patch, with a few more fixes.

Thanks for the testing. I've fixed a few more issues, although I'm not entirely sure if what I've done is correct in some cases. Ie, sometimes get_n_accessible_children would always return that an object has 1 child, but ref_child could not find the child and would end up printing a warning and returning NULL, so I've made get_n_accessible_children return 0 in those cases.
Comment 6 Milan Crha 2014-02-03 10:06:58 UTC
Thanks for the update. It's better, but unfortunately still too many runtime warnings on quit from the Calendar view. The first is shown in the backtrace below. The problem is that runtime warnings are there to show that something is going wrong, and here are going wrong many things.

  • #0 g_logv
  • #1 g_log
    at gmessages.c line 1025
  • #2 g_type_check_instance
    at gtype.c line 4094
  • #3 g_signal_connect_data
  • #4 ea_cal_view_real_initialize
    at ea-cal-view.c line 133
  • #5 gtk_widget_real_get_accessible
    from /lib64/libgtk-3.so.0
  • #6 gtk_notebook_page_accessible_new
    from /lib64/libgtk-3.so.0
  • #7 create_notebook_page_accessible.isra.1
    from /lib64/libgtk-3.so.0
  • #8 gtk_notebook_accessible_initialize
    from /lib64/libgtk-3.so.0
  • #9 gtk_widget_real_get_accessible
    from /lib64/libgtk-3.so.0
  • #10 gtk_container_accessible_real_remove_gtk
    from /lib64/libgtk-3.so.0
  • #11 g_closure_invoke
    at gclosure.c line 777
  • #12 signal_emit_unlocked_R
    at gsignal.c line 3586
  • #13 g_signal_emit_valist
    at gsignal.c line 3330
  • #14 g_signal_emit
    at gsignal.c line 3386
  • #15 gtk_widget_dispose
    from /lib64/libgtk-3.so.0

Comment 7 Mike Gorse 2014-03-12 03:32:04 UTC
Created attachment 271569 [details] [review]
Revised patch.

I wasn't seeing the warnings that you're getting, but I noticed that EaCalViewEvent was being derived with the wrong size, so I modified it to use glib macros to create the get_type function. I think that that should be done for all of the types, but that should probably be a separate bug. Anyway, I'm curious whether it is related to those warnings.
Comment 8 Milan Crha 2014-03-12 16:20:21 UTC
Review of attachment 271569 [details] [review]:

Thanks for the update. I looked on the runtime warnings you do not see and it turned out that it's not your fault, it's just that your changes uncovered an issue in the related code. I fixed that and will commit it together with your changes. Thanks again for the work on this.
Comment 9 Milan Crha 2014-03-12 16:25:18 UTC
Created commit 03ca50d in evo master (3.11.92+) [1]

[1] https://git.gnome.org/browse/evolution/commit/?id=03ca50d
Comment 10 Milan Crha 2014-10-13 17:12:33 UTC
There is regression after this patch, filled as bug #738463. I made some changes for current git master (other my changed in-there as well), which could influence other things as well, but otherwise it seems like the gtk_widget_class_set_accessible_type() doesn't do the right thing for some reason. I'm using gtk 3.10.6, but gtk 3.14.3 seems to suffer of the same issue.

Mike, could you join bug #738463, please?