Skip to content

Fix screenCurtain/Fullscreen conflict#20342

Merged
seanbudd merged 9 commits into
nvaccess:betafrom
France-Travail:fix/screenCurtainConflict
Jun 30, 2026
Merged

Fix screenCurtain/Fullscreen conflict#20342
seanbudd merged 9 commits into
nvaccess:betafrom
France-Travail:fix/screenCurtainConflict

Conversation

@Boumtchack

@Boumtchack Boumtchack commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #20289

Summary of the issue:

Two bugs occur when both the fullscreen magnifier and screen curtain are enabled and NVDA is restarted:

(Privacy/critical) The magnifier's initialization calls MagUninitialize() on the screen curtain's active API session, briefly exposing the screen.
After restart, disabling the screen curtain then enabling the magnifier fails on the first attempt due to stale Windows API state left by the screen curtain's MagSetFullscreenColorEffect(TRANSFORM_BLACK) call.

Description of user facing changes:

On restart, the screen curtain remains active during magnifier initialization, the screen is no longer exposed.
Enabling the screen curtain while the magnifier is active stops the magnifier (with a spoken message) and re-enables it automatically when the screen curtain is disabled.
Trying to enable the magnifier while the screen curtain is active speaks an error message.

Description of developer facing changes:

Magnifier (base class) gains _isBlockedByScreenCurtain(), onScreenCurtainEnabled(), and onScreenCurtainDisabled(), replacing the former inline screen curtain check in _startMagnifier().
FullScreenMagnifier gains _clearStaleApiState() and a guard if not self._isActive: return after super()._startMagnifier() to prevent API initialization when the base class blocked the start.
screenCurtain already called onScreenCurtainEnabled/Disabled — no change needed there.

Description of development approach:

Bug 1 is fixed by checking the screen curtain in Magnifier._startMagnifier() via _isBlockedByScreenCurtain() and returning without setting _isActive = True. The FullScreenMagnifier override detects this and skips _initializeNativeMagnification(), so the screen curtain's API session is never touched.

Bug 2 is a Windows API quirk: after MagSetFullscreenColorEffect(TRANSFORM_BLACK) + MagUninitialize(), the next MagSetFullscreenTransform silently fails. _clearStaleApiState() runs a dummy MagInitialize / MagSetFullscreenColorEffect(NORMAL) / MagUninitialize cycle before every real MagInitialize to clear this state.

Screen curtain interaction logic lives in the base Magnifier class so all future magnifier types inherit it. _clearStaleApiState() stays in FullScreenMagnifier as it is specific to the Windows Magnification API.

Testing strategy:

manual/unit

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.

@Boumtchack Boumtchack marked this pull request as ready for review June 15, 2026 15:17
@Boumtchack Boumtchack requested a review from a team as a code owner June 15, 2026 15:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@SaschaCowley SaschaCowley added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Jun 16, 2026
Comment thread source/_magnifier/fullscreenMagnifier.py Outdated
Comment thread source/_magnifier/fullscreenMagnifier.py Outdated
@seanbudd seanbudd marked this pull request as draft June 22, 2026 04:54
@Boumtchack Boumtchack marked this pull request as ready for review June 22, 2026 11:51
@seanbudd seanbudd requested a review from Copilot June 23, 2026 02:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread source/_magnifier/fullscreenMagnifier.py Outdated

@seanbudd seanbudd left a comment

Copy link
Copy Markdown
Member

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
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@seanbudd seanbudd marked this pull request as draft June 24, 2026 04:14
@Boumtchack Boumtchack marked this pull request as ready for review June 29, 2026 15:02
@seanbudd

Copy link
Copy Markdown
Member

Thanks @Boumtchack

@seanbudd seanbudd added this to the 2026.2 milestone Jun 30, 2026
@seanbudd seanbudd merged commit 8b2f5ae into nvaccess:beta Jun 30, 2026
34 of 39 checks passed
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.

4 participants