Skip to content

Beta to master#18122

Merged
seanbudd merged 6 commits into
masterfrom
beta
May 18, 2025
Merged

Beta to master#18122
seanbudd merged 6 commits into
masterfrom
beta

Conversation

@seanbudd
Copy link
Copy Markdown
Member

No description provided.

SaschaCowley and others added 6 commits May 16, 2025 10:35
Closes #18058

Summary of the issue:
The "Send Ctrl+Alt+Del" option in Remote Access often fails, as NVDA
does not have permission to call `SendSAS` on the controlled computer.

Description of user facing changes
The failure is reported to the user, rather than nothing happening.

Description of development approach
Refactor `LocalMachine.sendSAS` to rely on a helper function,
`_canSendSAS`, which attempts to detect all known cases in which calling
`SendSAS` will fail, and return `False` if any of these conditions hold,
as well as logging a specific reason why it returned `False`.

Testing strategy:
Configuration:
1. Enabled the `serviceDebug` system wide parameter.
2. Ticked "Require users to press Ctrl+Alt+Delete" in the Advanced tab
of `netplwiz`.

Test procedure:
1. Set SoftwareSASGeneration via the local group policy editor
(`gpedit.msc`, path Local Computer Policy | Computer Configuration |
Administrative Templates | Windows Components | Windows Logon Options |
Disable or enable software Secure Attention Sequence).
2. From the NVDA Python console,:
   ```py
   from _remoteClient import _remoteClient as rc
   rc.localMachine.sendSAS()
   ```
3. If step 2 fails, check that it wouldn't have succeeded if it weren't
for our checks:
   ```py
   import ctypes
   ctypes.windll.sas.SendSAS(1)  # If on a user desktop; or
   ctypes.windll.sas.SendSAS(0)  # If on the secure desktop
   ```
4. Sign out, and perform steps 2 and 3 from the sign in screen.

Situations tested:
* Running from source
	* User desktop: Fails as expected because no UI access
	* Secure desktop: Not tested
* Portable
	* User desktop: Fails as expected because no UI Access
	* Secure desktop: Not tested
* Installed
	* SoftwareSASGeneration not configured or disabled (not in registry)
		* User desktop:  Fails as expected because not on SD
		* Secure desktop: Succeeds as expected
	* SoftwareSASGeneration None
		* User desktop: Fails as expected because not allowed
		* Secure desktop: Fails as expected because not allowed
	* SoftwareSASGeneration Services
		* User desktop: Fails as expected because not allowed
		* Secure desktop: Fails as expected because not allowed
	* SoftwareSASGeneration UIAccess
		* User desktop: succeeds as expected
		* Secure desktop: succeeds as expected
	* SoftwareSASGeneration Both
		* User desktop: succeeds as expected
		* Secure desktop: succeeds as expected

Known issues with pull request:
1. The "Send ctrl+alt+delete" item in the "Remote Access" submenu is
enabled regardless of whether this is supported by the controlled
computer. It would be nice if the controlled computer communicated its
capabilities back to the controlling computer so we could disable the
menuitem when it won't work.
2. As the `SendSAS` function is governed by group policy, it is a lot to
expect users to understand how to change this on their device. It would
be nice to have a "Allow Remote Access to simulate ctrl+alt+delete"
button somewhere that would set the appropriate registry value for them.
3. This PR has only been tested on Windows 11 Business (and partially
tested on Windows 11 Pro). While I assume it will work fine on other
editions of Windows, I don't know how it will work on versions without
group policy. I assume the registry values are still exposed, so this
won't be an issue.
4. The `SendSAS`
documentation <https://learn.microsoft.com/en-us/windows/win32/api/sas/nf-sas-sendsas>
says "In addition, if an application is not running as a service, User
Account Control must be turned on to call SendSAS.". No testing is
currently being performed for this case. My understanding is that
completely disabling UAC is not supported in newer versions of Windows,
so I don't think testing of this case is needed.
Closes #18087 

Summary of the issue:
Remote Access currently has the option to choose between sounds and
beeps for audio cues. This is extra code that we have to maintain, and
is not an option that is present elsewhere in NVDA.

Description of user facing changes
The option has been removed. Remote Access will only play audio cues.

Description of development approach
1. In `gui.settingsDialogs.RemoteSettingsPanel`, removed the checkbox
and references thereto that controlled this setting.
2. In `_remoteClient.cues`, removed the code that playes beep sequences
from `_playCue`, including `shouldPlaySounds`, as this is now always
`True`. Removed each `beeps` field from the various cues, and edited the
type definition of `Cue` to no longer include this field.
3. In `tones`, removed `beepSequence`, `beepSequenceAsync`, and
associated datatypes and imports.
4. Remove the `playSounds` option from the config spec, and the rename
from the `upgradeConfigFrom_16_to_17` function.
5. Edit the user guide to no longer refer to the "Play sounds instead of
beeps" option.

Testing strategy:
1. Tested opening, closing, and saving the Remote Access settings to
ensure that they still work as expected.
2. Started a Remote Access session, and sent the clipboard to make sure
various cues still work as expected.
3. Unit tests.

Known issues with pull request:
None known
Summary of the issue:
Various mistakes in the change log.

 Description of user facing changes
* Removed the item related to `alt+f4` in CRFT (#18090), since the issue
is a regression of 2025.1 dev cycle.
* Removed the item related to the output from
`nvwave.outputDeviceNameToID`, and input to
`nvwave.outputDeviceIDToName` since these mapping have been removed
later in the same dev cycle.
* Reworded the items related to `WasapiWavePlayer` `__init__` method,
since `WasapiWavePlayer` has now been renamed to `WavePlayer`.

Also better wording of the the OCR related sentence in the intro
paragraph.

Description of development approach
N/A

Testing strategy:
Check generated change log.

Known issues with pull request:
None
@seanbudd seanbudd requested review from a team as code owners May 18, 2025 23:44
@seanbudd seanbudd merged commit d8b76d2 into master May 18, 2025
6 of 7 checks passed
@github-actions github-actions Bot added this to the 2025.2 milestone May 18, 2025
@AppVeyorBot
Copy link
Copy Markdown

  • Build execution time has reached the maximum allowed time for your plan (60 minutes).

See test results for failed build of commit bdf9e64c54

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants