fix screen reader mode issues

[meganrogge]

No description provided.

[Tyriar]

@alexdima would be a better review than me here

[meganrogge]

@deepak1556 do OSS and insider's use different versions of chromium or have different values for AutoDisableAccessibility?

OSS based on this branch works when I toggle VoiceOver but an Insider's build based on this branch does not.

Logging shows that when I toggle VoiceOver in OSS, the screen reader change is detected whereas in Insider's it is not.

I noticed in Chromium documentation that AutoDisableAccessibility is off by default. Could that be the issue?

fyi @isidorn

[alexdima]

@meganrogge I also looked at #167998 and this PR seems to also change areas changed there, but in a different way. Which version do you want to do?

[deepak1556]

Insiders and OSS use different Electron builds. OSS uses builds from https://github.com/electron/electron/releases/tag/v21.3.3 whereas product builds are from https://domoreexp.visualstudio.com/Teamspace/_git/electron-build which contain Chromium/Electron patches specific to VSCode.

For this specific case, AutoDisableAccessibility feature is off by default in Chromium but it is enabled for all builds of VSCode via runtime flag

vscode/src/main.js

Lines 242 to 245 in 1e4ac31

	/* Following features are enabled from the runtime.
	* `AutoDisableAccessibility` - https://github.com/microsoft/vscode/issues/162331#issue-1390744354
	*/
	app.commandLine.appendSwitch('enable-features', 'AutoDisableAccessibility');

In this iteration, we added a patch to product builds of Electron that rewires accessibility detection on all platforms. The regression with insiders seems highly likely to originate from that change, I did test the change on macOS ventura and voice over detection worked fine, @meganrogge what is your macOS version ?

[meganrogge]

@alexdima I separated the PRs because they attempt to fix different things -

#167998 fixes the case when you toggle the accessibility mode and it should impact all windows

This one attempts to fix an issue where toggling voice over (or other screen reader) does not cause our accessibility mode to update

[meganrogge]

@deepak1556 I am on macOS Ventura - is there a way to test that the switch is on? source maps aren't loading for me in insider's or i'd try to debug and hit a breakpoint to check

It works for me when I turn VoiceOver on, but not when I turn it off in insider's. I don't think that's a regression bc from my understanding, this detection was added last iteration

[meganrogge]

oss:

oss.mov

[meganrogge]

insider's:

insider.s.mov

[deepak1556]

is there a way to test that the switch is on?

This is not currently shipped with Insiders but you can do the following change and create a one-off insiders build,

• Replace the line with this.issueReporterWindow.loadURL('chrome://accessibility')
• Open the application and toggle the issue reporter window, you will now have the accessibility internals page from chromium that will display the various a11y states of a particular page.

I don't think that's a regression bc from my understanding, this detection was added last iteration

We did make voice over related detection changes last milestone but this iteration we changed how the a11y events are listened from the runtime and emitted to the application. Basically, Electron would trigger the event based on value change from the OS https://github.com/electron/electron/blob/909ee0ed6bbdf57ebaeda027b67a3a44185f6f7d/shell/browser/mac/electron_application.mm#L190 but the current change instead relies on Chromium to emit the event https://source.chromium.org/chromium/chromium/src/+/main:ui/accessibility/ax_mode_observer.h;l=17-18 . Chromium actually emits the event only after 2s since a11y client was turned off for reason mentioned in https://source.chromium.org/chromium/chromium/src/+/main:content/browser/accessibility/browser_accessibility_state_impl.cc;l=132-142, can you wait for that time period and check if that produces the off event.

[meganrogge]

It does not work when I wait even 10 seconds - I will try these steps next

This is not currently shipped with Insiders but you can do the following change and create a one-off insiders build,

Replace the line with this.issueReporterWindow.loadURL('chrome://accessibility')
Open the application and toggle the issue reporter window, you will now have the accessibility internals page from chromium that will display the various a11y states of a particular page.

[meganrogge]

@deepak1556 it's showing screen reader: disabled and the tree looks wrong while it looks correct in oss

[deepak1556]

Thanks for testing, confirms a regression from the patch. I will address it this week, will update here once new builds are available.

[deepak1556]

@meganrogge the issue in the runtime #167960 (comment) is now addressed with #170879. Do you want to pick up this PR again ? Thanks!

[meganrogge]

@deepak1556 thanks for the update - I just tested on macOS in the latest insider's and am not seeing it work when I turn voice over on or off. does it work for you?

[deepak1556]

Insiders with the above change has not been released yet, it will be available tomorrow PST :)

[deepak1556]

@meganrogge ping, latest insiders works fine with voiceover detection. Can you verify on your end, thanks!

[meganrogge]

@deepak1556 yes, it works for me now! thanks