GNOME Bugzilla – Bug 654630
accessibility needs updating to work with gtk 3.1 refactor
Last modified: 2014-04-22 17:20:07 UTC
Accessibility has been refactored somewhat in the latest gtk--the accessibility classes are now inside gtk rather than being in a separate gtk module, and linking atk types to gtk types now works differently, so that the code to fetch the GtkWidget accessible type no longer works. I talked with Benjamin Otte on IRC, since the gtk a11y headers are not currently installed. He said that ideally they would be exported but that they won't be in the short term because they need more clean-up before he is happy with them. So for now we might want to derive directly from GTK_TYPE_ACCESSIBLE (which would require replicating some of the GailWidget/GtkWidgetAccessible code).
I've added some documentation here: http://git.gnome.org/browse/gtk+/commit/?id=3cc22eed44b519c23fba2e2d32314c56d17c1b7b
(In reply to comment #1) > I've added some documentation here: > > http://git.gnome.org/browse/gtk+/commit/?id=3cc22eed44b519c23fba2e2d32314c56d17c1b7b Is there a reason not to allow third-party widgets to derive from the GtkWidgetAccessible and related classes, other than that the headers should be cleaned up and so aren't currently public? I'm working on a patch for vte to derive the terminal accessible directly from GtkAccessible, in the interest of ensuring that it doesn't break for 3.2, so I need to paste a significant amount of code from gtkwidgetaccessible.c to compute the stateset, notify when it gains focus, handle the widget being destroyed, etc. (VteTerminalAccessible already implements most of AtkComponent, so grab_focus is the only vfunc I have to copy for it.) It isn't ideal in the long term if ATK implementations for custom widgets will need to do this; for one thing, if the behavior of GtkWidgetAccessible changes, then atk implementations of third-party widgets deriving from it may need to be maintained in parallel, and the general behavior is different from that of gtk widgets, where creating a custom widget will have the new widget inherit all of the behavior of the parent widget except where it is changed.
Created attachment 192313 [details] [review] Proposed patch. Tested with gtk+ master and 2.22.1 (are we still supporting GTK 2?) I've noted in comments where vfuncs are copied/adapted from gtk+.
I thought I'd commented on this bug already, but obviously didn't. This patch is working for me, patch applies cleanly against vte 0.28.1 in Ubuntu oneiric, and haven't experienced any regressions with a11y and vte. I'd like to apply this patch against Ubuntu's vte package, however official upstream endorcement would make it easier for this patch to be used without question.
Review of attachment 192313 [details] [review]: ::: src/vte.c @@ +11450,3 @@ +static gboolean +gtk_includes_a11y_refactor (void) I don't like this. We can just drop gtk2 support (just do it in configure for now; it's already completely gone on vte-next) and depend on gtk 3.1.x. ::: src/vteaccess.c @@ +806,3 @@ + gboolean value; + + if (g_strcmp0 (pspec->name, "has-focus") == 0) pspect->name is never NULL, so just strcmp(). @@ +862,3 @@ ATK_OBJECT_CLASS (vte_terminal_accessible_parent_class)->initialize (obj, data); +#if GTK_CHECK_VERSION (2, 22, 0) Just drop gtk2. @@ +932,3 @@ + g_signal_connect_after (terminal, "focus-in-event", G_CALLBACK (focus_cb), NULL); + g_signal_connect_after (terminal, "focus-out-event", G_CALLBACK (focus_cb), NULL); + g_signal_connect (terminal, "notify", G_CALLBACK (notify_cb), NULL); Wrong indentation. @@ +2200,3 @@ + gtk_widget_get_allocation (viewport, &viewport_allocation); + +#if GTK_CHECK_VERSION (3, 0, 0) Same.
(In reply to comment #5) Thanks for the review. > @@ +932,3 @@ > + g_signal_connect_after (terminal, "focus-in-event", G_CALLBACK (focus_cb), > NULL); > + g_signal_connect_after (terminal, "focus-out-event", G_CALLBACK > (focus_cb), NULL); > + g_signal_connect (terminal, "notify", G_CALLBACK (notify_cb), NULL); > > Wrong indentation. It doesn't look as though any one style is used consistently throughout the code, so you'll need to tell me what would be "right."
The lines just before that are indented 8 spaces or 1 tab, while the new line is 2 spaces. Just go with 8 spaces.
Created attachment 193993 [details] [review] Updated patch. Fixes some indentation, and removes some gtk version checks. Also remove GTK 2 support from configure.in and the Makefile (so we now have an unused python directory in the repository, although I haven't removed it).
Hmm. I just thought you'd do - 2.0|3.0) ;; + 3.0) ;; in configure and leave all the other stuff in there... but this is ok as well. Please commit to the vte-0-28 branch ONLY (and keep this bug open, since this will need to be adapted to vte-next).
Actually, commit to the vte-0-30 branch which I'll now cut.
(In reply to comment #9) > Hmm. I just thought you'd do > > - 2.0|3.0) ;; > + 3.0) ;; > > in configure and leave all the other stuff in there... but this is ok as well. I'd assumed you'd wanted it gone, and I've surely broken GTK 2 support with that commit, but, anyway, I've committed the patch into vte-0-30 (but not to master).
That's on master now too.