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

Autocomplete accessibility glitch #3787

Closed
zersiax opened this issue Mar 7, 2016 · 17 comments · Fixed by #87880
Closed

Autocomplete accessibility glitch #3787

zersiax opened this issue Mar 7, 2016 · 17 comments · Fixed by #87880
Assignees
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues on-release-notes Issue/pull request mentioned in release notes suggest IntelliSense, Auto Complete upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-fixed The underlying upstream issue has been fixed upstream-issue-linked This is an upstream issue that has been reported upstream verified Verification succeeded
Milestone

Comments

@zersiax
Copy link

zersiax commented Mar 7, 2016

There's something odd going on when typing actual code into Vs Code. To reproduce:

  1. Create a file called index.html.
  2. Type .
  3. Press enter.
    Expected behavior:
    The cursor goes to the next line.
    Perceived behavior:
    Nothing actually happens. You have to press Enter again to go to the next line.

Expected cause: I think an autocomplete list pops up that the accessibility stack isn't aware of. Pressing the first enter likely selects the first option in this list.
Problem with this hypothesis: Arrowing up and down should make different things happen if my theory is correct, however it doesn't seem to do that. Therefore I am not a 100% sure I am correct, someone comment on it to check please.
I think fixing this so the prsumed opup is correctly handled should be the next step for accessibility of this product.

@weinand weinand added the accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues label Mar 7, 2016
@egamma egamma added this to the March 2016 milestone Mar 8, 2016
@egamma
Copy link
Member

egamma commented Mar 8, 2016

@zersiax we are currently investigating into making auto complete accessiblity aware.

@alexdima
Copy link
Member

alexdima commented Mar 8, 2016

@zersiax Indeed, the autocomplete list "steals" Enter, arrow up and arrow down when it pops up. I have pushed a change this morning to make arrow up and down read through the proposals as they are focused and just now another change to alert when a suggestion is accepted (Enter).

We will do some more testing on our side, and then I would love to give you a VSCode version containing these changes and get more feedback from you in this area.

Thank you!

@zersiax
Copy link
Author

zersiax commented Mar 8, 2016 via email

@alexdima
Copy link
Member

@zersiax I apologize if I'm going too much into HTML aria details.

Two days ago I made a change to make the suggest widget use proper aria semantics. In other words, when the suggest widget would pop up, I would add aria-haspopup="true", aria-autocomplete="list" and I would set aria-activedescendant to the id of the current suggest proposal.

This would give a pleasant experience when the suggest widget would pop up, it would announce the first suggestion and using arrow up and down would announce each suggestion, while focus was still in the editor (i.e. typing was still possible in the editor to narrow down results or to ignore the suggestions completely).

After further testing, I have discovered that when closing the suggest widget (when accepting a suggestion or when dismissing it with Escape), I would correctly on my side remove the aria-activedescendant attribute from the input element. However, navigating with arrow keys would no longer announce each character or each line, it would be completely silent. I believe this is a bug in Chromium, where they do not handle correctly the removal of aria-activedescendant until you focus out of the input and focus back in to it.

This issue is blocking the usage of proper ARIA semantics, as soon as suggestions are shown once, cursor navigation becomes completely silent. I have captured this issue and another 2 related issues in Chromium bug 593646.

In the meantime, I have commented out for now the usage of proper ARIA semantics, and have replaced them with alerts. I am sorry, I know alerts are probably the worst option, but here is the current behaviour:

  • when the suggestions pop up, they alert with the pre-selected suggestion. This is useful to inform that Enter will now be accepting said suggestion.
  • when the suggestions are shown, using arrow down/up or page down/up will change the selected suggestion. This will be announced again through an aria alert. Unfortunately, when pressing arrow down/up, NVDA reads for me the entire line again, and only afterwards the next suggestion. I believe NVDA expects that cursor down/up would move the cursor to a different line, thus its desire to read the entire line again. However, in this case cursor down/up keeps the cursor on the current line and selects the previous/next suggestion. Now that I think about it, we could overcome this by changing the keybindings for selecting previous/next suggestion to something else than arrow up/arrow down. I will try this out and get back to you.
    • when a suggestion has a documentation label we alert that it "has details". The documentation label can be read by using Ctrl+Space again. Ctrl+Space acts as a sort of toggle between suggestion items and an item's details (when they are available).
    • when a suggestion item has been accepted, we alert that it has been accepted.

I would be very happy if you could try it out, I am sure this still needs a lot more fine-tuning.

Here is a version of VSCode that will be installed in parallel with the stable version and that we can use for continuous feedback - patches.

@zersiax
Copy link
Author

zersiax commented Mar 10, 2016 via email

@zersiax
Copy link
Author

zersiax commented Mar 10, 2016 via email

@zersiax
Copy link
Author

zersiax commented Mar 11, 2016 via email

@alexdima alexdima added the upstream Issue identified as 'upstream' component related (exists outside of VS Code) label Mar 15, 2016
@alexdima alexdima modified the milestones: Backlog, March 2016 Mar 15, 2016
@alexdima
Copy link
Member

@zersiax Thank you for trying this out.

Moving focus to another element only to move it back in the same JS callstack has no effect. I could try to move the focus to another element and then move it back with a setTimeout of 0, but that might lead to the case that a keypress is missed (e.g. if a keypress is queued in the event loop). This might lead to missing typed characters.

Nice catch with the "undefined" for HTML suggestions, that was indeed the "typeLabel", which is undefined for HTML suggestions.

I have also added additional keybindings out of the box for navigating the suggestion list: Alt+DownArrow, Alt+UpArrow, Alt+PageDown, Alt+PageUp. These function the same as DownArrow, UpArrow, PageDown and PageUp when the suggest widget is opened, but have the advantage that they don't confused NVDA into reading the current line again.

@zersiax
Copy link
Author

zersiax commented Apr 11, 2016

heads up peeps :) It seems the chromium bug being referenced here is seeing some activity. @alexandrudima you might want to keep an eye out, I think you'll be able to uncomment the proper ARIA semantics soon

@alexdima alexdima added the electron Issues and items related to Electron label Mar 9, 2017
@alexdima alexdima added the bug Issue identified by VS Code Team member as probable bug label Apr 27, 2017
@bpasero bpasero added the suggest IntelliSense, Auto Complete label Nov 12, 2017
@alexdima alexdima removed the bug Issue identified by VS Code Team member as probable bug label Nov 24, 2017
@isidorn
Copy link
Contributor

isidorn commented Jun 18, 2019

@alexandrudima the chrome issue has been fixed and we are using the version of chrome which has the fix.
Let's look into reusing the proper aria labels and where that leads us.

@isidorn isidorn self-assigned this Jun 18, 2019
@isidorn isidorn modified the milestones: Backlog, On Deck Jun 18, 2019
@isidorn isidorn removed the electron Issues and items related to Electron label Jun 18, 2019
@isidorn isidorn modified the milestones: On Deck, July 2019, August 2019 Jul 5, 2019
@isidorn
Copy link
Contributor

isidorn commented Jul 10, 2019

The textarea of the editor must dynamically change aria-haspopup, aria-autocomplete and aria-activedescendant attributes based on the suggestion widget state.
For this we will need to add editor api setAria(options)

Assigning to next milestone so I invest time then to fix this properly.

@alexdima alexdima modified the milestones: August 2019, September 2019 Aug 26, 2019
@isidorn isidorn modified the milestones: September 2019, October 2019 Sep 30, 2019
@alexdima alexdima added upstream-issue-linked This is an upstream issue that has been reported upstream upstream-issue-fixed The underlying upstream issue has been fixed labels Oct 9, 2019
@isidorn isidorn modified the milestones: October 2019, November 2019 Oct 25, 2019
@alexdima alexdima modified the milestones: November 2019, December 2019 Dec 2, 2019
@zersiax
Copy link
Author

zersiax commented Dec 17, 2019

Gentle bump :-) I would say this is among the most asked questions I get when people use Code with screen readers, what is the status of this one?

@isidorn
Copy link
Contributor

isidorn commented Dec 17, 2019

I plan to look into this issue this or next week. Will keep you updated here. Thanks

@zersiax
Copy link
Author

zersiax commented Dec 18, 2019

Perfect, thanks :-)

@isidorn
Copy link
Contributor

isidorn commented Dec 30, 2019

I have created a PR which should solve this issue #87880
If the PR looks good we will review it some time next week and than insiders should have the fix for this. You can follow the PR for more details. Thanks

@ChaseKnowlden
Copy link
Contributor

Kamino cloned this issue to ChaseKnowlden/vscode

@isidorn
Copy link
Contributor

isidorn commented Jan 14, 2020

Adding verified per twitter thread https://twitter.com/zersiax/status/1216749912370663426

@isidorn isidorn added the on-release-notes Issue/pull request mentioned in release notes label Jan 31, 2020
@vscodebot vscodebot bot locked and limited conversation to collaborators Feb 24, 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 on-release-notes Issue/pull request mentioned in release notes suggest IntelliSense, Auto Complete upstream Issue identified as 'upstream' component related (exists outside of VS Code) upstream-issue-fixed The underlying upstream issue has been fixed upstream-issue-linked This is an upstream issue that has been reported upstream verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants