microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.53k stars 29.4k forks source link

Autocomplete accessibility glitch #3787

Closed zersiax closed 4 years ago

zersiax commented 8 years ago

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.

egamma commented 8 years ago

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

alexdima commented 8 years ago

@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 commented 8 years ago

Hi,

Awaiting that test version. Will wait for it before continuing conducting tests :)

alexdima commented 8 years ago

@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:

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 commented 8 years ago

Hi,

My primary area of development is the web, so don't worry about going into aria details :) AAs for the implementation, I will research the bug you have filed, see if other projects ran into the same thing. If this bug is fixed, the ARIA implementation would indeed be preferable. The interesting thing is that for the longest time, the behavior you are describing regarding NVDA not reading lines or characters was actually the default. This was the primary reason I deemed the product completely inaccessible last year. I'm guessing there's no way to quickly flip the focus back and forth without inconveniencing the user? As for remapping the keyboard keys, the problem with that I think is that there's not all that many keys that would be suitable. Ideally it would be single keys to make the interaction as smooth as possible, which means I guess only the arrow keys and tab + shift+tab would suffice without sacrificing efficiency. Would like to hear your input. I will test the current implementation now and report back soon.

zersiax commented 8 years ago

I have just tested the current implementation and see the limitations. At least the feature is usable now, which is already a very big plus. We will have to wait on the Chromium devs to see if the bug you found can be fixed on their end. Also, what is the second element in a help tag supposed to denote, return type? This reports undefined for HTML elements.

zersiax commented 8 years ago

To anyone concerned wondering on status, it seems the chromium bug reported has been picked up by someone who seems to know their stuff. It was smart to leave the ARIA code in but commented since we'll be needing it soon I predict. Also, another disadvantage of only using alerts is the fact these don't get reported on the braille display. This is fine for general alerts, but for this having that work properly could actually be a huge productivity booster. My guess is that the ARIA way of doingthings rather than alert will make this happen, which is a big plus. I'll report in once I know more.

alexdima commented 8 years ago

@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 commented 8 years ago

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

isidorn commented 5 years ago

@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 commented 5 years ago

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.

zersiax commented 4 years ago

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 commented 4 years ago

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

zersiax commented 4 years ago

Perfect, thanks :-)

isidorn commented 4 years ago

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 commented 4 years ago

Kamino cloned this issue to ChaseKnowlden/vscode

isidorn commented 4 years ago

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