GNOME Bugzilla – Bug 677491
'toolkit-accessibility' default value should be 'true' for 3.6
Last modified: 2014-12-12 12:12:57 UTC
Setting 'toolkit-accessibility' default value to true was proposed as a feature for GNOME 3.6. That proposal has a general positive feedback. You can read the thread here: https://mail.gnome.org/archives/desktop-devel-list/2012-April/msg00107.html
Created attachment 215660 [details] [review] Sets default value of "accessibility-toolkit" to true
Given that the only thing this should do is make GTK+ load a module, and that those modules aren't needed anymore, I think you should make those modules not rely on the setting anymore, and have GTK+ "enable" a11y all the time.
(In reply to comment #2) > Given that the only thing this should do is make GTK+ load a module, and that > those modules aren't needed anymore, I think you should make those modules not > rely on the setting anymore, and have GTK+ "enable" a11y all the time. Changing the default was value what I proposed on the feature proposal, not modify some code. And it is slightly more complex, so I would need to contact them to check if they agree. In the same way, this will probably require some modifications on stuff like Gecko or LibreOffice due their module loading hacks. And for sure on GNOME Shell. Finally, part of the rationale of this change, and implementation, is the following (from my original feature proposal): "In the same way, if something goes wrong (like core app X crashing constantly and no solution), it would be easy to revert the change prior to the end of the cycle." Although that is the worst-case scenario, revert a one-liner in just one module seems more easy that revert a slightly more complex modifications in more than one module.
(In reply to comment #3) > (In reply to comment #2) > > Given that the only thing this should do is make GTK+ load a module, and that > > those modules aren't needed anymore, I think you should make those modules not > > rely on the setting anymore, and have GTK+ "enable" a11y all the time. > > Changing the default was value what I proposed on the feature proposal, not > modify some code. And it is slightly more complex, so I would need to contact > them to check if they agree. I'm not sure the feature proposal process works that way... > In the same way, this will probably require some > modifications on stuff like Gecko or LibreOffice due their module loading > hacks. And for sure on GNOME Shell. Let's enable a11y support in GTK+ now then, and give Mozilla and LibreOffice time to fix their applications properly. > Finally, part of the rationale of this change, and implementation, is the > following (from my original feature proposal): > > "In the same way, if something goes wrong (like core app X crashing constantly > and no solution), it would be easy to revert the change prior to the end of the > cycle." That's a nice idea. But force enabling a11y support in parts of the eco-system we manage will result in a faster turn-around. Crashy applications will get fixed, instead of a11y being re-disabled by all. > Although that is the worst-case scenario, revert a one-liner in just one module > seems more easy that revert a slightly more complex modifications in more than > one module. A one-liner in GTK+ would do fine. Enabling a11y support in the whole desktop also enables a11y support in every non-GTK+ 3.x application, which means that we'll get back performance with GTK+ 2.x apps that use large treeviews, maybe a number of custom applications crashing because they've never been tested with a11y, etc. We control GTK+ 3.x, and that's what we know will handle a11y support well. Let's make enabling the a11y-toolkit option a noop with GTK+ 3.x then we can discuss if we're willing to push the setting to be on for every other application we don't manage, and take the blame for the possible crashers and bad performance.
(In reply to comment #4) > (In reply to comment #3) > > (In reply to comment #2) > > > Given that the only thing this should do is make GTK+ load a module, and that > > > those modules aren't needed anymore, I think you should make those modules not > > > rely on the setting anymore, and have GTK+ "enable" a11y all the time. > > > > Changing the default was value what I proposed on the feature proposal, not > > modify some code. And it is slightly more complex, so I would need to contact > > them to check if they agree. > > I'm not sure the feature proposal process works that way... Well, although I recognize that my paragraph was poorly phrased, the fact is that when you propose a feature you also need to propose a plan to achieve that. And the technologies proposed can be a reason to reject a feature proposal (as it was the case). > > Although that is the worst-case scenario, revert a one-liner in just one module > > seems more easy that revert a slightly more complex modifications in more than > > one module. > > A one-liner in GTK+ would do fine. Enabling a11y support in the whole desktop I don't think that it will be a one-liner in GTK+, although I didn't take a look yet. > also enables a11y support in every non-GTK+ 3.x application, which means that > we'll get back performance with GTK+ 2.x apps that use large treeviews, maybe a > number of custom applications crashing because they've never been tested with > a11y, etc. > > We control GTK+ 3.x, and that's what we know will handle a11y support well. > Let's make enabling the a11y-toolkit option a noop with GTK+ 3.x then we can > discuss if we're willing to push the setting to be on for every other > application we don't manage, and take the blame for the possible crashers and > bad performance. Mmm, yes true, sometimes I forgot that GTK2 and other legacy stuff is there. Taking into account that my proposal was a conservative one, its make sense to be more conservative. I will start to write some patches to the other modules as suggested.
Afaik setting accessibility to "on" does not have an effect on GTK itself though. What it does is cause the settings-daemon to make GTK load the modules named "gail" and "atk-bridge". "gail" is ignored by GTK3 because its always loaded already. "atk-bridge" is outside of GTK's control as it's shipped by ATSPI. So what we could do is always load the atk-bridge module if it exists. But that would lead to a bunch of complications with people installing the atk-bridge module later and then having to restart their computer and so on. So I think there's 2 possible ways: (1) Just turn the setting to ON. (2) Make atk-bridge a library and link to it from GTK3 and in that process turning it always-on.
(In reply to comment #6) > Afaik setting accessibility to "on" does not have an effect on GTK itself > though. > What it does is cause the settings-daemon to make GTK load the modules named > "gail" and "atk-bridge". > "gail" is ignored by GTK3 because its always loaded already. > "atk-bridge" is outside of GTK's control as it's shipped by ATSPI. > > So what we could do is always load the atk-bridge module if it exists. But that > would lead to a bunch of complications with people installing the atk-bridge > module later and then having to restart their computer and so on. Thats the reason I said "A one-liner in GTK+ would do fine." would be really unlikely. > So I think there's 2 possible ways: > > (1) Just turn the setting to ON. Plan mentioned on the feature proposal mail that I sent. Still the option I prefer, in spite that I mentioned "being conservative" could also make sense in the eco-system. *But* we are talking here about GNOME, specifically about GNOME3. In theory we are talking about GTK3 here and as mentioned, we don't control common programs like LibreOffice and Firefox. Is "we'll get back performance with GTK+ 2.x apps that use large treeviews" a real concern for upstream? And after all, each distro could shipt this with a different value. > (2) Make atk-bridge a library and link to it from GTK3 and in that process > turning it always-on. Option that I discarded for 3.6 on the same feature proposal mail due all the unanswered questions pending.
(In reply to comment #7) > (In reply to comment #6) > > Afaik setting accessibility to "on" does not have an effect on GTK itself > > though. > > What it does is cause the settings-daemon to make GTK load the modules named > > "gail" and "atk-bridge". > > "gail" is ignored by GTK3 because its always loaded already. > > "atk-bridge" is outside of GTK's control as it's shipped by ATSPI. > > > > So what we could do is always load the atk-bridge module if it exists. But that > > would lead to a bunch of complications with people installing the atk-bridge > > module later and then having to restart their computer and so on. Isn't this the exact same problem we would have now? If a11y is enabled but atk-bridge isn't installed, things won't work as expected. > Thats the reason I said "A one-liner in GTK+ would do fine." would be really > unlikely. diff --git a/gtk/gtkmodules.c b/gtk/gtkmodules.c index 4d92fa0..09b705c 100644 --- a/gtk/gtkmodules.c +++ b/gtk/gtkmodules.c @@ -551,6 +551,8 @@ _gtk_modules_init (gint *argc, */ g_slist_free (load_modules (gtk_modules_args)); } + /* Always load the atk-bridge, for a11y support */ + g_slist_free (load_modules ("atk-bridge")); } > > So I think there's 2 possible ways: > > > > (1) Just turn the setting to ON. > > Plan mentioned on the feature proposal mail that I sent. Still the option I > prefer, in spite that I mentioned "being conservative" could also make sense in > the eco-system. *But* we are talking here about GNOME, specifically about > GNOME3. In theory we are talking about GTK3 here and as mentioned, we don't > control common programs like LibreOffice and Firefox. Is "we'll get back > performance with GTK+ 2.x apps that use large treeviews" a real concern for > upstream? And after all, each distro could shipt this with a different value. What's the point of changing the default value if the distributions are just going to revert our change? > > (2) Make atk-bridge a library and link to it from GTK3 and in that process > > turning it always-on. > > Option that I discarded for 3.6 on the same feature proposal mail due all the > unanswered questions pending. Let's get those questions answered then, and see what's necessary to get the loading of atk-bridge hard-coded.
The main reason I dislike the + /* Always load the atk-bridge, for a11y support */ + g_slist_free (load_modules ("atk-bridge")); approach is because it gets us back into the mindset we had for a11y for so long. The "Hey, we'll just add a hack here for now and do it right later." mindset. And that is totally not something I want to see happening, in particular not for a11y. Because it's what got us into this mess in the first place.
As discussed with Matthias, for 3.6, we probably want something similar to the hack in comment 8, until we get to implement option 2. What do we want the automatic loading of atk-bridge to do? - Only load on non-windows/non-osx systems? - Don't warn if atk-bridge isn't present? (In reply to comment #9) > The main reason I dislike the > + /* Always load the atk-bridge, for a11y support */ > + g_slist_free (load_modules ("atk-bridge")); > approach is because it gets us back into the mindset we had for a11y for so > long. The "Hey, we'll just add a hack here for now and do it right later." > mindset. > > And that is totally not something I want to see happening, in particular not > for a11y. Because it's what got us into this mess in the first place. I agree that we really don't want that, and somebody will have to start the work on option 2). But we have a short-term fix right here.
Just for completeness: After discussing with Matthias and others a bit, I think the 2 biggest remaining problems are these two: (1) "Old" applications should not have accessibility turned on. (2) The key snooping is still in GTK3's accessibility layer.
> And that is totally not something I want to see happening, in particular not > for a11y. Because it's what got us into this mess in the first place. What is the alternative ? How hard would it be to librarify the gtk3 version of atk-bridge ? I guess the long-term goal would be to have dbus implementations directly in gtk/a11y ?
The long-term goal IMO is to fix the lower a11y layer to not get in the way all the time. This involves defining a proper d-bus protocol and switching apps from using ATK to dbus-codegen'd or hand-coded stuff. No work on this is done anywhere. The medium-term solution would be to switch from a loadable module to a library. A library could initially just expose void atk_bridge_init (void); but should ideally turn into something that gives applications more control, as a lot of the remaining problems with a11y are due to the atk-bridge interaction and the ATK indirection that toolkits have to do.
Created attachment 216344 [details] [review] gtk: Always load the atk-bridge
Created attachment 216405 [details] [review] gtk: Always load the atk-bridge But respect the NO_AT_BRIDGE envvar for gnome-shell and other dual-GTK+/Clutter applications.
(In reply to comment #11) > (2) The key snooping is still in GTK3's accessibility layer. Unfourtunately the last time we talked about that, we didn't get any feedback from X-devel folks: https://mail.gnome.org/archives/gnome-accessibility-list/2012-May/msg00007.html
(In reply to comment #13) > The medium-term solution would be to switch from a loadable module to a > library. A library could initially just expose void atk_bridge_init (void); > but should ideally turn into something that gives applications more control, as > a lot of the remaining problems with a11y are due to the atk-bridge interaction > and the ATK indirection that toolkits have to do. Well, one of the things I mentioned that it was not clear, is if that just moving the atk-bridge to be library (the easiest way to do that) would mean a new dependency on gtk and clutter(or gnome-shell), something that it was not clear (at least for me) at that moment (ie: was mentioned that one way to avoid that would be move all the bridge code to ATK itself). Good to know that this is not a problem anymore. I will start to test some of the stuff done by Bastien. Thanks for all the work.
Review of attachment 216405 [details] [review]: ::: gtk/gtkmodules.c @@ +561,3 @@ + * no way to only initialise Clutter a11y */ + load_bridge = g_getenv ("NO_AT_BRIDGE"); + if (!load_bridge || g_ascii_strtod (load_bridge, NULL) == 0) strtod, really ? is that what the code did before ?
(In reply to comment #18) > Review of attachment 216405 [details] [review]: > > ::: gtk/gtkmodules.c > @@ +561,3 @@ > + * no way to only initialise Clutter a11y */ > + load_bridge = g_getenv ("NO_AT_BRIDGE"); > + if (!load_bridge || g_ascii_strtod (load_bridge, NULL) == 0) > > strtod, really ? > is that what the code did before ? It seems so: http://git.gnome.org/browse/at-spi/commit/?id=a92fb22f Skimming the code of at-spi they use strtod several times, in some cases with odd lines like this: if (debug_env_string) _dbg = (int) g_ascii_strtod (debug_env_string, NULL); Probably it was a typo on at-spi and then C&P to at-spi2.
Created attachment 216438 [details] [review] gtk: Always load the atk-bridge But respect the NO_AT_BRIDGE envvar for gnome-shell and other dual-GTK+/Clutter applications.
(In reply to comment #18) > Review of attachment 216405 [details] [review]: > > ::: gtk/gtkmodules.c > @@ +561,3 @@ > + * no way to only initialise Clutter a11y */ > + load_bridge = g_getenv ("NO_AT_BRIDGE"); > + if (!load_bridge || g_ascii_strtod (load_bridge, NULL) == 0) > > strtod, really ? > is that what the code did before ? Copy/paste from at-spi2-atk I'm afraid. Maybe we should move the actual call to _gtk_accessibility_init() in gtk/a11y/gail.c?
Created attachment 216443 [details] [review] gtk: Always load the atk-bridge But respect the NO_AT_BRIDGE envvar for gnome-shell and other dual-GTK+/Clutter applications.
Why doesn't atk-bridge figure this out itself? Why do we need to check atk's env vars?
(In reply to comment #23) > Why doesn't atk-bridge figure this out itself? Why do we need to check atk's > env vars? I think that it makes sense. That NO_AT_BRIDGE test was done on the method that called adaptor_init. Probably it could make sense to move it to adaptor_init itself. I will modify Bastien patch and propose a different one for atk-bridge.
(In reply to comment #24) > (In reply to comment #23) > > Why doesn't atk-bridge figure this out itself? Why do we need to check atk's > > env vars? > > I think that it makes sense. That NO_AT_BRIDGE test was done on the method that > called adaptor_init. Probably it could make sense to move it to adaptor_init > itself. I will modify Bastien patch and propose a different one for atk-bridge. It doesn't make sense if you read what's necessary for gnome-shell to avoid loading the GTK+ atk-bridge, and have it be attached to clutter instead: http://git.gnome.org/browse/gnome-shell/tree/src/main.c#n377 The code used to be in the GTK+ module, so if we get rid of the module, it should happen in GTK+.
Created attachment 216456 [details] [review] gtk: Always load the atk-bridge Basically Bastien patch but with some changes: * NO_AT_BRIDGE test removed. As Benjamin said, the place to do that hack is at the bridge (see bug 678125) * Moved #include <atk-bridge.h>, for any reason I had a compilation problem with Bastien patch. * Moved the atk_bridge_adaptor_init. This call needs to be done with an available AtkUtil implementation loaded. So I moved it in order to be after _gail_util_install.
(In reply to comment #25) > (In reply to comment #24) > > (In reply to comment #23) > > > Why doesn't atk-bridge figure this out itself? Why do we need to check atk's > > > env vars? > > > > I think that it makes sense. That NO_AT_BRIDGE test was done on the method that > > called adaptor_init. Probably it could make sense to move it to adaptor_init > > itself. I will modify Bastien patch and propose a different one for atk-bridge. > > It doesn't make sense if you read what's necessary for gnome-shell to avoid > loading the GTK+ atk-bridge, and have it be attached to clutter instead: > http://git.gnome.org/browse/gnome-shell/tree/src/main.c#n377 > > The code used to be in the GTK+ module, so if we get rid of the module, it > should happen in GTK+. What we need is a way to say that the bridge shouldn't be loaded, in order to ensure that your atk implementation is the one used. That can happen on the at-spi2-atk side. (more info on bug 678125)
Review of attachment 216456 [details] [review]: thanks, please commit. We need a relase of the librarified atk-bridge soon, then
Attachment 216456 [details] pushed as ffe1e31 - gtk: Always load the atk-bridge
This breaks the build without X11 installed, because at-spi2-core depends on X11. Can it be guarded so that atk-bridge is required only for the X11 backend?
(In reply to comment #30) > This breaks the build without X11 installed, because at-spi2-core depends on > X11. Can it be guarded so that atk-bridge is required only for the X11 backend? Probably just a nitpick, but shouldn't this be a different bug?
I can see it either way, but I hoped that putting it here would catch Bastien's attention and he could fix it quickly.
Created attachment 217038 [details] [review] build: Add --without-atk-bridge, only check for it on X11 platforms Some builders using gtk3 outside of the GNOME cycle want an option to avoid linking to atk-bridge-2.0. Provide that, and at the same time ensure we're only looking for it on X11 platforms.
(Note patch is untested until I figure out why the cairo-pdf.h check is failing in my environment)
Hi Colin, Just a quick question-is your patch for --without-atk-bridge also meant to bypass the check on atk-bridge on Linux as well, or it is only meant for non-X11 platforms? It seems that even when I passed that option into autogen.sh when building the GTK+ tarball for myself on Ubuntu 12.04 LTS, it still looked for atk-bridge-2.0 and bails out when it's not found ("Required package atk-bridge-2.0 not found", something along this line), and I had to grab and build at-spi2-core (master) and at-spi2-atk (master) for the configuring stage to pass. Thank you, with blessings!
Thanks, I've tested it and pushed a slightly fixed version.
Created attachment 217191 [details] [review] gtkmodule blacklist depends on enable-atk-bridge option The issue is that if you compile gtk without atk-bridge support, as gtkmodules still have atk-brige on the blackslit, you couldn't use the bridge with the old "load a module" way. I think that it would be worth to change the blacklist in that case. That would also means that the at-spi2-atk gtk3 module should not disappear, and probably also getting back the gnome_accessibility_module_init method.
Review of attachment 217191 [details] [review]: Looks right; but can you at least add the link to the bug in the commit message?
I'm not a fan of this. It adds another thing we need to support. Having atk-bridge should be mandatory on Linux to build GTK. There is no reason to compile GTK without accessibility support. And on systems where atk-bridge doesn't exist (Windows or OS X), atk-bridge should still be blacklisted. We want to make sure nobody ever loads it. Nobody should be able to use the module. Whether by accident or on purpose.
(In reply to comment #39) > I'm not a fan of this. It adds another thing we need to support. Having > atk-bridge should be mandatory on Linux to build GTK. There is no reason to > compile GTK without accessibility support. > And on systems where atk-bridge doesn't exist (Windows or OS X), atk-bridge > should still be blacklisted. > > We want to make sure nobody ever loads it. Nobody should be able to use the > module. Whether by accident or on purpose. Agreed to that. Otherwise we might as well not have made a11y on by default...
Review of attachment 217191 [details] [review]: Update for Benjamins comment
(In reply to comment #39) > I'm not a fan of this. It adds another thing we need to support. Having > atk-bridge should be mandatory on Linux to build GTK. There is no reason to > compile GTK without accessibility support. To be clear, I just jumped in here because my understanding was that a build-time option was desired. If you have concerns about adding more build configurations (in this case, "GTK+ 3.6 with no atk bridge"), that's something to discuss. However, we should clearly preserve the part of my patch which ties the atk-bridge dependency to X11 or equivalent. > And on systems where atk-bridge doesn't exist (Windows or OS X), atk-bridge > should still be blacklisted. But the module doesn't build for those platforms now, so how could someone load it? I'm not objecting to blacklisting it, but it seems like you're just shooting from the hip here.
(In reply to comment #42) > To be clear, I just jumped in here because my understanding was that a > build-time option was desired. If you have concerns about adding more build > configurations (in this case, "GTK+ 3.6 with no atk bridge"), that's something > to discuss. > I think GTK should have exactly 1 compile-time configuration per supported backend. Everything else is a luxury because we have nobody testing it. Because let's face it, no core developer is gonna run X11 with atk-bridge disabled, so we won't catch any bugs. The same goes for building without XI2 and things like that. The only thing this gives you is more complex code that doesn't work for anyone on Gentoo. > However, we should clearly preserve the part of my patch which ties the > atk-bridge dependency to X11 or equivalent. > Yes, definitely. I consider atk-bridge the same sort of dependency as XI2 or cairo-xlib: A mandatory dependency for the X11 backend, but not necessary anywhere else. In fact, checking for atk-bridge availability should probably be done using #ifdef GDK_BACKEND_X11 (or whatever that define is called). > But the module doesn't build for those platforms now, so how could someone load > it? I'm not objecting to blacklisting it, but it seems like you're just > shooting from the hip here. > Ease of code and stability against idiotic users. Just blacklisting it everywhere is easier to write (no ifdefs required). And people trying to compile atk-bridge on Windows because they think that's the way to get accessibility to work will be caught early in the process.
(In reply to comment #43) > > I think GTK should have exactly 1 compile-time configuration per supported > backend. Everything else is a luxury because we have nobody testing it. That's a reasonable position. > > But the module doesn't build for those platforms now, so how could someone load > > it? I'm not objecting to blacklisting it, but it seems like you're just > > shooting from the hip here. > > > Ease of code and stability against idiotic users. > Just blacklisting it everywhere is easier to write (no ifdefs required). I guess...
Created attachment 217292 [details] [review] build: Drop --without-atk-bridge option Instead, always build it if and only if X11. This reduces the set of supported configurations.
Reopening, seeing as we have some patches up for review.
Review of attachment 217292 [details] [review]: ::: gtk/a11y/gail.c @@ +811,2 @@ _gail_util_install (); +#ifdef GDK_WINDOWING_X11 Should this have an additional run-time check X11 being used?
Comment on attachment 217292 [details] [review] build: Drop --without-atk-bridge option I think the patch is fine as is. From my pov it's the job of the bridge to do any runtime checks, like on OS X with X11 enabled there should be no a11y daemon running, so the bridge shouldn't do anything.
Attachment 217292 [details] pushed as ed8203e - build: Drop --without-atk-bridge option
*** Bug 741416 has been marked as a duplicate of this bug. ***