Make Screen Curtain profile independent by SaschaCowley · Pull Request #19177 · nvaccess/nvda

SaschaCowley · 2025-11-06T02:19:57Z

Link to issue number:

Fixes #10476
Partially addresses #16272

Summary of the issue:

Currently, the Screen Curtain is configuration profile dependent. This can lead to situations in which it is unexpectedly deactivated by a config profile change.

Description of user facing changes:

The Screen Curtain is no-longer config profile dependent.
Screen Curtain settings have been moved to anew "Privacy and Security" settings category.

Description of developer facing changes:

The visionEnhancementProviders.screenCurtain module has bee removed.

Description of development approach:

Added a new screenCurtain subpackage, and migrated the existing visionEnhancementProviders.screenCurtain implementation.
- The global object pattern was used, as it is a common pattern throughout the rest of NVDA's code.
- The core Screen Curtain code is in screenCurtain._screenCurtain. The public API is accessible from screenCurtan, including the global screenCurtain.screenCurtain object.
Updated core.main and core.resetConfiguration to initializeand terminate screen curtain.
Updated the screen curtain toggle script to use the new implementation.
Updated the OCR script and screen curtain block action.
Re-implemented the screen curtain settings as regular settings in a new "Privacy and Security" panl in gui.settingsDialogs.
- I decided to call this category "Privacy and Security" rather than "Security and Privacy" (as suggested in Add a new settings category called "security and privacy" in NVDA settings panel #16272), as we already have a "Speech" category, but no categories beginnin wit 'P'.
- Also added an unassigned command to jump directly to the privacy and security settings.
Completely removed the visionEnhancementProviders.screenCurtain module, and updated the change log to reflect that previously deprecated symbols that had redirects in this module have been removed.
Bumped the config schema version, and added a profile upgrade step that moves the settings out of vision and into the root section. As the config keys are otherwise identical, this is all that is necessary.
Updated changed strings to use pgettext (category "screenCurtain")

Testing strategy:

Running from source:

Enabled tempory screen curtain with the gesture, with and without the warning enabled. Disabled.
Enabled screen curtain with the gesture, with and without the dialog enabled. Disabled.
Checked and uncheckd the screen curtain option in Privacy and Security settings, with and without the warning enabled. Also checked that unchecking the option to show a warning was reflected in the settings dialog when the warning dialog was shown from NVDA settings.
Enabled and disabled Screen Curtain with sounds turned on and off.
Created a configuration profile. Enabled screen curtain. Saved settings. Activated the profile. Deactivated screen curtain. Saved settings. Switched back to normal config.
Reset to saved configuration after enabling and disabling screen curtain, and reset configuration to factory defaults.
Ran from source on master, and changed screen curtain settings. Ran from source on this branch and ensured the old settings were reflected. Saved the config to disk and ensured that the settings were correctly persisted to nvda.ini.

Known issues with pull request:

None

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.

cary-rowen · 2025-11-06T03:15:43Z

#10476 is too long for me to finish reading, but I just want to confirm one thing: will this change cause the screen curtain to not follow the process-name-based profile? If so, that would be terrible. I have a very important use case: I created separate profiles for my instant messaging applications, such as "WeChat," because I don't want these applications to be displayed in specific scenarios, while for other applications I don't think it matters much, so I don't want to enable the screen curtain.

SaschaCowley · 2025-11-06T06:35:45Z

@cary-rowen

will this change cause the screen curtain to not follow the process-name-based profile?

That is correct. The screen curtain will apply to all profiles.

If so, that would be terrible. I have a very important use case: I created separate profiles for my instant messaging applications, such as "WeChat," because I don't want these applications to be displayed in specific scenarios, while for other applications I don't think it matters much, so I don't want to enable the screen curtain.

Unfortunately, as things stand there is no solution that will please everyone. We have made the decision to make the screen curtain profile independent as we feel that this provides the best experience for users in terms of privacy. Perhaps as NVDA's configuration profile support improves this will be a use-case that can be added back.

CyrilleB79 · 2025-11-06T07:14:35Z

If so, that would be terrible. I have a very important use case: I created separate profiles for my instant messaging applications, such as "WeChat," because I don't want these applications to be displayed in specific scenarios, while for other applications I don't think it matters much, so I don't want to enable the screen curtain.

Unfortunately, as things stand there is no solution that will please everyone. We have made the decision to make the screen curtain profile independent as we feel that this provides the best experience for users in terms of privacy. Perhaps as NVDA's configuration profile support improves this will be a use-case that can be added back.

It's not even a matter of pleasing everyone, or satisfying different use cases.
It's just that a profile dependant, so an app-dependant screen curtain gives a false sense of privacy but cannot guarantee it actually.

The privacy issue is as follows:
Let's imagine you have the base profile without the screen curtain off and an IM app profile, e.g. WeChat profile, with screen curtain on. If you are in WeChat and WeChat window is big enough and you open another app with a smaller window, parts of the bigger window will be visible in the background since the smaller window will not cover the whole IM window in the background. I guess you can reproduce by maximizing WeChat window and opening the "Run" window with windows+R.

CyrilleB79 · 2025-11-07T15:39:45Z

@SaschaCowley wrote:

The visionEnhancementProviders.screenCurtain module is essentially just a stub now. If we are happy breaking the few redirects in that module, we can get rid of it, and the code excluding it when loading VEPs from the file system.

In the few add-ons that seem to use screen curtain API, most seem to be just testing if screen curtain is active or not to perform OCR, recognition, etc. with the following code (or similar):

import vision
from visionEnhancementProviders.screenCurtain import ScreenCurtainProvider
screenCurtainId = ScreenCurtainProvider.getSettings().getId()
screenCurtainProviderInfo = vision.handler.getProviderInfo(screenCurtainId)
isScreenCurtainActive = bool(vision.handler.getProviderInstance(screenCurtainProviderInfo))

Thus, I do not expect them to have difficulty to adapt to the new code if necessary.

Since 2026.1 will already be an important API-breaking release, I'd be in favor to take the opportunity to simplify NVDA's code here and remove the stub, rather than keeping this deprecated stub for only few add-ons.

CyrilleB79 · 2025-11-13T07:05:57Z

@seanbudd, @SaschaCowley this PR replaces all existing use of _ with pgettext. Using pgettext makes sense for new developments:

it allows disambiguation in case two identical translatable strings are used in two places in the code
and it give an additional hint for translators, in addition to the translators comments, to understand in which context this string applies

But for existing translatable strings that have already been translated for 5 years and that do not need disambiguation, there is no added value to switch to pgettext. Worse (and not least), it forces transltors to re-translate again all the screen curtain strings.

Would you mind consider switching back to _ usage in this PR for existing strings?
And in future PR avoid to convert _ to pgettext without any good reason in existing strings?
Thanks.

CyrilleB79 · 2025-11-13T08:14:40Z

Sorry, I had not seen that my previous comment regarding pgettext usage, and its answers, had been collapsed; I don't know why. Hence this duplicate message.

Co-authored-by: Sean Budd <sean@nvaccess.org>

CyrilleB79

Nice! All good for me.

seanbudd

Thanks @SaschaCowley - happy with this too. I have some suggestions, but I'm not sure how appropriate they are

I think because we have called the feature "Screen Curtain" (and it's capitalised like feature names we've used) - but the words themselves are common and could simply be written in a sentence which would make sense even if we didn't have such a feature (and in that case you would use "the"). If we change this, we should probably change other references to match. Co-authored-by: Sean Budd <sean@nvaccess.org>

Qchristensen

Reads well. Following Sean's suggestion about dropping "the" from "the Screen Curtain", I've changed other references in the user guide (I note there other uses of "the Screen Curtain" in the code comments but haven't changed those)

Co-authored-by: Quentin Christensen <quentin@nvaccess.org>

Fixes #19263 Closes #10545 Possible fix for #18693 and #18743 ### Summary of the issue: Since #19177, if Screen Curtain fails to initialise because `isScreenFullyBlack` returns `False`, NVDA crashes (#19263). It would appear that from time to time, bitblitting from the desktop context to the capture context in `nvdaHelper/local/screenCurtain.cpp` failes due to one or both of the context handles being invalid. I have no idea why this happens, but can only surmise that it's a timing issue, as we check the validity of the contexts upon creation, so something must be invalidating them in the meantime. I strongly suspect that this is the underlying cause of Screen Curtain failing to be activated on the Windows login screen (#18743) and after login (#18693). ### Description of user facing changes: NVDA no longer crashes if Screen Curtain fails to be enabled when initializing NVDA. Screen curtain may be enabled more reliably at the login screen and immediately after login. I have not included a change log entry, because #19263 was introduced as part of 2026.1 alphas, and I don't know if this PR fixes #18693 or #18743. ### Description of developer facing changes: None ### Description of development approach: * In `screenCurtain._screenCurtain.ScreenCurtain.enable`, add logic to automatically retry enabling the Screen Curtain a fixed number of times before admitting defeat. If the issue really is a race condition or other timing issue, future retries should theoretically be uneffected by the issue, so it will succeed. Currently the maximum number of attempts is hardcoded at 3. * In `screenCurtain._screenCurtain.ScreenCurtain.__init__`, wrap `self.enable` in a try/except block. * In `gui.settingsDialogs.PrivacyAndSecurityDialog._ensureScreenCurtainEnableState`, wrap `screenCurtain.screenCurtain.enable` in a try/except block, and notify the user when enabling the screen curtain has failed. ### Testing strategy: Intentionally introduced errors into `screenCurtain._screenCurtain.ScreenCurtain.enable`, and ensure that the error handling works correctly. Intentionally introduced errors that only happen a fixed number of times into `screenCurtain._screenCurtain.ScreenCurtain.enable`, and ensure that the retry logic works correctly. ### Known issues with pull request: None

Fix-up of #19177. Summary of the issue: Since #19177, Screen curtain settings have been moved from Vision to "Privacy and Security" settings panel in the settings dialog. Though, in the input gestures dialog, its toggle command is still located in the Vision category. This is not consistent. Description of user facing changes: In the input gestures dialog, the screen curtain toggle command is now located in the Misc category rather than in the Vision category. Description of developer facing changes: N/A Description of development approach: Removed the category of the script. A script with no category lands in the Misc category.

SaschaCowley added the api-breaking-change label Nov 6, 2025

SaschaCowley commented Nov 6, 2025

View reviewed changes

Comment thread source/visionEnhancementProviders/screenCurtain.py Outdated

Comment thread source/screenCurtain.py Outdated

SaschaCowley commented Nov 6, 2025

View reviewed changes

Comment thread source/screenCurtain/_screenCurtain.py

Comment thread source/globalCommands.py Outdated

SaschaCowley force-pushed the globalScreenCurtain branch from 32512a2 to af73311 Compare November 6, 2025 04:22

SaschaCowley marked this pull request as ready for review November 6, 2025 04:23

SaschaCowley requested review from a team as code owners November 6, 2025 04:23

SaschaCowley requested review from Qchristensen and seanbudd November 6, 2025 04:23

seanbudd mentioned this pull request Nov 6, 2025

Add a new settings category called "security and privacy" in NVDA settings panel #16272

Closed

SaschaCowley marked this pull request as draft November 6, 2025 05:19

SaschaCowley force-pushed the globalScreenCurtain branch from af73311 to 987caeb Compare November 6, 2025 06:25

SaschaCowley marked this pull request as ready for review November 6, 2025 06:26

seanbudd reviewed Nov 7, 2025

View reviewed changes

CyrilleB79 mentioned this pull request Nov 7, 2025

Magnifier integration #19172

Closed

8 tasks

seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Nov 11, 2025

seanbudd marked this pull request as draft November 11, 2025 05:42

SaschaCowley marked this pull request as ready for review November 12, 2025 07:06

CyrilleB79 reviewed Nov 12, 2025

View reviewed changes

Comment thread source/globalCommands.py Outdated

seanbudd reviewed Nov 12, 2025

View reviewed changes

SaschaCowley requested a review from seanbudd November 13, 2025 01:35

seanbudd approved these changes Nov 13, 2025

View reviewed changes

SaschaCowley added blocked/needs-product-decision A product decision needs to be made. Decisions about NVDA UX or supported use-cases. and removed blocked/needs-product-decision A product decision needs to be made. Decisions about NVDA UX or supported use-cases. labels Nov 13, 2025

SaschaCowley and others added 11 commits November 17, 2025 18:24

Make duplicated message an internal constant

8b51dd4

Rename to be constant

0cc4225

Add more thorough type hints

9407b16

Fix toggle command

1776a3c

Make imports global

130055c

Remove visionEnhancementProviders.screenCurtain altogether

4db17c4

Use pgettext

42440f6

Apply suggestions from code review

1ee6c6c

Co-authored-by: Sean Budd <sean@nvaccess.org>

Update copyright headers

6c488ee

Update user guide

dcd6bc3

Switch back to plain gettext for unchanged strings

2890d2c

SaschaCowley force-pushed the globalScreenCurtain branch from dca9c92 to 2890d2c Compare November 17, 2025 07:27

SaschaCowley marked this pull request as ready for review November 17, 2025 07:28

SaschaCowley requested review from CyrilleB79 and seanbudd November 17, 2025 07:28

CyrilleB79 approved these changes Nov 17, 2025

View reviewed changes

seanbudd approved these changes Nov 18, 2025

View reviewed changes

Comment thread source/screenCurtain/_screenCurtain.py

Comment thread source/gui/settingsDialogs.py

Comment thread user_docs/en/userGuide.md Outdated

Comment thread user_docs/en/userGuide.md Outdated

Qchristensen approved these changes Nov 18, 2025

View reviewed changes

Apply suggestions from code review

36112ad

Co-authored-by: Quentin Christensen <quentin@nvaccess.org>

SaschaCowley commented Nov 19, 2025

View reviewed changes

Comment thread user_docs/en/userGuide.md Outdated

Update user_docs/en/userGuide.md

f7441bc

SaschaCowley enabled auto-merge (squash) November 19, 2025 04:49

SaschaCowley merged commit 5e44446 into master Nov 19, 2025
77 of 79 checks passed

SaschaCowley deleted the globalScreenCurtain branch November 19, 2025 23:24

github-actions Bot added this to the 2026.1 milestone Nov 19, 2025

SaschaCowley mentioned this pull request Nov 30, 2025

Fix Screen Curtain #19305

Merged

5 tasks

CyrilleB79 mentioned this pull request Jan 16, 2026

Move Toggle Screen Curtain command to Misc category #19452

Merged

5 tasks

Uh oh!

Conversation

SaschaCowley commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Link to issue number:

Summary of the issue:

Description of user facing changes:

Description of developer facing changes:

Description of development approach:

Testing strategy:

Known issues with pull request:

Code Review Checklist:

Uh oh!

cary-rowen commented Nov 6, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SaschaCowley commented Nov 6, 2025

Uh oh!

CyrilleB79 commented Nov 6, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CyrilleB79 commented Nov 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CyrilleB79 commented Nov 13, 2025

Uh oh!

CyrilleB79 commented Nov 13, 2025

Uh oh!

CyrilleB79 left a comment

Choose a reason for hiding this comment

Uh oh!

seanbudd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Qchristensen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

SaschaCowley commented Nov 6, 2025 •

edited

Loading