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

screen reader issue since 1.89.0 with open file CTRL+p #211976

Closed
dstpierre opened this issue May 3, 2024 · 3 comments · Fixed by #212248
Closed

screen reader issue since 1.89.0 with open file CTRL+p #211976

dstpierre opened this issue May 3, 2024 · 3 comments · Fixed by #212248
Assignees
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders quick-pick Quick-pick widget issues regression Something that used to work is now broken
Milestone

Comments

@dstpierre
Copy link

Does this issue occur when all extensions are disabled?: Yes/No
Yes

  • VS Code Version: 1.89.0
  • OS Version: Windows 10

Steps to Reproduce:

I'm using NVDA as screen reader, before 1.89.0 this worked:

  1. Opening the "search file by name or symbol" with CTRL+p
  2. If I type path where only 1 file exists, the file name isn't "announced", cannot do up/down arrow as there's only one result.
  3. In fact, the first file isn't announce for result with more than one results.

To be clear, image we have the following file/directory tree

|- dir1/
|--- abc.go
|- main.go

Doing CTRL+p and than typing dir1/ it displayed the abc.go file but screen reader says nothing.

For result with multiple file, the first file of the list isn't "sent" to screen reader, doing down arrow and the second file in list is "said" by scree reader.

If I may this is a huge problem.
Thanks

@TylerLeonhardt
Copy link
Member

TylerLeonhardt commented May 3, 2024

Looking at NVDA's log, I see this:

d
1 Results  
Search files by name (append : to go to line or @ to go to symbol)  list
abc.go dir1, file results  1 of 1
i
r
slash

with your example:
image

So we do announce the name, but the second you type another character that gets lost. In 1.88 it would re-say the file.

@TylerLeonhardt TylerLeonhardt added bug Issue identified by VS Code Team member as probable bug quick-pick Quick-pick widget issues accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues regression Something that used to work is now broken and removed new release labels May 3, 2024
@TylerLeonhardt TylerLeonhardt added this to the May 2024 milestone May 3, 2024
@TylerLeonhardt
Copy link
Member

TylerLeonhardt commented May 3, 2024

The change that caused this regression is here:
#208913

cc @joaomoreno

and locally, in main, I reverted that commit and the issue went away.

It looks like we were depending on that insert in order to trigger the a11y event to read things out.

Repro:

  • I used NVDA on Windows
  • have a workspace with the following structure:
    • dir/
      • abc.go
    • main.go
  • Open the file picker (ctrl+P)
  • type, slowly, d I r.
  • 🐛 file abc.go only gets read out on the d and not the other typed characters

Here's the Quick Pick tree accessibility provider, if it helps:

class QuickInputAccessibilityProvider implements IListAccessibilityProvider<IQuickPickElement> {
getWidgetAriaLabel(): string {
return localize('quickInput', "Quick Input");
}
getAriaLabel(element: IQuickPickElement): string | null {
return element.separator?.label
? `${element.saneAriaLabel}, ${element.separator.label}`
: element.saneAriaLabel;
}
getWidgetRole(): AriaRole {
return 'listbox';
}
getRole(element: IQuickPickElement) {
return element.hasCheckbox ? 'checkbox' : 'option';
}
isChecked(element: IQuickPickElement): IValueWithChangeEvent<boolean> | undefined {
if (!element.hasCheckbox || !(element instanceof QuickPickItemElement)) {
return undefined;
}
return {
get value() { return element.checked; },
onDidChange: e => element.onChecked(() => e()),
};
}
}

And where we setChildren:

this._tree.setChildren(null, elements);

I'm surprised by this behavior, esp since we don't use an identity provider anymore.

@joaomoreno
Copy link
Member

The tree seems to be doing the right thing for performance sake: it is not modifying the DOM. Before #208913, the list row was thrown away and rendered again for no good reason. That had the nice side effect of A11Y being read as we desired.

Trying to find a way to kick NVDA to still read it, even though nothing has changed.

joaomoreno added a commit that referenced this issue May 8, 2024
joaomoreno added a commit that referenced this issue May 10, 2024
* quick pick: a11y workaround

fixes #211976

* fix tests
@VSCodeTriageBot VSCodeTriageBot added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Keyboard, mouse, ARIA, vision, screen readers (non-specific) issues bug Issue identified by VS Code Team member as probable bug insiders-released Patch has been released in VS Code Insiders quick-pick Quick-pick widget issues regression Something that used to work is now broken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants