Accessibility improvements for finding in files

[Neurrone]

Issue Type: Bug

CC: @isidorn

Hi,

Some parts of the workflow for searching within a file with ctrl+f aren't that accessible. As a demonstration, have a blank editor with the following contents:

hello world
goodbye world

From the top of the file, when pressing ctrl+f and typing world, visually the first occurance is highlighted, but no screen reader feedback is provided at all. So my instinct is to press enter, which of course brings me to the second match instead.
Next when on the second line, perform the same search. By default, it wraps you back to the beginning of the file if no more matches are found, which is doubly confusing due to the lack of feedback.

I expect to hear something similar to how its announced in the results view in project-wide search, but less verbose. E.g, "1/2: Found 'world' at 9:20 with text: "

Similar context should also be provided when moving using f3 and shift+f3, but haven't figured out what the best messages would be.

On a related note, I'm also pondering whether disabling such wrap-around by default if a screen reader is detected would be better. It is obvious that this has happened visually, but unless enough context is provided, it can be confusing.

VS Code version: Code - Insiders 1.32.0-insider (804373a, 2019-03-01T18:22:35.336Z)
OS version: Windows_NT x64 10.0.17134

System Info

Item	Value
CPUs	Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz (8 x 1992)
GPU Status	2d_canvas: enabled checker_imaging: disabled_off flash_3d: enabled flash_stage3d: enabled flash_stage3d_baseline: enabled gpu_compositing: enabled multiple_raster_threads: enabled_on native_gpu_memory_buffers: disabled_software rasterization: enabled surface_synchronization: enabled_on video_decode: enabled webgl: enabled webgl2: enabled
Memory (System)	15.86GB (8.89GB free)
Process Argv
Screen Reader	yes
VM	36%

Extensions (7)

Extension	Author (truncated)	Version
vscode-markdownlint	Dav	0.25.0
vscode-eslint	dba	1.8.0
python	ms-	2019.2.5433
cpptools	ms-	0.21.0
Go	ms-	0.9.2
rust	rus	0.5.3
markdown-preview-enhanced	shd	0.3.11

[rebornix]

I expect to hear something similar to how its announced in the results view in project-wide search, but less verbose. E.g, "1/2: Found 'world' at 9:20 with text: "

this one is a bug we should fix.

I'm also pondering whether disabling such wrap-around by default if a screen reader is detected would be better

IMO We need to provide more hints for screen reader when we reach the last item in the document, and users can be aware that the next search will go back to the top of the document. To allow users disable this feature, we have a tracking item #28666 , we can introduce a setting if it doesn't make sense to some users.

[Neurrone]

@rebornix I think that having better alerts when focus moves when moving to the next/previous result would be sufficient; if I was at 3/3 and suddenly it says 1/3, then I know its moved.

Another possibility is to play the system bell sound every time focus is wrapped back to the top.

Regarding the proposed format of the alert, I think it can be condensed to "1/2: Found 'world' at line 9:20: ".

[isidorn]

@rebornix can you maybe provide a code pointer for this. So we could add a help-wanted, or I might look into this if I find time.
Thanks!

[rebornix]

@isidorn thanks for the ping. The code change is intuitive by leveraging aria.status.

Firstly, we listen to find results change in https://github.com/microsoft/vscode/blob/master/src/vs/editor/contrib/find/findWidget.ts#L324 and update labels when search results count change. So we can alert the status at the same position.

Secondly, for the message format, we can follow Search Viewlet ones but less verbose as suggested by @Neurrone .

vscode/src/vs/workbench/contrib/search/browser/searchView.ts

Lines 1319 to 1335 in 579966f

	const hasExcludes = !!excludePatternText;
	const hasIncludes = !!includePatternText;
	let message: string;
	if (!completed) {
	message = nls.localize('searchCanceled', "Search was canceled before any results could be found - ");
	} else if (hasIncludes && hasExcludes) {
	message = nls.localize('noResultsIncludesExcludes', "No results found in '{0}' excluding '{1}' - ", includePatternText, excludePatternText);
	} else if (hasIncludes) {
	message = nls.localize('noResultsIncludes', "No results found in '{0}' - ", includePatternText);
	} else if (hasExcludes) {
	message = nls.localize('noResultsExcludes', "No results found excluding '{0}' - ", excludePatternText);
	} else {
	message = nls.localize('noResultsFound', "No results found. Review your settings for configured exclusions and check your gitignore files - ");
	}
	// Indicate as status to ARIA

[isidorn]

@rebornix Great. Thanks for the pointers. Let's see if some user jumps on this.
If not, I can take a look in the future.

[gbmeow]

@rebornix @isidorn I will have a look at this -

I expect to hear something similar to how its announced in the results view in project-wide search, but less verbose. E.g, "1/2: Found 'world' at 9:20 with text: "

I tested with NVDA 2019.1.1 - and the above issue is reproducible

[isidorn]

@georgebatalinski thanks for being interested. Feel free to provide a PR. @rebornix provided nice code pointers.

[gbmeow]

@isidorn this is my first guess at the issue - let me know your thoughts

1. Re-using the existing label document.createTextNode(label) - would be the most beneficial

• as hiding the text from the screen - has not worked for me - under matchesCount with NVDA

.sr-only {
  position: absolute;
  left: -10000px;
  width: 1px;
  height: 1px;
  overflow: hidden;
}

2. role="status" - is not consistent, hence I set it for 'role', 'alert'

#77031

[isidorn]

@georgebatalinski thanks! I will comment in the PR directly.

[rebornix]

The PR is already merged.

[Neurrone]

@georgebatalinski this is such a massive quality of life improvement and does exactly what I would expect, thanks a lot!