Skip to content

Skip auto-loading PSReadLine on Windows if the NVDA screen reader is active #10385

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.

Sign up for GitHub

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

Merged
iSazonov merged 6 commits into from
Aug 23, 2019
Merged

Skip auto-loading PSReadLine on Windows if the NVDA screen reader is active #10385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d42db83
Skip auto-loading PSReadLine when the NVDA screen reader is on
daxian-dbw Aug 19, 2019
17b1c11
Update comments
daxian-dbw Aug 19, 2019
3ba280f
Print the warning
daxian-dbw Aug 20, 2019
1c16f62
Add test
daxian-dbw Aug 20, 2019
a1242f5
Fix minor issue
daxian-dbw Aug 20, 2019
1d068a6
Fix the test to not initialize on non-windows
daxian-dbw Aug 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 53 additions & 39 deletions src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
using System.Runtime.InteropServices;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.PowerShell.Telemetry;
using Microsoft.PowerShell.Commands;

using ConsoleHandle = Microsoft.Win32.SafeHandles.SafeFileHandle;
using Dbg = System.Management.Automation.Diagnostics;
Expand Down Expand Up @@ -55,6 +55,11 @@ internal sealed partial class ConsoleHost
internal const int ExitCodeCtrlBreak = 128 + 21; // SIGBREAK
internal const int ExitCodeInitFailure = 70; // Internal Software Error
internal const int ExitCodeBadCommandLineParameter = 64; // Command Line Usage Error
private const uint SPI_GETSCREENREADER = 0x0046;

[DllImport("user32.dll", SetLastError = true)]
Copy link
Collaborator

@iSazonov iSazonov Aug 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could remove SetLastError = true - we don't use it.

Copy link
Member Author

@daxian-dbw daxian-dbw Aug 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to keep it, even it's not used in this case. People copy code, and this is a good practice for PInvoke.

[return: MarshalAs(UnmanagedType.Bool)]
private static extern bool SystemParametersInfo(uint uiAction, uint uiParam, ref bool pvParam, uint fWinIni);

// NTRAID#Windows Out Of Band Releases-915506-2005/09/09
// Removed HandleUnexpectedExceptions infrastructure
Expand Down Expand Up @@ -158,7 +163,7 @@ internal static int Start(
}

s_cpp = new CommandLineParameterParser(
(s_theConsoleHost != null) ? s_theConsoleHost.UI : (new NullHostUserInterface()),
(s_theConsoleHost != null) ? s_theConsoleHost.UI : new NullHostUserInterface(),
bannerText, helpText);

s_cpp.Parse(args);
Expand Down Expand Up @@ -582,15 +587,15 @@ public void PushRunspace(Runspace newRunspace)
}

// Connect a disconnected command.
this.runningCmd = Microsoft.PowerShell.Commands.EnterPSSessionCommand.ConnectRunningPipeline(remoteRunspace);
this.runningCmd = EnterPSSessionCommand.ConnectRunningPipeline(remoteRunspace);

// Push runspace.
_runspaceRef.Override(remoteRunspace, hostGlobalLock, out _isRunspacePushed);
RunspacePushed.SafeInvoke(this, EventArgs.Empty);

if (this.runningCmd != null)
{
Microsoft.PowerShell.Commands.EnterPSSessionCommand.ContinueCommand(
EnterPSSessionCommand.ContinueCommand(
remoteRunspace,
this.runningCmd,
this,
Expand Down Expand Up @@ -858,7 +863,6 @@ public override PSObject PrivateData
/// </summary>
/// <value></value>
/// <exception/>

public override System.Globalization.CultureInfo CurrentCulture
{
get
Expand All @@ -875,7 +879,6 @@ public override System.Globalization.CultureInfo CurrentCulture
/// </summary>
/// <value></value>
/// <exception/>

public override System.Globalization.CultureInfo CurrentUICulture
{
get
Expand All @@ -890,7 +893,6 @@ public override System.Globalization.CultureInfo CurrentUICulture
/// <summary>
/// </summary>
/// <exception/>

public override void SetShouldExit(int exitCode)
{
lock (hostGlobalLock)
Expand Down Expand Up @@ -1320,7 +1322,6 @@ internal TextWriter ConsoleTextWriter
/// <returns>
/// The process exit code to be returned by Main.
/// </returns>

private uint Run(CommandLineParameterParser cpp, bool isPrestartWarned)
{
Dbg.Assert(cpp != null, "CommandLine parameter parser cannot be null.");
Expand Down Expand Up @@ -1382,23 +1383,6 @@ private uint Run(CommandLineParameterParser cpp, bool isPrestartWarned)
return exitCode;
}

/// <summary>
/// This method is retained to make V1 tests compatible with V2 as signature of this method
/// is slightly changed in v2.
/// </summary>
/// <param name="bannerText"></param>
/// <param name="helpText"></param>
/// <param name="isPrestartWarned"></param>
/// <param name="args"></param>
/// <returns></returns>
[SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")]
private uint Run(string bannerText, string helpText, bool isPrestartWarned, string[] args)
{
s_cpp = new CommandLineParameterParser(this.UI, bannerText, helpText);
s_cpp.Parse(args);
return Run(s_cpp, isPrestartWarned);
}

/// <summary>
/// Loops over the Host's sole Runspace; opens the runspace, initializes it, then recycles it if the Runspace fails.
/// </summary>
Expand Down Expand Up @@ -1504,12 +1488,32 @@ private void CreateRunspace(object runspaceCreationArgs)
}

/// <summary>
/// This method is here only to make V1 tests compatible with V2. DO NOT USE THIS FUNCTION! Use DoCreateRunspace instead.
/// Check if a screen reviewer utility is running.
/// When a screen reader is running, we don't auto-load the PSReadLine module at startup,
/// since PSReadLine is not accessibility-firendly enough as of today.
/// </summary>
[SuppressMessage("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")]
private void InitializeRunspace(string initialCommand, bool skipProfiles, Collection<CommandParameter> initialCommandArgs)
private bool IsScreenReaderActive()
{
DoCreateRunspace(initialCommand, skipProfiles, staMode: false, configurationName: null, initialCommandArgs: initialCommandArgs);
if (_screenReaderActive.HasValue)
{
return _screenReaderActive.Value;
}

_screenReaderActive = false;
if (Platform.IsWindowsDesktop)
{
// Note: this API can detect if a third-party screen reader is active, such as NVDA, but not the in-box Windows Narrator.
// Quoted from https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-systemparametersinfoa about the
// accessibility parameter 'SPI_GETSCREENREADER':
// "Narrator, the screen reader that is included with Windows, does not set the SPI_SETSCREENREADER or SPI_GETSCREENREADER flags."
bool enabled = false;
if (SystemParametersInfo(SPI_GETSCREENREADER, 0, ref enabled, 0))
{
_screenReaderActive = enabled;
}
}

return _screenReaderActive.Value;
}

private bool LoadPSReadline()
Expand Down Expand Up @@ -1542,28 +1546,37 @@ private void DoCreateRunspace(string initialCommand, bool skipProfiles, bool sta
bool psReadlineFailed = false;

// Load PSReadline by default unless there is no use:
// - screen reader is active, such as NVDA, indicating non-visual access
// - we're running a command/file and just exiting
// - stdin is redirected by a parent process
// - we're not interactive
// - we're explicitly reading from stdin (the '-' argument)
// It's also important to have a scenario where PSReadline is not loaded so it can be updated, e.g.
// powershell -command "Update-Module PSReadline"
// This should work just fine as long as no other instances of PowerShell are running.
ReadOnlyCollection<Microsoft.PowerShell.Commands.ModuleSpecification> defaultImportModulesList = null;
ReadOnlyCollection<ModuleSpecification> defaultImportModulesList = null;
if (LoadPSReadline())
{
// Create and open Runspace with PSReadline.
defaultImportModulesList = DefaultInitialSessionState.Modules;
DefaultInitialSessionState.ImportPSModule(new[] { "PSReadLine" });
consoleRunspace = RunspaceFactory.CreateRunspace(this, DefaultInitialSessionState);
try
if (IsScreenReaderActive())
{
OpenConsoleRunspace(consoleRunspace, staMode);
s_theConsoleHost.UI.WriteLine(ManagedEntranceStrings.PSReadLineDisabledWhenScreenReaderIsActive);
s_theConsoleHost.UI.WriteLine();
}
catch (Exception)
else
{
consoleRunspace = null;
psReadlineFailed = true;
// Create and open Runspace with PSReadline.
defaultImportModulesList = DefaultInitialSessionState.Modules;
DefaultInitialSessionState.ImportPSModule(new[] { "PSReadLine" });
consoleRunspace = RunspaceFactory.CreateRunspace(this, DefaultInitialSessionState);
try
{
OpenConsoleRunspace(consoleRunspace, staMode);
}
catch (Exception)
{
consoleRunspace = null;
psReadlineFailed = true;
}
}
}

Expand Down Expand Up @@ -2834,6 +2847,7 @@ private class ConsoleHostStartupException : Exception
private bool _setShouldExitCalled;
private bool _isRunningPromptLoop;
private bool _wasInitialCommandEncoded;
private bool? _screenReaderActive;

// hostGlobalLock is used to sync public method calls (in case multiple threads call into the host) and access to
// state that persists across method calls, like progress data. It's internal because the ui object also
Expand Down
3 changes: 3 additions & 0 deletions src/Microsoft.PowerShell.ConsoleHost/resources/ManagedEntranceStrings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ Copyright (c) Microsoft Corporation. All rights reserved.
https://aka.ms/powershell
Type 'help' to get help.</value>
</data>
<data name="PSReadLineDisabledWhenScreenReaderIsActive" xml:space="preserve">
<value>Warning: PowerShell detected that you might be using a screen reader and has disabled PSReadLine for compatibility purposes. If you want to re-enable it, run 'Import-Module PSReadLine'.</value>
</data>
<data name="UsageHelp" xml:space="preserve">
<value>Usage: pwsh[.exe] [-Login] [[-File] &lt;filePath&gt; [args]]
[-Command { - | &lt;script-block&gt; [-args &lt;arg-array&gt;]
Expand Down
54 changes: 54 additions & 0 deletions test/powershell/Host/ScreenReader.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT License.

Describe "Validate start of console host" -Tag CI {
BeforeAll {
if (-not $IsWindows) {
return
}

$csharp_source = @'
using System;
using System.Runtime.InteropServices;

public class ScreenReaderTestUtility {
private const uint SPI_SETSCREENREADER = 0x0047;

[DllImport("user32.dll", SetLastError = true)]
[return: MarshalAs(UnmanagedType.Bool)]
private static extern bool SystemParametersInfo(uint uiAction, uint uiParam, IntPtr pvParam, uint fWinIni);

public static bool ActivateScreenReader() {
return SystemParametersInfo(SPI_SETSCREENREADER, 1u, IntPtr.Zero, 0);
}

public static bool DeactivateScreenReader() {
return SystemParametersInfo(SPI_SETSCREENREADER, 0u, IntPtr.Zero, 0);
}
}
'@
$utilType = "ScreenReaderTestUtility" -as [type]
if (-not $utilType) {
$utilType = Add-Type -TypeDefinition $csharp_source -PassThru
}

## Make the screen reader status active.
$utilType::ActivateScreenReader()
}

AfterAll {
if ($IsWindows) {
## Make the screen reader status in-active.
$utilType::DeactivateScreenReader()
}
}

It "PSReadLine should not be auto-loaded when screen reader status is active" -Skip:(-not $IsWindows) {
$output = pwsh -noprofile -noexit -c "Get-Module PSReadLine; exit"
$output.Length | Should -BeExactly 2

## The warning message about screen reader should be returned, but the PSReadLine module should not be loaded.
$output[0] | Should -BeLike "Warning:*'Import-Module PSReadLine'."
$output[1] | Should -BeExactly ([string]::Empty)
}
}