Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Screen curtain should not automatically toggle off when switching configuration profiles #10476

Open
Neurrone opened this issue Nov 9, 2019 · 27 comments
Labels
bug squash target feature/configuration-profiles feature/screen-curtain p2 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority security triaged Has been triaged, issue is waiting for implementation.

Comments

@Neurrone
Copy link

Neurrone commented Nov 9, 2019

Steps to reproduce:

  1. Assign the screen curtain command to a gesture.
  2. Turn on screen curtain while in the default profile. It doesn't matter if its only temporarily or permanently on while NVDA is active.
  3. Switch to an app which would cause a different profile to load, e.g one for programming.

Actual behavior:

The screen curtain is automatically disabled when switching to an application which uses a profile where screen curtain has not yet been enabled. Users also receive no alerts about this surprising and potentially dangerous behaviour. The only way to know is to visually see the screen.

Expected behavior:

The screen curtain should not automatically disable itself when switching profiles. I think this is a setting which makes much more sense to be global to avoid unwanted surprises. I'm also not sure what the use case is for having it automatically turn on only in certain applications, which the current implementation enables.

System configuration

NVDA installed/portable/running from source:

Installed

NVDA version:

alpha-19136,0124e647

Windows version:

1905

Name and version of other software in use when reproducing the issue:

N/a

Other information about your system:

N/a

Other questions

Does the issue still occur after restarting your PC?

Yes

Have you tried any other versions of NVDA? If so, please report their behaviors.

N/a

@Neurrone Neurrone changed the title Screen curtain should not be based off configuration profiles Screen curtain should not automatically toggle off when switching configuration profiles Nov 9, 2019
@LeonarddeR
Copy link
Collaborator

This is an interesting point. Cc @feerrenrut

While I think there might be potential for having screen curtain enabled only in certain profiles/applications, I think what you're raising here is actually broader than just screen curtain. The active providers are saved in a list. This means that, if you have screen curtain enabled in one profile and then disable it in another, then enabling the nvda highlighter in the default profile, the highlighter will also be off in that other profile.

I see several solutions:

  1. Making the vision section a global config parameter. I think this is problematic though. While valid for the screen curtain may be, I can think of reasons why one wants the highlighter on in certain situations or hide the highlight in some other situations. Also if a magnifier would be integrated into the vision system, we have to deal with different color schemes / magnification levels
  2. Giving every provider an enabled boolean in its own config section. This ensures that changes for a provider's activity in a certain profile won't touch other providers.

Additionally, we might want to make the provider announce its enabled/disabled state when the state is toggled by a config profile switch.

@Neurrone
Copy link
Author

Neurrone commented Nov 9, 2019

@LeonarddeR I think that having the disabled state announced would mostly address the privacy concerns here, and I agree that per-profile settings are appropriate for most vision providers. Not sure if there are any other cases like this where being global is more desirable.

@Neurrone
Copy link
Author

Neurrone commented Dec 9, 2019

Wondering if an intermediate solution could be implemented in time for the release of 2019.3 to address the potential privacy concerns? E.g, an announcement when the screen curtain gets automatically toggled off.

@LeonarddeR
Copy link
Collaborator

After thinking about this a little more, I really want to keep the possibility to switch on the Screen Curtain on for some profiles and of for others.

@Neurrone
Copy link
Author

It sounds like there is a lot of design work that will take time.

I suggest as an intermediate solution just announcing when the curtain gets automatically switched off? This addresses the immediate privacy concerns, without committing to a design for how vision providers interact with profiles.

@feerrenrut
Copy link
Contributor

An interim solution has been merged (to beep when screen curtain enables / disables). I think this could be solved by looking at what caused the screen curtain became enabled. If screen curtain is enabled via gestures, it should override (but not modify) the profile. Though note, the opposite is not true, if screen curtain is disabled by a gesture, a new profile trigger should be able to enable screen curtain.

If we accept that behavior, we also must consider whether state change by gestures should be saved to the config at all. Perhaps there should be different behavior between temporary and permanent screen curtain gestures?

@feerrenrut
Copy link
Contributor

I'm removing the 2019.3 milestone and leaving this issue open. I think the immediate concern is resolved, but there is more to discuss.

@feerrenrut feerrenrut removed this from the 2019.3 milestone Jan 13, 2020
@Adriani90
Copy link
Collaborator

A solution would be to implement two shortcut for the screen curtain like

  1. Enable screen curtain for default profile
  2. Enable screen curtain for all profiles.

These shortcuts would solve most concerns and would most probably fulfil every expectation from users.

@rperez030
Copy link

I think if we follow that logic we will need to duplicate every shortcut where the problem occurs. I would say that screen curtain together with the rest of the vision settings should be toggle off when switching profile only iff the user has actively saved that setting with the value of false for that profile. Until the user has that ability, I would suggest to make the vision settings global.

@TechHorseG
Copy link

Hello, I am new to using profiles and have recently discovered the issue of the screen curtain disabling on its own.

I would like to mention two points here. First, if you are going to have the screen curtain status be per-profile, then NVDA should remember what the last status of the current active profile was, when you switch back to it.

If for example I hide Notepad because I am working with a file that contains private information, switch to another visible app, and then switch back to Notepad, then NVDA should remember that I had enabled the screen curtain for this session of Notepad and re-enable it. Just as it would remember when switching back and forth between an app you have enabled sleep mode for.

Instead NVDA doesn't remember this and I have to manually switch the screen curtain back on every time Notepad loses then regains focus.

I'd assume that this is because I have "Save configuration when exiting NVDA" set to off, but NVDA should keep track of this in memory for the current session regardless, so I don't have to keep turning the screen curtain back on myself.

My second point is that despite the above, I would like to agree with those who are suggesting that there be an option for the screen curtain to be profile-independent / global / only manually deactivated.

I am not sure why anyone would need this setting to be deactivated based on what has system focus?

If you are trying to prevent private information in the current app from being displayed onscreen, then surely you would want the screen to remain black for as long as that window is visible onscreen?

Switching to a new window does not guarantee that the original window has been fully covered.

The only circumstances in which I perhaps wouldn't mind so much if the screen curtain disables on its own, would be if either the hidden app is closed, fully minimises, or the newly focused window is fullscreen or otherwise covers it completely.

But that is a lot more complicated than just being about what profile is currently active. Much simpler to just let me have the screen hidden until I manually decide otherwise.

By all means allow people to set some sort of "always on" or "always off" option for a profile if they want this. But please also offer the ability to have no profile-specific overrides.

@Adriani90
Copy link
Collaborator

Adriani90 commented Mar 3, 2024 via email

@CyrilleB79
Copy link
Collaborator

I have already expressed this in other issues. But I confirm it here.

My opinion is that screen curtain should be global, at least due to privacy issues when the foreground window does not fully covers the whole screen. Keeping the screen curtain linked to profiles gives a false sense of security.

The current issue and other one related to screen curtain and privacy are stalled for years now, what is not a good image regarding security and privacy. Cc @seanbudd and @gerald-hartig.

Also, I am even in favor in a simpler way to use the screen curtain, i.e.:

  1. Remove the temporary mode
  2. Remove screen curtain from vision settings. Indeed I have some vision left but screen curtain is not a visual help for me... actually it prevents me from seeing anything on the screen, what cannot be called an aid at all!

I am sorry for the people who have worked on screen curtain and have implemented it as a visual provider at the time, but that was not a good design choice IMO.

@XLTechie
Copy link
Collaborator

XLTechie commented Mar 4, 2024 via email

@LeonarddeR
Copy link
Collaborator

I still stand for my point in #10476 (comment).
In other words, I prefer leaving the situation as is. I really want to have the screen curtain on in only specific profiles and off by default.
That said, I understand that there are some concerns regarding privacy and a false sense of security, but I'd rather see the technical part to be fixed in a broader scope. E.g.

  1. Allow specific options to be ignored in profiles
  2. Allow dirty profiles to be cleaned up. With a dirty profile, I mean the case of accidentally toggling an option with a shortcut when a profile is activated. When a setting is once changed in a profile, it is not possible to remove it from the profile other than manually editing the profile.

Both these options are somehow covered in #10156

@CyrilleB79
Copy link
Collaborator

I would really love to see #10156 fixed.

Though, depending on how the UX is defined in #10156's implementation, I am not sure that it would completely address the security and privacy concern with screen curtain. I also fear that if we expect #10156 to be fixed as a base for solving this issue, we shall still wait for another five years to address the screen curtain security concern.

@LeonarddeR, I understand that you have a use case for screen curtain usage with profile. Could you describe it/them here to clarify?

@LeonarddeR
Copy link
Collaborator

@LeonarddeR, I understand that you have a use case for screen curtain usage with profile. Could you describe it/them here to clarify?

I often share my screen to colleagues or other people, so my default setup is screen curtain off. However when i open WhatsApp or another application that may contain private info, I want the screen to be black, regardless of the state of the default configuration. So that's why I have a blackness profile assigned to these applications.

@TechHorseG
Copy link

@LeonarddeR, I understand that you like to have certain apps always hidden, but the current system does not cater for this. Your WhatsApp becomes visible as soon as any other window or UI element takes focus. In my case even pressing win+shift+v to listen to a system notification will cause the entire screen to become visible.

It seems to me that the current system does not cater for either those who want manual-only activation / deactivation, nor those who want a specific app to always be hidden from the screen.

I think that the only way to have a per-app setting would be if NVDA tracked the window states of triggering Apps. As in, once WhatsApp has triggered the screen curtain, it remains on until either WhatsApp has closed, or has fully minimised. And only then will NVDA stop and think about what the newly-switched to profile wants for the screen?

Still, if some people do like the current system then as others have suggested, everyone could be catered for by the inclusion of an extra option somewhere, even if only in the advanced settings, to set whether the screen curtain status is global, or handled by profiles.

@CyrilleB79
Copy link
Collaborator

@LeonarddeR, as explained by @TechHorseG, you cannot guarantee that your background Whatsapp window is not visible from people with whom you share your screen.
Or more specifically, maybe you can guarantee this, because, as an advanced user:

  1. you take care to have all your windows in full screen, avoiding to see background windows
  2. you do not jump to notifications (windows+shift+V) while in Whatsapp
  3. You do not use start menu directly while using Whatsapp, else your background window is visible
  4. you do not open non maximizable windows while using Whatsapp, e.g. pressing windows+R
  5. You take care not to keep NVDA settings dialog open in the background before switching to Whatsapp because you are aware of In recent alpha snapshots when any NVDA dialog is open configuration profiles triggers are disabled. #8821
  6. you know other situations where privacy may be compromised and you avoid them

But you cannot expect any user to have the same level of awareness regarding screen curtain, even users able to use profiles.

Thus I'd really recommend that:

  • by default the screen curtain be not profile dependant
  • if we keep the profile-dependant screen curtain feature for advanced users, its limitation regarding privacy be well documented and it be activated in the advanced settings.

@seanbudd seanbudd added p2 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority feature/screen-curtain labels Mar 13, 2024
@seanbudd seanbudd added triaged Has been triaged, issue is waiting for implementation. security labels Mar 13, 2024
@CyrilleB79
Copy link
Collaborator

@seanbudd since you have triaged this issue, could you also indicate which design you would accept to be implemented?

  • make screen curtain global, i.e. not profil dependant anymore
  • a more complex design, e.g. screen curtain can only be enabled upon profile change but not disabled upon profile change

I am in favor of the first proposal because of its much simpler and understandable UX, but would like to hear from NV Access and others before something is implemented.

@LeonarddeR, you have not answered to #10476 (comment) and #10476 (comment). It would be nice to hear your opinion on these comments to understand if you agree with our reasoning or if we have missed something.

@LeonarddeR
Copy link
Collaborator

@LeonarddeR, you have not answered to #10476 (comment) and #10476 (comment). It would be nice to hear your opinion on these comments to understand if you agree with our reasoning or if we have missed something.

I'm sorry.

@TechHorseG wrote:

@LeonarddeR, I understand that you like to have certain apps always hidden, but the current system does not cater for this. Your WhatsApp becomes visible as soon as any other window or UI element takes focus. In my case even pressing win+shift+v to listen to a system notification will cause the entire screen to become visible.

This is actually a very valid point I didn't think of before. Making this a global setting makes much more sense to me now.

I would personally try something like this:

  1. Replace config.ConfigManager.BASE_ONLY_SECTIONS with BASE_ONLY_SETTINGS
  2. This list contains either:
    • Strings (for sections)
    • Tuples (for paths, e.g. ("vision", "screenCurtain")
  3. BASE_ONLY_SECTIONS can be kept in a backwards compatible way by adding it to a getattr returning only strings from BASE_ONLY_SETTINGS
  4. I think it would be best to remove all the other cases where BASE_ONLY_SECTIONS are used and instead, let config.AggregatedSection deal with base only sections and settings.

@LeonarddeR
Copy link
Collaborator

Actually, AggregatedSection receives the path to the section as one of its constructor parameters, so it should be quite trivial to initialize a normal section in that case instead.
Question remains whether we want to support specific settings in a section, or only complete sections.

@CyrilleB79
Copy link
Collaborator

Thanks for your answers and proposals @LeonarddeR.

Before proceeding maybe it would be interesting to discuss #16272. As explained by me various times, there is no sense to put the screen curtain in the Vision settings: it's not at all a feature allowing sighted or visually impaired people to see better things, but that's a privacy feature.
Thus it should be moved in the future security / privacy panel (#16272) or in the general settings.
There are still pending discussions in #16272 though.

I acknowledge that the base only setting implementation would be nice (not only for this issue) and would already be required for some values.
Though, if screen curtain is moved to general section of the config, the implementation of base-only settings is not blocking this issue. But this can be done only in an API-breaking release.
On the other side, would the implementation of base-only settings be API-breaking?

@LeonarddeR
Copy link
Collaborator

Before proceeding maybe it would be interesting to discuss #16272. As explained by me various times, there is no sense to put the screen curtain in the Vision settings: it's not at all a feature allowing sighted or visually impaired people to see better things, but that's a privacy feature.

Although I think you're right, it seems to me that removing the screen curtain from the vision framework would have quite a big impact. I don't know if I would personally be that concerned about that.

Thanks for your answers and proposals @LeonarddeR.

Before proceeding maybe it would be interesting to discuss #16272. As explained by me various times, there is no sense to put the screen curtain in the Vision settings: it's not at all a feature allowing sighted or visually impaired people to see better things, but that's a privacy feature. Thus it should be moved in the future security / privacy panel (#16272) or in the general settings. There are still pending discussions in #16272 though.

Though, if screen curtain is moved to general section of the config, the implementation of base-only settings is not blocking this issue. But this can be done only in an API-breaking release.

Is that right? A config profile step should be able to migrate this just fine. Or are paths in the configuration now also part of the API?

On the other side, would the implementation of base-only settings be API-breaking?

That depends on whether add-ons modify config.ConfigManager.BASE_ONLY_SECTIONS. I'm afraid they might do that.

@XLTechie
Copy link
Collaborator

XLTechie commented Mar 14, 2024 via email

@lukaszgo1
Copy link
Contributor

Making screen curtain profile independent, yet keeping its settings in the vision panel is pretty inconsistent. Until now all settings which are saved in the global configuration are either in the general settings where it is clear they are not tied to a profile, or target developers i.e. use scratchpad. I also don't think we need to refactor / migrate config here - cannot we just expand our config handling so that there is ability to retrieve particular settings, not only sections, from the default profile?

@seanbudd
Copy link
Member

seanbudd commented Apr 29, 2024

See also #14121

@TristanBurchett
Copy link
Contributor

TristanBurchett commented Jul 30, 2024

Would people be open to Privacy Settings being added to the settings area of NVDA? I think it would be a good place to put the Screen Curtain and any new features that come up that could relate to privacy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug squash target feature/configuration-profiles feature/screen-curtain p2 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority security triaged Has been triaged, issue is waiting for implementation.
Projects
None yet
Development

No branches or pull requests