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

Orca fails to reliably speak the settings tree #92488

Closed
ajborka opened this issue Mar 11, 2020 · 60 comments
Closed

Orca fails to reliably speak the settings tree #92488

ajborka opened this issue Mar 11, 2020 · 60 comments
Assignees
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues important Issue identified as high-priority settings-editor VS Code settings editor issues

Comments

@ajborka
Copy link

ajborka commented Mar 11, 2020

@isidorn: In Linux, when Orca is in focus mode, it doesn't reliably speak the settings tree when navigating with the arrow keys. This causes problems because the user has no reference point when navigating the tree. I am thinking that the selection is moving the focus, but the focus itself isn't reported to Orca.

  • VSCode Version: 1.44 (Insider's)
  • OS Version: Ubuntu 20.04, Orca master (3.37.1 pre).

Steps to Reproduce:

  1. Start Orca screen reader.
  2. Start vscode with accessibility mode turned on.
  3. Press ctrl+, (comma) for the settings window.
  4. Press e to navigate to the settings search.
  5. Press TAB twice to navigate to the settings tree.
    Note: This is the only way to access the settings tree with Orca. Everything else fails.
  6. Press arrows to navigate the settings tree.

Actual results: In most cases, Orca remains silent. However, Orca will announce the tree items, but only after revisiting a tree node.
Expected results: Orca should reliably speak each tree item when in focus mode.

@isidorn
Copy link
Contributor

isidorn commented Mar 11, 2020

@ajborka this is basicaly a duplicate of this issue #57372
The settings tree is designed to be navigated only by tab which I think is wrong and I think a user should be able to navigate it via arrow keys.
Forwarding this to @roblourens since he owns the settings tree and he said he might reconsider the navigation design if users complain. And now you are complaing which is great :)

@roblourens this makes the settings tree very unfriendly to accessiblity users and almost unusable. Thus adding important label. Hope that is ok.

If we do not want to change the settings UI design an alternative is to always use the JSON editor when we detect a screen reader is attached.

@isidorn isidorn added accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues important Issue identified as high-priority settings-editor VS Code settings editor issues labels Mar 11, 2020
@ajborka
Copy link
Author

ajborka commented Mar 11, 2020

@isidorn,
See #92485 for an explanation on why the json editor isn't a wise choice as a default, especially for new users.

@joanmarie
Copy link

@isidorn I can reproduce what @ajborka says regarding Orca being silent the first time a tree node is navigated to. Will investigate that aspect.

@ajborka
Copy link
Author

ajborka commented Mar 13, 2020

@isidorn , @joanmarie I investigated more on this. When going into focus mode with Orca, and the settings tree has focus, pressing down arrow changes focus or (selection?). However, when using flat review (numpad keys), the focus or selection does not change.

  1. Turn on Orca.
  2. Start vscode.
  3. Press ctrl+, (comma).
  4. Press TAB twice to enter the settings tree.
  5. Press numpad+5. Orca speaks 'commonly used'.
  6. Press down arrow a few times.
  7. Repeat step 5.

Actual result: Orca still speaks 'commonly used.'.
Expected results: Orca tracks the focus change.

Even though this might be an Orca problem, I am putting it here in case vscode might contribute to the problem.

@joanmarie
Copy link

@isidorn. Ok, I can reproduce the problem without Orca, but using an accessibility event listener.

Steps to reproduce in Linux:

  1. Run the attached listener in a terminal (after removing the ".txt" extension; github complained about ".py").
  2. Launch VSCode (relaunch it if it is already running)
  3. Arrow Down several times in the Settings tree

Expected results: The listener would print out state-changed events for items in the tree.
Actual results: The listener does not print out any state-changed events in the tree.

  1. Arrow Back up in the Settings tree

Now there are events. And if you arrow back down there are events. This is why Orca initially isn't presenting anything.

state-changed.py.txt

@joanmarie
Copy link

@ajborka Does the flat review problem only occur when Orca is not presenting the items as you arrow? If so, then that's the same issue as in your opening report and what I just stated above.

@ajborka
Copy link
Author

ajborka commented Mar 13, 2020

@joanmarie flat review remains on the commonly used node even when Orca speaks the tree items. The problem seems to be inconsistent. When arrowing up and down the tree, some items that were announced may not get announced a second time, but do a third. This is not a problem for JAWS and NVDA though.

@joanmarie
Copy link

@ajborka Ok, I was just able to reproduce the flat review issue after Orca did present a tree item. That's very likely an Orca bug. Will see about fixing that.

The missing events (i.e. Orca not initially presenting items when you navigate down) is a bug in either VSCode or Chromium.

@joanmarie
Copy link

@ajborka So this is going to be a good-news/bad-news thing. The good news is that the flat review issue is now fixed in Orca master. The bad news is that the fix means Orca needs to rebuild the flat review context when you switch between focus mode and flat review. In the particular case of VSCode, which has a zillion accessible objects, that's not a very performant task for Orca to perform.

So.... I will see if there's a way to make the context-rebuilding more performant for very objectful web apps. In the meantime, it's a known issue.

@isidorn
Copy link
Contributor

isidorn commented Mar 16, 2020

@joanmarie @ajborka thanks for looking into this!

If I understand the full issue correctly the problem in VS Code is the following: up / down keyboard navigation does not correctly traverse the Settings tree. That can be fixed in VS Code and is the cause of the issue on our end. Up / down should go through each element and not go into the values of settings. This issue is unrelated to screen readers, focus movement is simply wrong. Am I missing something here or am I oversimplifiying?

@ajborka
Copy link
Author

ajborka commented Mar 16, 2020

@isidorn , @joanmarie To summarize: the settings tree which contains categories such as commonly used, editor, workbench, application, etc is not properly read by Orca in Linux. Based on @joanmarie 's testing, the tree isn't sending out focus change events when first navigated so screen readers can properly read the focus change. Settings pane itself has its own issues that we discussed in another report.

@isidorn
Copy link
Contributor

isidorn commented Mar 16, 2020

@ajborka thanks for the summery. However that tree is just like all other trees in vscode (it is using a same tree implementation). It nicely sets the activedescendent based on the focused element. So not sure what would be special about it.

@ajborka
Copy link
Author

ajborka commented Mar 16, 2020

@isidorn : as @joanmarie said in an earlier post, vscode is not firing some events when keyboard navigation is performed. Try running her sample code. She can reproduce the problem without a screen reader running.

@joanmarie
Copy link

joanmarie commented Mar 16, 2020

@isidorn I've attached a modified pyatspi accessibility-event listener which you can use without Orca and which potentially sheds some more light on where the problem might be: vscode-tree-issues.py.txt

Here's what I did using the listener:

  1. Preparation (we want VSCode to not have trees of interest showing until we interact with them)
    a. Close settings
    b. Click on the Search button in on the left
    c. Quit VSCode
  2. Launch the attached accessibility listener (again, nuke the ".txt" from the file name)
  3. Press Ctrl+Shift+E to cause the Explorer to be visible. Note what the event listener prints out.
  4. Press Ctrl+comma to cause Settings to be visible.
  5. Arrow down in the tree. Note what the event listener prints out.
  6. Arrow up in the tree. Note what the event listener prints out.

My output from performing these steps is below. But what I find interesting is that as soon as the explorer is showing, the tree gets populated (see the object:children-changed:add events). But this doesn't happen when the settings tree is initially shown. In fact, there are no events from the settings tree descendants until you start arrowing down in the settings tree. And when you do that, children get removed/destroyed; not added. It's not until you start up arrowing that children get added.)

So.... I get that the tree is just like all the others. But is there something else which you're doing for the explorer tree which causes their accessibility objects to be immediately created? Alternatively, are you doing something to the settings tree which would cause container to destroy its children?

# This happens when the explorer is initially shown with Ctrl+Shift+E
[section | ] object:children-changed:add [tree item | test]
[section | ] object:children-changed:add [tree item | orca-scalable.svg]
[section | ] object:children-changed:add [tree item | po]
[section | ] object:children-changed:add [tree item | .gitignore]
[section | ] object:children-changed:add [tree item | ChangeLog]
[section | ] object:children-changed:add [tree item | acinclude.m4]
[section | ] object:children-changed:add [tree item | AUTHORS]
[section | ] object:children-changed:add [tree item | orca-symbolic.svg]
[section | ] object:children-changed:add [tree item | orca-splash.svg]
[section | ] object:children-changed:add [tree item | configure.ac]
[section | ] object:children-changed:add [tree item | ChangeLog-pre-2.27.1]
[section | ] object:children-changed:add [tree item | src]
[section | ] object:children-changed:add [tree item | orca-256x256.png]
[section | ] object:children-changed:add [tree item | m4]
[section | ] object:children-changed:add [tree item | .cvsignore]
[section | ] object:children-changed:add [tree item | autogen.sh]
[section | ] object:children-changed:add [tree item | orca-24x24.png]
[section | ] object:children-changed:add [tree item | help]
[section | ] object:children-changed:add [tree item | AUTHORS]
[section | ] object:children-changed:add [tree item | orca-16x16.png]
[section | ] object:children-changed:add [tree item | Makefile.am]
[section | ] object:children-changed:add [tree item | icons]
[section | ] object:children-changed:add [tree item | orca-32x32.png]
[section | ] object:children-changed:add [tree item | orca-48x48.png]
[section | ] object:children-changed:add [tree item | orca-22x22.png]
[section | ] object:children-changed:add [tree item | COPYING]
[section | ] object:children-changed:add [tree item | docs]

# This happens when I Tab to give part of the tree focused and then arrow
[tree item | docs] object:state-changed:focused True
[tree item | docs] object:state-changed:focused False
[tree item | help] object:state-changed:focused True
[tree item | help] object:state-changed:focused False
[tree item | icons] object:state-changed:focused True
[tree item | icons] object:state-changed:focused False
[tree item | AUTHORS] object:state-changed:focused True

# When I press Ctrl+comma to open settings causing the settings tree
# to be shown, the listener prints nothing.

# Each time I arrow down in the settings tree, children get removed.
# One press of Down (e.g. from "Text Editor" to "Workbench" in the tree)
[section | ] object:children-changed:remove [DEAD]
[section | ] object:children-changed:remove [DEAD]
[section | ] object:children-changed:remove [DEAD]
[section | ] object:children-changed:remove [DEAD]
[section | ] object:children-changed:remove [DEAD]
[section | ] object:children-changed:remove [DEAD]

# Another press of Down (e.g. from "Workbench" to "Window" in the tree)
[section | ] object:children-changed:remove [DEAD]
[section | ] object:children-changed:remove [DEAD]
[section | ] object:children-changed:remove [DEAD]
[section | ] object:children-changed:remove [DEAD]
[section | ] object:children-changed:remove [DEAD]
[section | ] object:children-changed:remove [DEAD]

# One press of Up:
# Don't worry about the events from text. That's a known issue and irrelevant to this problem.
# What is interesting is NOW we get the children being added.
[text | Window] object:children-changed:remove [DEAD]
[text | Features] object:children-changed:remove [DEAD]
[section | ] object:children-changed:remove [DEAD]
[section | ] object:children-changed:add [tree item | window.autoDetectColorScheme Auto Detect Color Scheme Auto Detect Color Scheme. If set, automatically switch to the preferred color theme based on the OS appearance.]
[tree item | Window, group] object:state-changed:selected True
[tree item | Window, group] object:state-changed:focused True
[section | ] object:children-changed:add [tree item | window.enableMenuBarMnemonics Enable Menu Bar Mnemonics Enable Menu Bar Mnemonics. Controls whether the main menus can be opened via Alt-key shortcuts. Disabling mnemonics allows to bind these Alt-key shortcuts to editor commands instead.]
[section | ] object:children-changed:add [tree item | window.menuBarVisibility Menu Bar Visibility Control the visibility of the menu bar. A setting of 'toggle' means that the menu bar is hidden and a single press of the Alt key will show it. By default, the menu bar will be visible, unless the window is full screen. Menu Bar Visibility. ]
[section | ] object:children-changed:add [tree item | window.closeWhenEmpty Close When Empty Close When Empty. Controls whether closing the last editor should also close the window. This setting only applies for windows that do not show folders.]
[section | ] object:children-changed:add [tree item | Window]
[section | ] object:children-changed:add [tree item | window.doubleClickIconToClose Double Click Icon To Close Double Click Icon To Close. If enabled, double clicking the application icon in the title bar will close the window and the window cannot be dragged by the icon. This setting only has an effect when Window: Title Bar Style is set to custom.]
[section | ] object:children-changed:add [tree item | window.customMenuBarAltFocus Custom Menu Bar Alt Focus Custom Menu Bar Alt Focus. Controls whether the menu bar will be focused by pressing the Alt-key. This setting has no effect on toggling the menu bar with the Alt-key.]

@isidorn
Copy link
Contributor

isidorn commented Mar 16, 2020

@joanmarie thanks a lot for this testing. I wrote the Explorer tree, however I did not write the Settings tree. For that @roblourens would be best to answer if we are doing something special there.
If he is busy I can also investigate deeper, but for now let's wait for Rob to reply.

@joanmarie can you please clarify are you navigating the actual settings or the settings tree of content on the left?

@joanmarie
Copy link

joanmarie commented Mar 16, 2020

@joanmarie can you please clarify are you navigating the actual settings or the settings tree of content on the left?

The settings tree on the left where it says "Text Editor", "Workbench", "Window", "Features", .... That is the thing that Orca is failing to present when the user initially down arrows.

I appologise if you are visualy impaired and if these gifs are not very helpful

I am sighted. But the fast movement of the second is literally making me motion sick. Assuming we're on the same page, perhaps delete that image?

I also just edited the listener output to make where I was arrowing more clear (I hope!).

@roblourens roblourens added this to the March 2020 milestone Mar 16, 2020
@roblourens
Copy link
Member

roblourens commented Mar 16, 2020

I replaced the inline gif with a link.

Yeah, I don't do anything weird with that list as far as I know. I don't implement getActiveDescendantId but it looks like that should be ok right @isidorn? And the attribute on the list element is still set properly. Also this works with VoiceOver on mac.

Maybe all the changes in the tree on the right that happen when it scrolls are interfering somehow?

I am trying to run your script, but it doesn't print anything out. Are you using python3? Can you tell me how you installed pyatspi?

@roblourens
Copy link
Member

Oh, my Ubuntu VM is not GNOME. That is probably an issue

@roblourens
Copy link
Member

Hm I still don't get any output, do you think there are some other prereqs I have to install? I had to modify the script to get rid of errors, changed the top part to

import gi
gi.require_version('Atspi', '2.0')
from gi.repository import Atspi
import pyatspi

I am not a python person, I'm not sure what that's doing.

@isidorn
Copy link
Contributor

isidorn commented Mar 17, 2020

@joanmarie appologies for the gifs, I have removed them.
@roblourens you do not need to implement getActiveDescendantId that is only if you have a sub-element that gets focus (like the explorer for multiple compacted folders in one element). Tree will handle the activeDescendent setting automatically. And yes I also see all the attributes properly set on the Table of Contents tree.

My main suspect is the overall structure of the Settings UX page. And what @roblourens suggest: when I change the Table of Contents a lot of elemnts are changing in the other tree.
Note when I inspect the monaco-list-rows for the content of the actual settings it takes the whole page real estate (coverring the Table of Contents section). So they seem to be somewhat overlapping, could this potentialy confuse Orca?

I suggest the following:

  1. Remove the second Tree which shows the settings, and only render the Table of content tree
  2. Check if the issue still reproduces

Once we know that it will be easier to investigate in the right direction.
Let me know what you think. Thanks!

Screenshot 2020-03-17 at 10 31 18

@ajborka
Copy link
Author

ajborka commented Mar 17, 2020

@roblourens that Python code tries to import the GTK+ user interface library. It has nothing to do with atspi in this context. It technically should be something like:

gi.require_version('Gtk', '3.0')

@isidorn there is a possibility that the settings design is interfering with Orca since it is the only tree in the entire application that fails to work as expected. It would be worth getting a build of vscode with only the settings tree displayed on the settings screen so I can test the theory. Thinking about the difference between the file explorer tree and the settings tree, this is what I came up with:

  1. The settings tree dynamically loads UI elements as you arrow through the tree contents, the files list doesn't change UI elements.
  2. Since you are using an older version of electron, is it possible the chromium implementation is not yet accessible under linux?

It is worth trying to rebuild the UI screen a component at a time to see where the tree breaks, and possibly upgrade electron to 8 or 9 to see if it fixes the problem.

@isidorn
Copy link
Contributor

isidorn commented Mar 17, 2020

@ajborka makes sense. For a build without a settings tree, I suggest that @roblourens just makes to changes and runs VS Code out of source and tries it out.
We could try a newer version of electron, however I could not find an exploration build of electron 8 or 9 for Linux. @deepak1556 do we have that somehwere? Not sure if this will solve the issue, but does not hurt to try it out.

@ajborka
Copy link
Author

ajborka commented Mar 17, 2020

@isidorn Electron is cross-platform. There are no linux builds. Here is the github repo for electron's releases. They are already up to 10.x prerelease, but I recommend 8.1.1 since it is a stable build, although it might not hurt to try a prerelease of 10.x since it should have chromium 82 in it.

@joanmarie
Copy link

  1. If I manualy remove the settings tree from the DOM the issue is no longer reproducable. Thus our suspicion is correct that having two trees in the DOM somehow confuses Chrome / Orca.

I guess then file a bug against Chromium. Again, what my dead-simple pyatspi listener does is listen for events and spit them out. That shows the problem exists independent of Orca.

@roblourens
Copy link
Member

roblourens commented Mar 21, 2020

I haven't forgotten about this issue - I spent a while this morning trying to get this to work in a couple different Linux vms in Parallels and had a variety of issues. After trying to update one of them it ended up totally broken, so my current status is setting up a fresh one so I can start over form scratch. So basically, the same thing that happens every time I spend a lot of time debugging Linux issues 😆😭

And my understanding is that something about the way the two trees on the settings page are arranged is interfering with the left one's accessibility events, so I should figure out why and probably find a different way to arrange them.

@roblourens
Copy link
Member

And on a totally new VM, espeak just crashes when orca starts.

image

@ajborka
Copy link
Author

ajborka commented Mar 23, 2020

@roblourens did you figure out your espeak problem? What linux distro are you trying to use?

@roblourens
Copy link
Member

Ok I actually got it to work and can reproduce the issue. Here are some things I know

  • If I simply delete the list of settings on the right, it works properly
  • I thought that the overlapping layout with the two trees might be an issue, but if I give them a normal side by side layout, there is no change
  • Toggling different CSS properties on the settings tree, no impact
  • If I drag the TOC somewhere else in the DOM, there is no difference
  • It works fine when multiple other trees are on the screen, like the search tree + settings editor trees

So what else is different? When the selection changes in the TOC, it scrolls the settings editor. I will try disabling that.

@roblourens
Copy link
Member

Ok, so if I disable the action that the TOC takes when a row is focused, then this actually works. That might be one workaround - in accessibility mode, we can require the user to press enter to select a TOC row. Would that be acceptable? But I'd like to fix the issue.

Trying to reduce to a Chromium bug. It works fine in Firefox. The workaround doesn't actually fix it in Chromium but there are other issues there with the web UI too, like it doesn't read the search tree or the explorer at all. @isidorn do you know if there are issues there or is there something else I have to do to set it up? I did have to check these two checkboxes to get it to read anything:

image

@ajborka
Copy link
Author

ajborka commented Apr 6, 2020 via email

@roblourens
Copy link
Member

roblourens commented Apr 6, 2020

It sounds like other contributing issues are that the settings list is virtualized, but the actual rows don't get focused so you can't interact with it like other trees in vscode. Maybe it could work in a different mode like that when a screen reader is in use. Will have to think about it. Not a straightforward change. Could you open a new issue?

@isidorn
Copy link
Contributor

isidorn commented Apr 6, 2020

@roblourens it seems like you are also hitting this issue #94404
And thanks for looking into this and trying to distill a chrome issue

@roblourens
Copy link
Member

Oh that would be it, thanks @isidorn.

@roblourens
Copy link
Member

I think that this should be fixed now, between the accessibility changes this summer to make this work like a normal list, and electron updates. Please let me know if that's not the case.

@roblourens roblourens removed this from the On Deck milestone Nov 5, 2020
@ajborka
Copy link
Author

ajborka commented Nov 5, 2020

@roblourens , @isidorn the settings tree works as expected. FYI, the new implementation is represented as two treeviews: one for the settings TOC, and another for the settings.

@isidorn
Copy link
Contributor

isidorn commented Nov 5, 2020

Yeah, I think that is expected.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues important Issue identified as high-priority settings-editor VS Code settings editor issues
Projects
None yet
Development

No branches or pull requests

5 participants