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

Basic accessibility support for the terminal view #344

Merged
merged 4 commits into from
Jun 29, 2018

Conversation

pvagner
Copy link
Contributor

@pvagner pvagner commented Jun 20, 2017

Terminal view as implemented in the termux app is a view resembling terminal windows known from computers. It's designed to be efficient however for me and other people who are using accessibility services such as Google Talkback to access their android devices it's not very usable. By adding contentDescription property to the view dynamically as it repaints, we can get at least very basic accessibility support.
When using termux on Android with Talkback running it is possible to review currently visible text on the screen by characters, by words and by lines.
It is possible to use two finger scrolling (android built-in feature) to scroll the view and access other parts of the text.
I'm using this for a few months already and I have got some friends interested in this functionality so I'm submitting this as a pull request.
I know it's very minimalistic, but even in its current limited form it's a life changing feature for tech savvy blind people so hopefully you can accept it and / or point out how I might be able to enhance it so everyone can benefit.

services can get the text currently being shown.
@fornwall
Copy link
Member

Thanks a lot!

Is it necessary to call setContentDescription(getText()) on each render call, or would it be enough to only call it when updating the screen in TerminalView.onScreenUpdated()?

@pvagner
Copy link
Contributor Author

pvagner commented Jun 26, 2017

Hello,
I just wanted to make sure we will get all the updates.
Now when thinking more about it I assume
TerminalView.onScreenUpdated()
would be enough.
I'll change and test it today.

@pvagner
Copy link
Contributor Author

pvagner commented Jun 26, 2017

Changed, tested and working fine.

@pvagner
Copy link
Contributor Author

pvagner commented Jul 1, 2017

@fornwall Please can you take a look one more time?
I hope it's changed as you have suggested.

@stormdragon2976
Copy link

Did this ever get added to Termux? I installed it today and wasn't able to read the terminal's output. Is there something I need to do to get it working with Talkback?

@pvagner
Copy link
Contributor Author

pvagner commented Apr 14, 2018

This PR is still open thus not merged to the code base.
It still merges cleanly so your only option in order to try this out is to rebuild termux on your own either using Android studio or gradle directly cherrypicking this patch.

@stormdragon2976
Copy link

I'm having lots of issues with the jre and android-studio playing nicely together. Is there an apk somewhere that I can just install until this is merged?

@Grimler91
Copy link
Member

@stormdragon2976 you can try this one: https://grimler.se/apps/termux-app-access.apk
I have the other apps available from there as well, signed with the same key.

I haven't tested it at all so it's great if you are willing to!

@stormdragon2976
Copy link

Thanks so much for this. It is very helpful. The accessibility is very basic indeed, but it does work. Definitely a good start.

@fornwall
Copy link
Member

@pvagner I'm really sorry for forgetting about this one, and I would really like to see it included!

Could you (or anyone else interested) have a look at enabling this only if accessibility services are enabled (or something similar), as this may slow down the app needlessly if not?

Perhaps with something like this in the TerminalView constructor

private boolean mAccessibilityEnabled;
[...]
public TerminalView(Context context, AttributeSet attributes) {
[...]
    AccessibilityManager am = (AccessibilityManager) context.getSystemService(ACCESSIBILITY_SERVICE);
    mAccessibilityEnabled = am.isEnabled();

and then guarding the call with if (mAccessibilityEnabled) setContentDescription(getText());. Would this work?

@pvagner
Copy link
Contributor Author

pvagner commented Jun 14, 2018

I'll be able to use and integrate this code snipped in a week or so. I'm sure something like this is doable.

@fornwall
Copy link
Member

Thanks a lot!

It's beyond the scope of this immediate change, but could you give some pointers about possible next steps to improve accessibility support?

@pvagner
Copy link
Contributor Author

pvagner commented Jun 27, 2018

@fornwall I apologize for another delay. I have tested your suggestion on my devices and it's working fine over here. Hopefully I've got it right.

I can imagine a few more improvements to the TerminalView accessibility support however I am not sure I am able to implement them.

  • It would be nice to sent out AccessibilityEvent.TYPE_VIEW_SCROLLED when scrolling the view so we know how much content there is displayed in the view.
  • Currently we are exposing the whole view similar to a text view. We set the text retrieved from the visible content as an accessible description and we are using screen reader features to navigate over that. We might consider implementing our own AccessibilityNodeInfo representing each line seperately. That would enable screen reader users to use their fingers to explore individual lines on the screen. This might then be further enhanced by implementing custom accessibility actions for example allowing screen reader users to select and copy parts of the text from the TerminalView.
  • There is one major disadvantage with the current approach I have taken in regard to TerminalView accessibility and I am unable to come up with a better plan. However we need to be able to somehow expose the caret control where it makes sense. We can type, but screen readers can't detect where the caret is pointing to. For simple commands it's all right but for more complex commands and when using text editors, ncurses apps and similar pseudo UI equivalent interfaces accessibility support with the caret handling is not very usefull.

Perhaps there is more however these are things I can imagine being either critical for the comfortable usage of the app or easy or moderate to implement.

@fornwall fornwall merged commit d1f0c76 into termux:master Jun 29, 2018
@fornwall
Copy link
Member

Thanks a lot, both for this change and the suggestions for further improvements!

@vortex1024
Copy link

any idea about what would be required to make new text be read automatically in the terminal? I guess using an accessibility live region for the whole terminal view won't work, as the whole terminal would be read out, not only the new text. A hidden accessibility region containing only the new text would work, but sounds like an ugly hack. Other ideas? Maybe something linked to the accessibility node info @pvagner talked about?

alzwded added a commit to alzwded/termux-app that referenced this pull request Aug 15, 2024
**tl;dr** it's a lot easier to read the output of the last command
and to get to the copy/paste/more... actions

It's a bit yiffy to navigate the wall of text in the terminal view. As things
stand right now with content description, accessibility services see the
`TerminalView` as an opaque blob with a wall-of-text description. Since it's
a description, there is nothing locking you in the control when using reading
controls (like you would be in a `TextView`).

Additionally, if I read the documentation of `setContentDescription` right,
it is really meant to offer a description of non-textual elements, or of
irritatingly opaque content; *not* to be the content itself. This is probably
irrelevant, as the goal was to make it easier to get to the output of the
last command.

It is also nigh impossible to notice the popup menu (copy/paste/more...)
if you don't... *see* it. It ends up at the end of the control stack and it
is not announced (if I understand correctly, you have to issue an accessibility
focus event, but the accessibility of the selection handle thing is a whole
different problem to the one I'm trying to solve here).

The goal was to make the `TerminalView` behave more like a text view:
- have reading controls (by line, word or line) available regardless of
  what the system guesses, within the terminal view itself
- don't accidently leave the terminal view when using reading controls
  too much (e.g. by word)
- ability to easily jump to end, by swiping up/back with reading controls
  after activating the view (since it no longer "escapes")
- note, reading controls are different to basic navigation; basic navigation
  let's you flick between the view, soft keys (CTRL, ALT etc), copy/paste/menu
  popup, your soft keyboard, etc; reading controls allow you to navigate
  a wall of text within a control, i.e. the terminal view
- added accessibility actions to open the More... menu and copy and paste
  actions; note, these are apparent if TalkBack is running

Actual changes, on `TerminalView`:
- explicitly mark the view as important to accessibility, because it does
  things which are important to accessibility, and it is the main focus of
  the whole app. The documentation of that method asks to set the flag to
  `YES` if a view does any of the things below;
- call `sendAccessibilityEvent(TYPE_VIEW_TEXT_CHANGED)` when text changes
  and implement `onPopulateAccessibilityEvent` to let accessibility services
  know the text has changed (and what it changed to)
- implement `onInitializeAccessibilityNodeInfo` which is called by
  accessibility services to detect what is on screen; this gives us a chance
  to explain that this `TerminalView` is a multiline text container and not
  some random eye candy that may be skipped over
- no longer set content description, since we now set actual content by
  way of `AccessibilityNodeInfo` and `AccessibilityEvent`; replaced with
  all of the above
- added custom accessibility actions for copy (screen), paste, and
  open termux menu (the *More...* menu).  Accessibility Actions are something
  an Accessibility Service may query and present in a way that makes sense
  to the user. In the case of *TalkBack*, it has an *Actions* "reading mode"
- added new strings to `strings.xml` and added 3 new IDs to `ids.xml` to
  support the custom accessibility actions.
- refactor method out from `onTouchEvent`, branch `BUTTON_TERTIARY`,
  to use it for the accessibility paste action

The *Copy* action could be better. I had 6 goes at integrating with the
custom selection widget handle thing, I failed. So the *Copy*-accessibility
action will copy the screen as a compromise.

Something ought to be done about the side panel, since you rather have to
know it's there. But since the More... menu is now easily discoverable,
someone with TalkBack might stumble onto the Help panel and read that there's
a secret side panel on the left. That wouldn't be much different to the way
I myself discovered that sessions side panel on the left :-)

It would be worth adding landmarks for the terminalview, button bar and
side panel, but that's a different PR, since it probably needs to be done
in the Activity, not here.

There's also something iffy about the fact that `AccessibilityManager.isEnabled()`
is only checked at start up.  Maybe it should be documented somewhere that
accessibility is checked only at startup for performance reasons. For people
who turn TalkBack on after launching the app, it might appear broken, even
though it really isn't. I didn't want to touch that behaviour since it was
something requested in the review of !344[1]. I am merely stating my dissent.

[1]: termux#344 (comment)

Someone also mused about having the terminal speak out text as it comes in,
but that's also a different PR, since it rather requires interplay between
the TerminalEmulator code and TerminalView (providing accessibility), plus
setting up a virtual view that is live; and I assume people might want to
turn that off / control verbosity, which is a lot of work, so... not in this
PR.

It is theoretically possible to do partial text updates in `onPopulateAccessibilityEvent`,
(instead of blasting away the whole screen each time you type a character)
but I'm not convinced it actually works the way I hope, and it requires a
lot of work to get there (see previous paragraph). So... not in this PR.

But this PR makes interacting with the TerminalView itself much nicer
if you can't see what you're doing, and there's no reason to require people
to see what they're doing in this case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants