Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add accessibility suport to Linux shell. #19634

Merged

Conversation

robert-ancell
Copy link
Contributor

@robert-ancell robert-ancell commented Jul 10, 2020

Description

Add initial accessibility support to the Linux shell. This exposes the widget tree via ATK and allows actions to be perfomed on the nodes. Using accerciser I am able to run the example app and click the button via ATK. I don't expect this to be completely functional for a a11y consumer, but we should be able to build from here. If we develop common code to do the AtkObject logic then much of this PR would remain, and we could replace the core of it.

Related Issues

flutter/flutter#23601

Tests

Unit tests added for FlAccessibleNode. No integration tests added - this code is much like the rendering code in that it is not easily testable without running in a real environment. Code has been manually tested using accerciser and the Orca screen reader.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the contributor guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the C++, Objective-C, Java style guides for the engine.
  • I read the tree hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

@fluttergithubbot
Copy link
Contributor

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@robert-ancell robert-ancell marked this pull request as draft July 10, 2020 04:40
@auto-assign auto-assign bot requested a review from chinmaygarde July 10, 2020 04:41
@robert-ancell robert-ancell force-pushed the linux-shell-fl-accessibility-channel branch from 6a4bb00 to 06f750d Compare September 3, 2020 04:03
@robert-ancell robert-ancell force-pushed the linux-shell-fl-accessibility-channel branch 2 times, most recently from bd85c20 to a483552 Compare November 20, 2020 03:06
@robert-ancell robert-ancell force-pushed the linux-shell-fl-accessibility-channel branch 3 times, most recently from e39e8db to 9665f63 Compare December 2, 2020 21:46
@robert-ancell robert-ancell force-pushed the linux-shell-fl-accessibility-channel branch from 9665f63 to 4741e6d Compare December 9, 2020 03:39
@robert-ancell robert-ancell marked this pull request as ready for review December 9, 2020 03:40
@chinmaygarde chinmaygarde requested review from cbracken and removed request for chinmaygarde December 10, 2020 22:51
@stuartmorgan
Copy link
Contributor

No tests added - this code is much like the rendering code in that it is not easily testable without running in a real environment.

Is there (longer term) integration testing that we can do that can inspect accessibility data provided to the system? E.g., on iOS the standard UI testing infrastructure actually uses accessibility data to identify elements, so it's relatively simple to test at least portions of the accessibility tree that's vended by the application; is there anything similar for Linux?

shell/platform/linux/fl_accessible_node.h Show resolved Hide resolved
@@ -440,6 +464,10 @@ gboolean fl_engine_start(FlEngine* self, GError** error) {
self->settings_plugin = fl_settings_plugin_new(self->binary_messenger);
fl_settings_plugin_start(self->settings_plugin);

result = self->embedder_api.UpdateSemanticsEnabled(self->engine, TRUE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to tell whether accessibility is being used at the system level? The mobile embeddings only enable this when an assistive technology is enabled at the OS level, so that we're not paying all of the overhead of maintaining the tree when it's not being used.

@cbracken Do you know how expensive this is for the engine to do? I don't know if it's something we need to worry about on desktop or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a GSettings key (gsettings get org.gnome.desktop.interface toolkit-accessibility) that is set by gnome-settings-daemon when the screen-reader or the screen-keyboard is enabled. I haven't found anything that consumes this - GTK seems to generate all the ATK objects always regardless of them being consumed. This key may not be set on systems that aren't running gnome-settings-daemon and if we relied on it then a11y might not work on those systems.

shell/platform/linux/fl_engine.cc Outdated Show resolved Hide resolved
shell/platform/linux/fl_view.cc Show resolved Hide resolved
shell/platform/linux/fl_view_accessible.cc Outdated Show resolved Hide resolved
shell/platform/linux/fl_accessible_node.cc Show resolved Hide resolved
shell/platform/linux/fl_accessible_node.cc Outdated Show resolved Hide resolved
shell/platform/linux/fl_accessible_node.cc Show resolved Hide resolved
shell/platform/linux/fl_accessible_node.cc Outdated Show resolved Hide resolved
@robert-ancell
Copy link
Contributor Author

No tests added - this code is much like the rendering code in that it is not easily testable without running in a real environment.

Is there (longer term) integration testing that we can do that can inspect accessibility data provided to the system? E.g., on iOS the standard UI testing infrastructure actually uses accessibility data to identify elements, so it's relatively simple to test at least portions of the accessibility tree that's vended by the application; is there anything similar for Linux?

This is not something I know of any easy methods or had any good experience in getting working. What I know of:

  • xvfb-run is a tool that runs your test in a virtual framebuffer. This allows us to run a full GTK app and test that. It appears to be able to run the example Flutter app fine. There doesn't seem to be a common equivalent for running a virtual Wayland session.
  • We'll need to be running inside a D-Bus session to access AT-SPI, which might mean we need to run dbus-run-session on the tests.
  • There was a project called Autopilot that provided testing using the a11y layer, but I'm pretty sure this is dead.

I'll continue to investigate if we can get the above working and add FlView tests first.

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@robert-ancell robert-ancell merged commit 20991a5 into flutter:master Jan 13, 2021
@robert-ancell robert-ancell deleted the linux-shell-fl-accessibility-channel branch January 13, 2021 00:49
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 13, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 13, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 13, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 13, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 13, 2021
zanderso pushed a commit to flutter/flutter that referenced this pull request Jan 13, 2021
* 51dd6aa [web] Enable the new rich paragraph implementation (flutter/engine#23162)

* 0f9fc3d Roll Skia from 7cf3addb1bd8 to 93c2d81f199a (1 revision) (flutter/engine#23614)

* a1e7424 [dart-runner] Avoid calling Destroy on nullptr (flutter/engine#23608)

* 846d958 Windows textures: Add placeholder flutter_texture_registrar.h (flutter/engine#23623)

* 55afc18 Roll Dart SDK from 7fcbd388b620 to ef8bf7f0a667 (5 revisions) (flutter/engine#23628)

* d2b8154 [canvaskit] apply invser scale on the left (flutter/engine#23550)

* be1a3c0 Roll Fuchsia Linux SDK from UB6RsTbdU... to FfWbbB4r8... (flutter/engine#23633)

* f743c89 Roll Skia from 93c2d81f199a to 9fd75e96d712 (29 revisions) (flutter/engine#23635)

* cf42dbe Roll wuffs to google/wuffs@c86add2 (flutter/engine#23607)

* f1278d0 Link SkShaper/SkParagraph into the engine by default (flutter/engine#23626)

* fb56b4b Android deeplink sends "path + query" instead of just path (flutter/engine#23561)

* 22bb891 Plumbing refactor to allow the usage of Dart_CreateIsolateInGroup (flutter/engine#23549)

* 20991a5 Add accessibility suport to Linux shell. (flutter/engine#19634)

* 145922b Roll Dart SDK from ef8bf7f0a667 to 636ff0ec97e0 (1 revision) (flutter/engine#23639)

* 176ae6e Roll Fuchsia Mac SDK from oll0Dgp9o... to JSzm8D59u... (flutter/engine#23641)

* d2320a8 Roll Dart SDK from 636ff0ec97e0 to d3d7b77e8165 (1 revision) (flutter/engine#23642)

* 8bdd099 Roll Skia from 9fd75e96d712 to 38ca513408d1 (1 revision) (flutter/engine#23643)

* cc572e1 Roll Skia from 38ca513408d1 to be2a8614c5d6 (2 revisions) (flutter/engine#23644)

* e9383a0 Roll Dart SDK from d3d7b77e8165 to 010633edc631 (1 revision) (flutter/engine#23645)

* 9d6ed8b Roll Fuchsia Linux SDK from FfWbbB4r8... to BUsKF6z4t... (flutter/engine#23646)

* 6d55dd4 Roll Dart SDK from 010633edc631 to 724d9e5e7d71 (1 revision) (flutter/engine#23647)

* 299f081 Roll Skia from be2a8614c5d6 to 0d7de6bc9ac3 (1 revision) (flutter/engine#23648)

* 0ec99cf Roll Fuchsia Mac SDK from JSzm8D59u... to BsUY1yjWh... (flutter/engine#23650)

* 859494f Revert "[web] Enable the new rich paragraph implementation (#23162)" (flutter/engine#23651)

* 648dae9 Roll Skia from 0d7de6bc9ac3 to 92969f265686 (7 revisions) (flutter/engine#23652)
gspencergoog pushed a commit to gspencergoog/engine that referenced this pull request Jan 20, 2021
Add accessibility support to the Linux shell
hjfreyer pushed a commit to hjfreyer/engine that referenced this pull request Mar 22, 2021
Add accessibility support to the Linux shell
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants