Skip to content

handle conflicts between Windows magnifier and NVDA magnifier#20025

Merged
seanbudd merged 3 commits intonvaccess:masterfrom
France-Travail:fix/WindowsNVDAConflicts
Apr 29, 2026
Merged

handle conflicts between Windows magnifier and NVDA magnifier#20025
seanbudd merged 3 commits intonvaccess:masterfrom
France-Travail:fix/WindowsNVDAConflicts

Conversation

@Boumtchack
Copy link
Copy Markdown
Contributor

@Boumtchack Boumtchack commented Apr 27, 2026

Link to issue number:

fix #20012

Summary of the issue:

When Windows Magnifier was already running, NVDA Magnifier could still start because MagInitialize() succeeds even when the API is held by another process. Only MagSetFullscreenTransform() would fail, triggering the recovery loop — which itself only tested MagInitialize() and MagSetFullscreenColorEffect(), both of which succeed, causing it to falsely declare victory and restart the timer indefinitely.

Description of user facing changes:

NVDA now shows a clear message ("Cannot start magnifier: the magnification API is unavailable. Windows Magnifier may already be running.") when the magnification API cannot be fully initialized at startup. If the conflict occurs after startup, the recovery mechanism is now capped at 3 attempts before stopping the magnifier and notifying the user, preventing log spam.

Description of developer facing changes:

_initializeNativeMagnification() no longer uses @trackNativeMagnifierErrors — it now raises OSError on failure (including a MagSetFullscreenTransform probe that detects the Windows Magnifier conflict). _startMagnifier() catches this error, calls _stopMagnifier() to properly unregister the display change handler, and returns early. _attemptRecovery() reuses _initializeNativeMagnification() to avoid duplicating the probe logic, and is now capped by _MAX_RECOVERY_ATTEMPTS = 3.

Description of development approach:

The fix centers on making _initializeNativeMagnification() a reliable gate: it probes MagSetFullscreenTransform (the actual failing call) as part of initialization and raises on failure rather than silently returning. This single change fixes both the startup path and the recovery path, since _attemptRecovery() can delegate to the same method. A _recoveryAttempts counter (capped at _MAX_RECOVERY_ATTEMPTS) prevents the infinite restart loop when the API remains permanently unavailable.

Testing strategy:

unit test

Known issues with pull request:

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@seanbudd seanbudd changed the title resolves conflicts handle conflicts between Windows magnifier and NVDA magnifier Apr 28, 2026
@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Apr 28, 2026
@Boumtchack Boumtchack marked this pull request as ready for review April 28, 2026 13:12
@Boumtchack Boumtchack requested a review from a team as a code owner April 28, 2026 13:12
@Boumtchack Boumtchack requested review from Copilot and seanbudd and removed request for Copilot April 28, 2026 13:12
@CyrilleB79
Copy link
Copy Markdown
Contributor

@Boumtchack the translators comment check test fails. This one does not fail randomly (as other sometimes do), so it needs to be fixed.

@CyrilleB79
Copy link
Copy Markdown
Contributor

My bad, the translators comments check test is failing due to cache miss. Unrelated to this PR.

@Boumtchack
Copy link
Copy Markdown
Contributor Author

that's weird, I shouldn't be abble to push if my pre-commit fail, I'll look into it

Copy link
Copy Markdown
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

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

Thanks @Boumtchack

Comment thread source/_magnifier/fullscreenMagnifier.py Outdated
Comment thread source/_magnifier/fullscreenMagnifier.py Outdated
@seanbudd seanbudd requested a review from Copilot April 29, 2026 02:53
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
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

Addresses conflicts between Windows Magnifier and NVDA’s full-screen magnifier by making Magnification API initialization/recovery reliably detect unusable API states and preventing runaway recovery loops/log spam.

Changes:

  • Make _initializeNativeMagnification() validate API usability (including a MagSetFullscreenTransform probe) and raise on failure, with _startMagnifier() handling user notification + cleanup.
  • Add recovery attempt tracking and reset logic to avoid repeated error loops.
  • Extend unit tests to cover API-conflict startup behavior and recovery scenarios.

Reviewed changes

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

File Description
source/_magnifier/fullscreenMagnifier.py Adds initialization failure handling, API usability probing, and capped recovery attempts.
source/_magnifier/magnifier.py Introduces _recoveryAttempts state and resets it on successful update cycles.
tests/unit/test_magnifier/test_fullscreenMagnifier.py Adds tests for API conflict detection and updated recovery expectations.

Comment thread source/_magnifier/fullscreenMagnifier.py Outdated
Comment thread source/_magnifier/fullscreenMagnifier.py Outdated
Comment thread source/_magnifier/fullscreenMagnifier.py Outdated
Comment thread source/_magnifier/fullscreenMagnifier.py Outdated
@seanbudd seanbudd merged commit 7f2d68f into nvaccess:master Apr 29, 2026
34 of 37 checks passed
@github-actions github-actions Bot added this to the 2026.2 milestone Apr 29, 2026
@Boumtchack Boumtchack deleted the fix/WindowsNVDAConflicts branch May 6, 2026 07:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When Windows and NVDA magnifiers are both started, errors are logged and NVDA Magnifier commands report incorrect information

4 participants