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

Links for errors and warnings on status bar are not correctly labelled for screen readers #41406

Closed
Neurrone opened this issue Jan 10, 2018 · 18 comments
Assignees
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Milestone

Comments

@Neurrone
Copy link

  • VSCode Version: Code - Insiders 1.20.0-insider (2bab83a, 2018-01-10T05:13:51.946Z)
  • OS Version: Windows_NT x64 10.0.15063
  • Extensions (4)
    Extension Author (truncated) Version
    vscode-markdownlint Dav 0.12.1
    vscode-rust kal 0.4.2
    rust rus 0.3.2
    markdown-preview-enhanced shd 0.3.2

Another relatively small thing.

Steps to Reproduce:

  1. Go to the status bar of an open file with NVDA open.
  2. You will hear something like:
content info landmark 
link master
link
link
link 0
link
link 0

The "link 0" appear to be indicating the number of errors/warnings. Expected it to read "0 warnings" or "0 errors", perhaps this could be done by aria-label?

Also, not sure if the links read out as just "link" are part of the errors/warning links that launch the problem window (somehow spanning multiple lines in virtual cursor) or if they are different.

@vscodebot vscodebot bot added the insiders label Jan 10, 2018
@dbaeumer dbaeumer added the accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues label Jan 11, 2018
@dbaeumer dbaeumer added this to the On Deck milestone Jan 11, 2018
@chrmarti chrmarti removed the insiders label Jan 18, 2018
@dbaeumer
Copy link
Member

@isidorn is this on our list somewhere. Do we need to address it?

@isidorn
Copy link
Contributor

isidorn commented Sep 24, 2018

@dbaeumer no this is not on the list, so you can adrress it when you want.

@Neurrone
Copy link
Author

Is there a prioritized list of which to fix first? Would be curious to take a look.

@dbaeumer dbaeumer added the bug Issue identified by VS Code Team member as probable bug label Sep 25, 2018
@isidorn
Copy link
Contributor

isidorn commented Oct 10, 2018

I actually prefer the accessiibility issues @Neurrone filled compared to our accessiblity testing team. So assigning this to October to investigate if something can be done here.
@Neurrone thanks for filling great issues

@isidorn isidorn modified the milestones: On Deck, October 2018 Oct 10, 2018
@Neurrone
Copy link
Author

@isidorn Np, this is relatively minor. I'm currently using it at work for web development, and am filing what I've experienced. Appreciate the great work you guys are doing :).

@dbaeumer dbaeumer modified the milestones: October 2018, November 2018 Oct 29, 2018
@dbaeumer dbaeumer modified the milestones: February 2019, March 2019 Feb 22, 2019
@dbaeumer dbaeumer modified the milestones: March 2019, April 2019 Mar 18, 2019
@RMacfarlane RMacfarlane removed this from the April 2019 milestone May 10, 2019
@isidorn
Copy link
Contributor

isidorn commented Jun 18, 2019

This seems like low hanging fruit to me.
@alexr00 would you like to look into this?
If you do not have time let me know and I can assign myself.

@alexr00 alexr00 added this to the June 2019 milestone Jun 19, 2019
@alexr00 alexr00 assigned alexr00 and unassigned dbaeumer Jun 19, 2019
@alexr00
Copy link
Member

alexr00 commented Jun 24, 2019

I talked with @isidorn about this and it looks like this is a status bar problem. None of the items in the status bar are able to be tabbed to and get focus. This was deliberate since the status bar doesn't have new information and it would just be more things to be tabbed through. If we add an aria-label for each status bar item and allow them to be tabbed too, then the label will get read out when the item is clicked or tabbed too.

@Neurrone is tabbing through the status bar something that would be useful?

@bpasero for status bar.

@alexr00 alexr00 modified the milestones: June 2019, July 2019 Jun 24, 2019
@bpasero
Copy link
Member

bpasero commented Jun 24, 2019

Yeah I am not sure what else to say here. If we want to fix this issue by adding tabindex to status bar entries we risk making a lot of people upset that you now need to tab 20 more times around the UI to reach an element of choice (provided that is the only way how to fix this).

@isidorn
Copy link
Contributor

isidorn commented Jun 24, 2019

@alexr00 can you please experiment with setting the tabindex=-1 to the statusbar items.
This basically means that those items should not be tabable, but can be focusable. This might be what we are looking for.
More details here https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/tabindex

@alexr00
Copy link
Member

alexr00 commented Jun 24, 2019

Adding tabindex=-1 causes the title of the element to be narrated when the status bar item is clicked. It also still keeps it from getting tabbed to. This would probably be good to add to all status bar entries.

This still doesn't provide a narration when hovering over the element. @Neurrone is narration on hover useful?

@isidorn
Copy link
Contributor

isidorn commented Jun 24, 2019

@alexr00 my guess would be that narattion on hover is useful, for helping with mouse navigation.
So it seems that tabindex=-1 is better then the current state. So I would go with that approach.
But let's wait for @Neurrone feedback.

@Neurrone
Copy link
Author

  1. Having the aria-label set would be useful regardless of which approach we go for.
  2. Regarding this being focusable, the main use I have for clicking these is to expand / collapse or move focus to the problems or notifications pane. I usually activate these by using the screen reader's virtual buffer, which is able to simulate clicks if its able to find an element, so didn't realize that it isn't reachable via tab navigation. Since the issue's scope is just for labels not being read and this might cause other issues, I suggest we defer changing tabIndex, until we get more feedback about making it tabbable being useful.
  3. I don't use the mouse much, so wouldn't be able to judge how useful hover would be.

@isidorn
Copy link
Contributor

isidorn commented Jun 24, 2019

@Neurrone Thanks for feedback.
@alexr00 @bpasero So I suggest to go with tabindex=-1 for all status bar items. This will not make them reachable via tab navigation, but will make NVDA read out it's title as if it were an aria-label.

@bpasero
Copy link
Member

bpasero commented Jun 24, 2019

Fine by me.

@alexr00 alexr00 modified the milestones: July 2019, June 2019 Jun 24, 2019
@alexr00
Copy link
Member

alexr00 commented Jun 24, 2019

To verify, start a screen reader then click on the errors, warnings, and infos entry in the status bar. It should read something like "link No Problems" or "link 1 Problems".

@isidorn
Copy link
Contributor

isidorn commented Jun 24, 2019

@alexr00 it is cool that we added the tabindex, but could we try to get rid of reading "link" as a prefix. Could you try to add an aria-label which would just be "No Problems" for example, would that change the screen reader behavior to not read "link"?
Basically itemContainer ariaLabel = name

@alexr00
Copy link
Member

alexr00 commented Jun 25, 2019

@isidorn if I do that then it just reads it twice. Once as "No Problems" then again with "link No Problems". I'm going to leave it without the aria-label for now.

@isidorn
Copy link
Contributor

isidorn commented Jun 25, 2019

Thanks for trying it out. Yeah that is due to this NVDA issue nvaccess/nvda#7841

kiku-jw added a commit to kiku-jw/vscode that referenced this issue Jun 26, 2019
* beautify macos keyboard layout label

* Open folders and workspaces in new windows

* Basic file opening via Open File command

* Update auto detect layout info.

* Respect openFoldersInNewWindow setting for folders/workspaces

* Make openWindow function resolve at right time

* keyboard layout status bar item tooltip

* Move workspace menu and action to fileActions.contribution

* Add clarifying comment on instance service request events

* Fullscreen change event.

* Remove unneeded margin on settings editor scrollbar
Fix microsoft#75724

* fix: microsoft#72626

* Remove extra register of automatic tasks

Fixes microsoft#75758

* remove trailing '/' from repo url for baseFolderName

* handle style-attribute modifications, cache requests in addition to results, microsoft#75061

* fix microsoft#75818

* fix bad tree guide indentation

* remove TODO

* update eslint

* update distro

fixes microsoft#73872

* Revert "Revert "Merge pull request microsoft#75695 from orange4glace/master""

This reverts commit a05e05c.

* Revert "Revert "explorero: file actions disablment no longer needed""

This reverts commit b634152.

* more code insets API tweaks, microsoft#66418

* Alpine build

* Update distro hash

* Remove duplicate cp

* shellscript: Add folding markers

* fixes microsoft#75829

* show setting on windows only

* add ExtensionKind and remoteName propsed APIs, microsoft#74188

* debt - use file service based configuration file service

* fix tests

* debt create configuration file service inside configuration service

* First cut of file service based user data service

* Use user data service for reading settings

* Update distro hash

* add diagnostic tool for git file event issues

* 💄

* Update distro hash

* introduce VSCODE_STEP_ON_IT

* remove env scripts

fixes microsoft#74792

* Update xterm.css

Fixes microsoft#75827

* check if file exists

* remove alert, aria-live will read the content even with no focus

fixes microsoft#41356

* win code.sh fix

* 🧀 Fix microsoft#75831

* Add proposed api check for shell API

Part of microsoft#75091

* launch ext host window internally

* EH debugging: support multiple files and folders

* Update distro

* xterm@3.15.0-beta50

Diff: xtermjs/xterm.js@846a189...96eafd3

Changes:

- Publish improvements
- Layering/strict updates

* Fire onDidChangeMaximumDimension when dimensions are set

Fixes microsoft#73496

* Fix potential race

* Delete cached service worker entries after a short timeout

* Fix webview developer command not being registered

* Re-queue canceled geterr requests before remaining buffers

We should give higher priority to files that have previously had geterr triggered on them but did not have their request completed

* Remove log uploader

Fixes microsoft#75748

* Use localized name for macOS keyboard layout

* fixes microsoft#75856

* User keyboard layout

* simplify common keymap layer

* load user keyboard layout after initialization

* US Standard keyboard info

* better score for layout

* fast return keyboard layout if 48-keymap matches

* a single keyboard event can be a keymap

* switch to user selected keyboard layout

* Have `.get` return promise directly

* Make sure we wait until service worker is ready before creating content

* Add version check to service worker

Try to make sure our page is talking to the expected version of the service worker

* Don't use clone as much

* Move host javascript to own file

* Update distro

* Remove icon explorations before shipping stable

* Move listener to window service.

* Minimap: Render find match decorations, fixes microsoft#75216

* Fix `navigator.serviceWorker.ready` is a Promise, not a function

* Use update instead of manually tring to re-register

* Extract ITypeScript server interface

* extract server error to own file

* Extract server spanwer to own file

* Renames

* Move getQueueingType into class

* Add experimental dual TS server

Fixes microsoft#75866

* Enable "typescript.experimental.useSeparateSyntaxServer" for VS Code workspace

* Remove trailing comma

* Include server id in TS server errors

* Make execute command a configuration object

* Also include format in the syntax commands

* Fix method name

* Renames

* Better encapsulate logic of spawning different server kinds

* some fixes for mac web

* New test runner API for microsoft#74555

* update doc, microsoft#74188

* build: release only iff all builds succeed, introduce VSCODE_RELEASE env

* first version of vscode.workspace.fs

* 💄

* Tasks registration + the local ext host now has an autority

Part of microsoft/vscode-remote-release#757

* Add platform override to getDefaultShellAndArgs in terminal

Part of microsoft/vscode-remote-release#757

* Ensure no trailing path separtor on URIs from file picker

Part of microsoft#75847

* data tree view state should store scrollTop, microsoft#74410

* fix microsoft#75564

* Change promise structure of creating terminal in tasks

Potential fix for microsoft#75774

* do not allow additionalProperties

microsoft#75887

* explorer: roots forget children on new file system provider registration

microsoft#75720

* Update max tokenization limit without reload

* Use interfaces for keyboard layout registration

* Separate keyboard layout loading logic for testing

* Test browser keymapper

* unused standard keyboard event.

* Make sure we dismiss the zoom status bar entry when switching editors

* Reduce state

* Added strictly typed telemetry function (microsoft#75915)

* Added strictly typed telemetry function

* cleanup publicLog2 signature

* Extract port mapping helper function

* Re-use extractLocalHostUriMetaDataForPortMapping for openUri

* Also map 127.0.0.1 in webviews and forward it for openExternal

Fixes microsoft/vscode-remote-release#108

* use empty model when content is empty

* 💄

* Update keyboard layout file comments

* Delete breadcrumbs.filterOnType unused setting. Fixes microsoft#75969

* Add quick open/input color registrations (fixes microsoft#65153)

* Update API

* implements ExtHostEditorInsetsShape

* use divs for tree indent guides

fixes microsoft#75779

* comment out more (for microsoft#74898)

* Quick Open > Quick Input (microsoft#65153)

* build - enable language server tests again (for microsoft#74898)

* use polish for wsl1

* move extension kind to Extension-interface

* init log level of remote log service

* Open/Save local commands should not show in the command palette

Fixes microsoft#75737

* chockidar: use polling

* fix build conditions

* xterm fixes for cglicenses

* oss 1.36.0

* workaround for microsoft#75830

* update distro commit

* electron - still call setBounds() as workaround for first window

* fixes microsoft#75753

* node-debug@1.35.3

* remove user data service

* use posix.join

* update doc

* Add -1 tab index to status bar entries

This keeps them out of the tab order, but allows them to be read with a screen reader

Fixes microsoft#41406

* empty view polish labels for remote case

microsoft/vscode-remote-release#511

* send remote watcher error to file service (fixes microsoft/vscode-remote-release#329)

* update distro

* better error handling in case of loader error in tests

* fix win 32 bits unit tests

* electron@4.2.5 (microsoft#76020)

* Code-insiders started from WSL doesn't return to console/ doesn't connect. Fixes microsoft/vscode-remote-release#780

* Group decorations by line before rendering

* disable support for simple fullscreen (microsoft#75054)

* telemetry - add window.nativeFullScreen

* move API to stable,  microsoft#74188

* build - add and use --disable-inspect for integration tests (microsoft#74898)

* 💄

* bump distro

* Report workspace stats in shared process

* Make return undefined explicit

* Add missing return

* Use explicit window.createWebviewManager

* gdpr comments

* webkit fullscreen detection

* Fix file name spelling

* update distro

* add logging

* disabling installing extension from gallery when not enabled

* status.workbench.keyboardLayout

* Move Inspect Keyboard Layout JSON to workbench

* return local extension after install

* install deps and packs while installing from gallery

* Fix default shell selector outside of Windows

Fixes microsoft#76040

* Add explicit win32 gheck for using user specific temp folder

* Always use settings UI when querying online services, fixes microsoft#75542

* Disable conpty in terminal when accessibility mode is on

Fixes microsoft#76043

* Move the webviewResourceRoot property to be set on each webview instead of as a global property

For microsoft#72155

This allows  us to potentially change the resource root per webview

* Make RelativeWorkspacePathResolver  a static class

* Use openExternal

* Auto restart when changing  typescript.experimental.useSeparateSyntaxServer

* Fix regular expression for rewriting iframe webview html replacing quotes

* Telemetry Command (microsoft#76029)

* Added telemetry command

* Initial Build support

* Added build logic for telemetry

* Linux Builds

* Windows builds sort of work

* Remove arm telemetry extraction

* Remove alpine telemetry extraction

* Remove accidental s

* More try catch

* Use full resource uri for transforming webview resources

This ensures we still work even if there is no base uri set

* Use outerHtml to make sure we write `<html>` element from extensions too

* Use a regexp that works across browsers

* Implement reload on iframe based webview Elements

* fix various nls issues

* 💄

* add debug output (microsoft#76024)

* Fix tasks platform for quoting

Fixes microsoft#75774

* fix hockeyapp symbols and report errors (fix microsoft#76024)

* update distro

* fix bad watch

* update distro

* Fix drive letter casing on typescript tasks

Occurs when opening by double clicking on workspace file. Fixes microsoft#75084

* update distro

* update distro

* Test remoteName and extensionKind (for microsoft#76028)

* MainThreadFileSystem does not await

* Fix microsoft#76096

* Rename runInBackground to hideFromUser

See microsoft#75278

* Update distro

* Fix minimap decoration rendering on horizontal scroll, fixes microsoft#76128

* Handle windows paths correctly when loading webvie resources

* Fix standard link handler for iframe based webviews

* Mark extensions.all as readonly

This iteration, we marked a few other arrays as readonly. We should do the same for extensions.all

* Fix microsoft#75927.

* Register mouse down on container dom node.

* Make sure we never cancel a request to just one of the ts servers

Fixes microsoft#76143

* Show document link tooltip first and put click instructions in parens

Fixes microsoft#76077

This change also update our standard link hovers to follow this format

* reset listener once users choose a dedicated keyboard layout

* switch to a new layout only when the score is higher.

* Fix kb unit test

* fix microsoft#76149

* web - document some API

* 💄 workbench API

* disable arm and alpine for stable

fixes microsoft#76159

* Fix extra auto complete on fast delete (microsoft#74675)

Fixes #vscode-remote-release/4

* use yarn --frozen-lockfile for builds

* remove `update.enableWindowsBackgroundUpdates` from online settings

* fix microsoft#76076

* revert the change

* prevent product.json containing gallery

* fix microsoft#76074

* fixes microsoft#54084

* Fix microsoft#76105

* fix microsoft#75904

* workaround for  microsoft#74934
@RMacfarlane RMacfarlane added the verified Verification succeeded label Jun 26, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Aug 8, 2019
joaomoreno pushed a commit that referenced this issue Jun 16, 2020
This keeps them out of the tab order, but allows them to be read with a screen reader

Fixes #41406
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 bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

8 participants