Skip to content

beta to master#19481

Merged
seanbudd merged 2 commits intomasterfrom
beta
Jan 21, 2026
Merged

beta to master#19481
seanbudd merged 2 commits intomasterfrom
beta

Conversation

@seanbudd
Copy link
Copy Markdown
Member

No description provided.

cary-rowen and others added 2 commits January 21, 2026 16:20
…19442)

Summary of the issue:

In NVDA Remote Access, if a user is in "Follower" mode and the "Confirm disconnection when controlled" option is enabled, triggering the "Disconnect" action multiple times in rapid succession (e.g. by clicking the menu item repeatedly) results in multiple confirmation dialogs stacking on top of each other.
Description of user facing changes:

When attempting to disconnect from a Remote Access session, if a confirmation dialog is already open, NVDA will now focus the existing dialog instead of opening a new one.
Description of developer facing changes:

None.
Description of development approach:

The RemoteClient now maintains a reference to the active disconnection confirmation dialog in self._disconnectConfirmationDialog.
In doDisconnect, if this reference is set, the existing dialog is brought to the foreground. Otherwise, a new dialog is created, stored in the reference, and cleared in a finally block after it closes. This prevents re-entrancy while targeting only the specific dialog instance.
Replaces #19454
Replaces #19467
Fixes #19103
Summary of the issue:

Users are not notified when connecting as the controlled computer (follower) fails. Only the leader connection errors are shown to users.
Description of user facing changes:

Users will now see an error message when connection as the controlled computer fails, just like when connecting as the controlling computer.
Description of developer facing changes:

None.
Description of development approach:

Made follower connection error handling consistent with leader connection by adding the missing event handler and user notification.

    Added onConnectAsFollowerFailed method in client.py to handle follower connection failures;
    Registered transportConnectionFailed event handler for follower transport;
    Added cleanup for event handlers in disconnectAsFollower method.
@seanbudd seanbudd requested a review from a team as a code owner January 21, 2026 05:49
@seanbudd seanbudd merged commit 890d2d6 into master Jan 21, 2026
14 of 16 checks passed
@github-actions github-actions Bot added this to the 2026.2 milestone Jan 21, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request merges changes from the beta branch to master, focusing on improvements to the Remote Access functionality in NVDA. The changes enhance user notifications when connection attempts fail and prevent multiple disconnection confirmation dialogs from appearing.

Changes:

  • Added user notifications when connecting as the controlled computer fails
  • Fixed an issue where multiple disconnection confirmation dialogs could be opened if triggered repeatedly
  • Updated copyright year to 2026

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
user_docs/en/changes.md Documents the two bug fixes related to Remote Access notifications and dialog handling
source/_remoteClient/client.py Implements the connection failure notification handler and dialog tracking mechanism, plus copyright year update

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +333 to +335
caption=pgettext("remote", "Error Connecting"),
# Translators: Message shown when unable to connect to the remote computer.
message=pgettext("remote", "Unable to connect to the remote computer"),
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

Inconsistent use of translation functions. The onConnectAsLeaderFailed method uses _() for both caption and message (lines 318-320), while this new onConnectAsFollowerFailed method uses pgettext("remote", ...). These should be consistent. Consider using the same translation function approach as onConnectAsLeaderFailed to maintain consistency within the same file and similar functionality.

Suggested change
caption=pgettext("remote", "Error Connecting"),
# Translators: Message shown when unable to connect to the remote computer.
message=pgettext("remote", "Unable to connect to the remote computer"),
caption=_("Error Connecting"),
# Translators: Message shown when unable to connect to the remote computer.
message=_("Unable to connect to the remote computer"),

Copilot uses AI. Check for mistakes.
Comment on lines +298 to +299
if self.followerTransport:
self.followerTransport.transportConnectionFailed.unregister(self.onConnectAsFollowerFailed)
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

Unregistering an event handler from within its own callback execution may cause issues depending on the event system implementation. The handler is being unregistered at line 299 within disconnectAsFollower(), which is called from within onConnectAsFollowerFailed itself (line 328). Note that disconnectAsLeader() does not perform a similar unregistration for its connection failed handler. Consider reviewing whether this unregistration is necessary here, or if the transport cleanup handles it automatically. If this unregistration is intentional and safe, consider adding it to disconnectAsLeader() for consistency.

Suggested change
if self.followerTransport:
self.followerTransport.transportConnectionFailed.unregister(self.onConnectAsFollowerFailed)

Copilot uses AI. Check for mistakes.
@github-actions github-actions Bot requested a deployment to snapshot January 21, 2026 06:42 Abandoned
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.

4 participants