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

Give an option to make the computer screen black to nvda #7857

Open
fernando-jose-silva opened this Issue Dec 13, 2017 · 75 comments

Comments

Projects
None yet
@fernando-jose-silva

fernando-jose-silva commented Dec 13, 2017

This call has already appeared here.
Sorry I could not find the previous call, maybe because of my bad english.
What's new is that this feature was announced in the December update of jaws 2018, so I believe this is technically possible.
My request would be for some nv acces specialist to study and see the feasibility of giving nvda this feature.

@ehollig

This comment has been minimized.

Show comment
Hide comment
@ehollig

ehollig Dec 13, 2017

Collaborator

This is a duplicate of #5856. On the Enhancements and Improvements in JAWS 2018 page, it does say something about Screen Shade. Leaving this open in case @michaelDCurran or @jcsteh may have any ideas or know if Microsoft has changed something.

Collaborator

ehollig commented Dec 13, 2017

This is a duplicate of #5856. On the Enhancements and Improvements in JAWS 2018 page, it does say something about Screen Shade. Leaving this open in case @michaelDCurran or @jcsteh may have any ideas or know if Microsoft has changed something.

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Dec 13, 2017

Contributor
Contributor

jcsteh commented Dec 13, 2017

@Adriani90

This comment has been minimized.

Show comment
Hide comment
@Adriani90

Adriani90 Dec 13, 2017

I am a bit struggling with this function due to following reasons:

  1. There are lots of other possibilities to turn off the screen in windows
  2. It could be a trigger for compliance conflicts especially in professional fields where you are not allowed to use software which can influence screen properties such as layout, brightness or visibility (multi screen work places where you work in a team)
  3. Would it cause issues when using remote desktop or Citrix?

And in fact, sighted people don't use such functions to hide what they are doing. If you want data privacy, you could change the screen orientation and turn the brightness on 0. In case you use a laptop and you have a bluetooth keyboard you can change your power settings so that the full functionality is being maintained even when the lid is closed.

Adriani90 commented Dec 13, 2017

I am a bit struggling with this function due to following reasons:

  1. There are lots of other possibilities to turn off the screen in windows
  2. It could be a trigger for compliance conflicts especially in professional fields where you are not allowed to use software which can influence screen properties such as layout, brightness or visibility (multi screen work places where you work in a team)
  3. Would it cause issues when using remote desktop or Citrix?

And in fact, sighted people don't use such functions to hide what they are doing. If you want data privacy, you could change the screen orientation and turn the brightness on 0. In case you use a laptop and you have a bluetooth keyboard you can change your power settings so that the full functionality is being maintained even when the lid is closed.

@Adriani90

This comment has been minimized.

Show comment
Hide comment
@Adriani90

Adriani90 Dec 13, 2017

I think NVDA should maintain and improve its function as a screen reader and not as a data privacy tool.

Adriani90 commented Dec 13, 2017

I think NVDA should maintain and improve its function as a screen reader and not as a data privacy tool.

@fernando-jose-silva

This comment has been minimized.

Show comment
Hide comment
@fernando-jose-silva

fernando-jose-silva Dec 14, 2017

I believe that this function can only be done for private purposes.
Logically this functionality in nvda could be disabled and rehabilitated to the user's liking.
I understand that windows already provides resources for this functionality, however it is laborious, especially when you want to have this privacy feature quickly, when for example you are entering banking data on a web page.
It is relevant to have this functionality because as a visual deficient I am not sure of who is close to me, and may be looking at my screen mainly in public places.
This is a feature that is being appraised and implemented by many screen readers like jaws now, in voice over for iphone, and if I am not mistaken the screen readers for android too, talkback and shine plus. In Brazil I also believe that there may be in other countries, when a visually impaired person is using the ATM the box makes the screen black too.
I am a frequent user of this feature on iphone especially when I am reading my information as bank account stratum in public spaces.
This functionality in the computer would give me much more confidence, to use my computer without fear in public spaces, another example is when reading a little, inside the workspace itself, I already had a situation where I was reading my pamper, and suddenly a his own co-worker made a comment about the values, this either he was reading quietly and he did not persuade that this personal and sensible information was being collected by a person who has a minimum relationship.

Finally, we must also think, I live in Brazil, which is a recognized place, even in the outskirts, as a place not very safe, here scams are common in people without disabilities, imagination about people with disabilities.

fernando-jose-silva commented Dec 14, 2017

I believe that this function can only be done for private purposes.
Logically this functionality in nvda could be disabled and rehabilitated to the user's liking.
I understand that windows already provides resources for this functionality, however it is laborious, especially when you want to have this privacy feature quickly, when for example you are entering banking data on a web page.
It is relevant to have this functionality because as a visual deficient I am not sure of who is close to me, and may be looking at my screen mainly in public places.
This is a feature that is being appraised and implemented by many screen readers like jaws now, in voice over for iphone, and if I am not mistaken the screen readers for android too, talkback and shine plus. In Brazil I also believe that there may be in other countries, when a visually impaired person is using the ATM the box makes the screen black too.
I am a frequent user of this feature on iphone especially when I am reading my information as bank account stratum in public spaces.
This functionality in the computer would give me much more confidence, to use my computer without fear in public spaces, another example is when reading a little, inside the workspace itself, I already had a situation where I was reading my pamper, and suddenly a his own co-worker made a comment about the values, this either he was reading quietly and he did not persuade that this personal and sensible information was being collected by a person who has a minimum relationship.

Finally, we must also think, I live in Brazil, which is a recognized place, even in the outskirts, as a place not very safe, here scams are common in people without disabilities, imagination about people with disabilities.

@Adriani90

This comment has been minimized.

Show comment
Hide comment
@Adriani90

Adriani90 Dec 14, 2017

Ok I get your Point. I think the Screen shadowing function in NVDA could indeed make sense especially for touch screen devices. But it should be implemented in such a way that the user can not shadow the Screen accidentally by pressing an easy key stroke or so. This could cause confusions in professional situations. I think an Addon for this would be a better solution.

Adriani90 commented Dec 14, 2017

Ok I get your Point. I think the Screen shadowing function in NVDA could indeed make sense especially for touch screen devices. But it should be implemented in such a way that the user can not shadow the Screen accidentally by pressing an easy key stroke or so. This could cause confusions in professional situations. I think an Addon for this would be a better solution.

@Brian1Gaff

This comment has been minimized.

Show comment
Hide comment
@Brian1Gaff

Brian1Gaff Dec 15, 2017

Brian1Gaff commented Dec 15, 2017

@jcsteh

This comment has been minimized.

Show comment
Hide comment
@jcsteh

jcsteh Dec 15, 2017

Contributor
Contributor

jcsteh commented Dec 15, 2017

@Brian1Gaff

This comment has been minimized.

Show comment
Hide comment
@Brian1Gaff

Brian1Gaff Dec 15, 2017

Brian1Gaff commented Dec 15, 2017

@bhavyashah

This comment has been minimized.

Show comment
Hide comment
@bhavyashah

bhavyashah Dec 16, 2017

bhavyashah commented Dec 16, 2017

@dineshkaushal

This comment has been minimized.

Show comment
Hide comment
@dineshkaushal

dineshkaushal Dec 16, 2017

Contributor
Contributor

dineshkaushal commented Dec 16, 2017

@Adriani90

This comment has been minimized.

Show comment
Hide comment
@Adriani90

Adriani90 Dec 16, 2017

Adriani90 commented Dec 16, 2017

@nunahosting

This comment has been minimized.

Show comment
Hide comment
@nunahosting

nunahosting Dec 17, 2017

nunahosting commented Dec 17, 2017

@Adriani90

This comment has been minimized.

Show comment
Hide comment
@Adriani90

Adriani90 Dec 17, 2017

Adriani90 commented Dec 17, 2017

@bhavyashah

This comment has been minimized.

Show comment
Hide comment
@bhavyashah

bhavyashah Dec 17, 2017

bhavyashah commented Dec 17, 2017

@Adriani90

This comment has been minimized.

Show comment
Hide comment
@Adriani90

Adriani90 Dec 17, 2017

As far as I know Jaws is the only screen reader which comes as an independent software with this feature. Talk back and voice over are meant to give the user security and data privacy on mobile systems, I wonder if voice over has this function on Mac?

I agree that the function is important to some extent, but in my opinion, it could cause unexpected issues that's why I said an addon should come first before integrating this in core.

Adriani90 commented Dec 17, 2017

As far as I know Jaws is the only screen reader which comes as an independent software with this feature. Talk back and voice over are meant to give the user security and data privacy on mobile systems, I wonder if voice over has this function on Mac?

I agree that the function is important to some extent, but in my opinion, it could cause unexpected issues that's why I said an addon should come first before integrating this in core.

@bhavyashah

This comment has been minimized.

Show comment
Hide comment
@bhavyashah

bhavyashah Dec 17, 2017

bhavyashah commented Dec 17, 2017

@fernando-jose-silva

This comment has been minimized.

Show comment
Hide comment
@fernando-jose-silva

fernando-jose-silva Dec 17, 2017

I am in favor of this feature of nvda being implemented in the kernel.
Nvda already has very important addons that are very important for a good experience of using the screen reader. This is fine, however when I indicate and I will give nvda to a new user I have to indicate nvda, and I still have to indicate all the add-ons that make use of the screen reader, and we still have to teach how to download and install the nvda. addons This sequence of rooms for a new user with basic knowledge of the reader is boring, and for times very complicated.
Many times I have to give myself a copy of nvda already with the addons implemented.

fernando-jose-silva commented Dec 17, 2017

I am in favor of this feature of nvda being implemented in the kernel.
Nvda already has very important addons that are very important for a good experience of using the screen reader. This is fine, however when I indicate and I will give nvda to a new user I have to indicate nvda, and I still have to indicate all the add-ons that make use of the screen reader, and we still have to teach how to download and install the nvda. addons This sequence of rooms for a new user with basic knowledge of the reader is boring, and for times very complicated.
Many times I have to give myself a copy of nvda already with the addons implemented.

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Dec 17, 2017

Collaborator

Hi,

I think the overall purpose of NVDA should be screen reading (at least the Core).

Based on research, screen shade won't improve battery life, save power, nor provide privacy. In essence, when this is on, an overlay window is imposed on top of other windows, and this is gone once this feature is turned off. Thus, it'll not result in improved battery life, which means no power savings. The overlay won't provide privacy as noted by some, as NVDA can use multiple output modules (namely, speech, braille, tones, logs, waves, remote access with an add-on, etc.), and if tricked, can send whatever it is saying to others (including other apps or over the Internet if told by malware). Input is vulnerable as well: screen reading is more than output, and because input is involved, it can be captured if not secured properly).

Thus, if people truly need this, I suggest creating an add-on. Then I think we should get very good justifications that cannot be refuted before returning to including this into Core.

Thanks.

Collaborator

josephsl commented Dec 17, 2017

Hi,

I think the overall purpose of NVDA should be screen reading (at least the Core).

Based on research, screen shade won't improve battery life, save power, nor provide privacy. In essence, when this is on, an overlay window is imposed on top of other windows, and this is gone once this feature is turned off. Thus, it'll not result in improved battery life, which means no power savings. The overlay won't provide privacy as noted by some, as NVDA can use multiple output modules (namely, speech, braille, tones, logs, waves, remote access with an add-on, etc.), and if tricked, can send whatever it is saying to others (including other apps or over the Internet if told by malware). Input is vulnerable as well: screen reading is more than output, and because input is involved, it can be captured if not secured properly).

Thus, if people truly need this, I suggest creating an add-on. Then I think we should get very good justifications that cannot be refuted before returning to including this into Core.

Thanks.

@bhavyashah

This comment has been minimized.

Show comment
Hide comment
@bhavyashah

bhavyashah Dec 18, 2017

bhavyashah commented Dec 18, 2017

@nidza07

This comment has been minimized.

Show comment
Hide comment
@nidza07

nidza07 Dec 18, 2017

nidza07 commented Dec 18, 2017

@Brian1Gaff

This comment has been minimized.

Show comment
Hide comment
@Brian1Gaff

Brian1Gaff Dec 18, 2017

Brian1Gaff commented Dec 18, 2017

@fernando-jose-silva

This comment has been minimized.

Show comment
Hide comment
@fernando-jose-silva

fernando-jose-silva Dec 18, 2017

In my opinion the function of making the screen black contributes a lot to the privacy, in the case of sound there is always the feature of putting on a headset, on the screen there is no easy solution to hide what is displayed.
Nvda can help novice users by not requiring them to download numerous addons to get relevant functionality.

fernando-jose-silva commented Dec 18, 2017

In my opinion the function of making the screen black contributes a lot to the privacy, in the case of sound there is always the feature of putting on a headset, on the screen there is no easy solution to hide what is displayed.
Nvda can help novice users by not requiring them to download numerous addons to get relevant functionality.

@leonardder

This comment has been minimized.

Show comment
Hide comment
@leonardder

leonardder Dec 18, 2017

Collaborator

I'm not in favour of this request as per the following arguments:

  1. As mentioned earlier, NVDA is a screen reader, not a privacy guard. I therefore agree with those saying that an add-on would be best for this.
  2. The reason why I use screen curtain on my iPhone is battery saving, not privacy, that is, I'm not even sure screen curtain saves battery. Anyhow, sighted people can't turn off the screen as they simply need to see what is on it, so why should we as blind or partially sighted people have the functionality to turn the screen off because of privacy? Sighted people don't have that privacy, so why should we have it? And, if you really need the privacy you desire, you can close a lit, dim the display, set windows projection mode to an external display (if the system supports that).
  3. As a simple screen changer like focus highlight has a major negative impact on the functioning of screen review, I'm afraid that blacking out the screen might have even more negative impact on the functioning of screen review. Update: I tested this with JAWS 2018 and this particular feature in JAWS doesn't seem to affect screen review.
  4. We don't have to offer everything JAWS has in core. JAWS is known to have a different philosophy than NVDA, e.g. JAWS has research it and other features that go beyond the scope of what a screen reader should doin my opinion. I'd really appreciate it if NVDA core keeps the philosophy of only including the things in core that are directly in aid of screen reading.
Collaborator

leonardder commented Dec 18, 2017

I'm not in favour of this request as per the following arguments:

  1. As mentioned earlier, NVDA is a screen reader, not a privacy guard. I therefore agree with those saying that an add-on would be best for this.
  2. The reason why I use screen curtain on my iPhone is battery saving, not privacy, that is, I'm not even sure screen curtain saves battery. Anyhow, sighted people can't turn off the screen as they simply need to see what is on it, so why should we as blind or partially sighted people have the functionality to turn the screen off because of privacy? Sighted people don't have that privacy, so why should we have it? And, if you really need the privacy you desire, you can close a lit, dim the display, set windows projection mode to an external display (if the system supports that).
  3. As a simple screen changer like focus highlight has a major negative impact on the functioning of screen review, I'm afraid that blacking out the screen might have even more negative impact on the functioning of screen review. Update: I tested this with JAWS 2018 and this particular feature in JAWS doesn't seem to affect screen review.
  4. We don't have to offer everything JAWS has in core. JAWS is known to have a different philosophy than NVDA, e.g. JAWS has research it and other features that go beyond the scope of what a screen reader should doin my opinion. I'd really appreciate it if NVDA core keeps the philosophy of only including the things in core that are directly in aid of screen reading.
@nunahosting

This comment has been minimized.

Show comment
Hide comment
@nunahosting

nunahosting Dec 18, 2017

nunahosting commented Dec 18, 2017

@nunahosting

This comment has been minimized.

Show comment
Hide comment
@nunahosting

nunahosting Dec 18, 2017

nunahosting commented Dec 18, 2017

@Brian1Gaff

This comment has been minimized.

Show comment
Hide comment
@Brian1Gaff

Brian1Gaff Dec 18, 2017

Brian1Gaff commented Dec 18, 2017

@fernando-jose-silva

This comment has been minimized.

Show comment
Hide comment
@fernando-jose-silva

fernando-jose-silva Dec 18, 2017

A blind man needs the privacy feature simply because he can not see who is watching his screen.
A person with vision can see who is watching your screen.
The nvda can give users a quick way to lay out the black screen, without having to make a lot of rooms to leave the screen black, these rooms simple for a more complicated advanced user for an unexpected user.

fernando-jose-silva commented Dec 18, 2017

A blind man needs the privacy feature simply because he can not see who is watching his screen.
A person with vision can see who is watching your screen.
The nvda can give users a quick way to lay out the black screen, without having to make a lot of rooms to leave the screen black, these rooms simple for a more complicated advanced user for an unexpected user.

@nunahosting

This comment has been minimized.

Show comment
Hide comment
@nunahosting

nunahosting Dec 18, 2017

nunahosting commented Dec 18, 2017

@Adriani90

This comment has been minimized.

Show comment
Hide comment
@Adriani90

Adriani90 Dec 18, 2017

Adriani90 commented Dec 18, 2017

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Jul 31, 2018

Collaborator
Collaborator

josephsl commented Jul 31, 2018

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Jul 31, 2018

Contributor

Hi, great. As a spanish person, even if this is developed as aproof of concept add-on, I would like this is registered on the translation system and created as a complete add-on.
Disadvantages:

  1. This will require more time and work by more people, included translators.

Advantages:

  1. This will bring more feed-back for a widest range of people and testing will be done in a way more similar respect the feature to include in NVDA.
  2. If it is not included soon, people will enjoy this as better as possible.
    Please, @leonardder and @josephsl, consider this request.

Thanks

Contributor

nvdaes commented Jul 31, 2018

Hi, great. As a spanish person, even if this is developed as aproof of concept add-on, I would like this is registered on the translation system and created as a complete add-on.
Disadvantages:

  1. This will require more time and work by more people, included translators.

Advantages:

  1. This will bring more feed-back for a widest range of people and testing will be done in a way more similar respect the feature to include in NVDA.
  2. If it is not included soon, people will enjoy this as better as possible.
    Please, @leonardder and @josephsl, consider this request.

Thanks

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Jul 31, 2018

Collaborator
Collaborator

josephsl commented Jul 31, 2018

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Jul 31, 2018

Contributor

If this option won't be available for profiles, I agree, though maybe useful to make it configurable to disable or enable it for certain programs or situations.
Thanks.

Contributor

nvdaes commented Jul 31, 2018

If this option won't be available for profiles, I agree, though maybe useful to make it configurable to disable or enable it for certain programs or situations.
Thanks.

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Jul 31, 2018

Collaborator
Collaborator

josephsl commented Jul 31, 2018

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Jul 31, 2018

Contributor

You are free, of course, but if it's for testing and maybe used while this is included in NVDA (milestone is not set), imo it would be better to make an add-on as complete as possible, with support for profiles and the possibility of adding a gesture for enabling and disabling, or whatever is included in the PR.
Thanks

Contributor

nvdaes commented Jul 31, 2018

You are free, of course, but if it's for testing and maybe used while this is included in NVDA (milestone is not set), imo it would be better to make an add-on as complete as possible, with support for profiles and the possibility of adding a gesture for enabling and disabling, or whatever is included in the PR.
Thanks

@fernando-jose-silva

This comment has been minimized.

Show comment
Hide comment
@fernando-jose-silva

fernando-jose-silva Jul 31, 2018

it would be possible to assign a shortcut key to enable and disable this function.
It would be interesting to enable the dark screen for a website from a bank, and normal display the screen to a google site.

fernando-jose-silva commented Jul 31, 2018

it would be possible to assign a shortcut key to enable and disable this function.
It would be interesting to enable the dark screen for a website from a bank, and normal display the screen to a google site.

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Jul 31, 2018

Collaborator

Hi,

James Scholes says:

You have a check in place to raise an error if the user isn't running the required version of Windows, but it's ineffective because of the fact that the user needs to be on Windows 8 or above to successfully import the winMagnification module. Please try to test these checks if at all possible, or at the very least, pay attention to what your code does when imported. Ideally, your code shouldn't do anything at module level unless absolutely necessary.

Error importing global plugin screenCurtain Traceback (most recent call last):
File "globalPluginHandler.pyc", line 22, in listPlugins
File
"C:\Users\jscholes\AppData\Roaming\nvda\addons\screenCurtain\globalPlugins\screenCurtain_init_.py",
line 15, in
File
"C:\Users\jscholes\AppData\Roaming\nvda\addons\screenCurtain\globalPlugins\screenCurtain\winMagnification.py",
line 98, in
AttributeError: function 'MagSetFullscreenTransform' not found

Commend end

Although the traceback is from my add-on, I think it is applicable to a future pull request - perhaps doing try/except when trying to load magnification API wrapper.

Thanks.

Collaborator

josephsl commented Jul 31, 2018

Hi,

James Scholes says:

You have a check in place to raise an error if the user isn't running the required version of Windows, but it's ineffective because of the fact that the user needs to be on Windows 8 or above to successfully import the winMagnification module. Please try to test these checks if at all possible, or at the very least, pay attention to what your code does when imported. Ideally, your code shouldn't do anything at module level unless absolutely necessary.

Error importing global plugin screenCurtain Traceback (most recent call last):
File "globalPluginHandler.pyc", line 22, in listPlugins
File
"C:\Users\jscholes\AppData\Roaming\nvda\addons\screenCurtain\globalPlugins\screenCurtain_init_.py",
line 15, in
File
"C:\Users\jscholes\AppData\Roaming\nvda\addons\screenCurtain\globalPlugins\screenCurtain\winMagnification.py",
line 98, in
AttributeError: function 'MagSetFullscreenTransform' not found

Commend end

Although the traceback is from my add-on, I think it is applicable to a future pull request - perhaps doing try/except when trying to load magnification API wrapper.

Thanks.

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Jul 31, 2018

Collaborator

Hi,

According to another user, screen curtain functionality (add-on) is working as advertised.

Thanks.

Collaborator

josephsl commented Jul 31, 2018

Hi,

According to another user, screen curtain functionality (add-on) is working as advertised.

Thanks.

@derekriemer

This comment has been minimized.

Show comment
Hide comment
@derekriemer

derekriemer Aug 1, 2018

Collaborator

@nvdaes commented on Jul 31, 2018, 2:22 PM MDT:

You are free, of course, but if it's for testing and maybe used while this is included in NVDA (milestone is not set), imo it would be better to make an add-on as complete as possible, with support for profiles and the possibility of adding a gesture for enabling and disabling, or whatever is included in the PR.
Thanks

If it's not hard, you can add a checkMenuItem

Collaborator

derekriemer commented Aug 1, 2018

@nvdaes commented on Jul 31, 2018, 2:22 PM MDT:

You are free, of course, but if it's for testing and maybe used while this is included in NVDA (milestone is not set), imo it would be better to make an add-on as complete as possible, with support for profiles and the possibility of adding a gesture for enabling and disabling, or whatever is included in the PR.
Thanks

If it's not hard, you can add a checkMenuItem

@derekriemer

This comment has been minimized.

Show comment
Hide comment
@derekriemer

derekriemer Aug 1, 2018

Collaborator

you might want to post the link here for completeness.

Collaborator

derekriemer commented Aug 1, 2018

you might want to post the link here for completeness.

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Aug 1, 2018

Collaborator
Collaborator

josephsl commented Aug 1, 2018

@Neurrone

This comment has been minimized.

Show comment
Hide comment
@Neurrone

Neurrone Aug 1, 2018

Hi,

This add-on works great. The only minor thing I noticed is that the mouse cursor appears on the display, and everything else is blacked out.

Btw, does this improve battery life, or is it just a privacy enhancing tool?

Neurrone commented Aug 1, 2018

Hi,

This add-on works great. The only minor thing I noticed is that the mouse cursor appears on the display, and everything else is blacked out.

Btw, does this improve battery life, or is it just a privacy enhancing tool?

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Aug 1, 2018

Collaborator
Collaborator

josephsl commented Aug 1, 2018

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Aug 1, 2018

Contributor

Hi cloned the add-on and, if the purpose is to provide screen curtain functionality when it's enabled, I'm sure it works fine.
However, if the objective is to test the PR, I think it would be better trying to use the screenCurtain.py file:

  1. It would be easier for reviewers to check a part of the code used in the PR.
  2. For testers, this would be more real too, though the add-on doesn't include all the functionality provided in the module.
  3. It would be easy to make add-on improvements including other vision features for testing.

Just an opinion.

Thanks

Contributor

nvdaes commented Aug 1, 2018

Hi cloned the add-on and, if the purpose is to provide screen curtain functionality when it's enabled, I'm sure it works fine.
However, if the objective is to test the PR, I think it would be better trying to use the screenCurtain.py file:

  1. It would be easier for reviewers to check a part of the code used in the PR.
  2. For testers, this would be more real too, though the add-on doesn't include all the functionality provided in the module.
  3. It would be easy to make add-on improvements including other vision features for testing.

Just an opinion.

Thanks

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Aug 1, 2018

Contributor

@leonardder, I'm seing your code on vision branch and small errors related to a tipo or aduplicate import of config are found.
I know this is a prototype. Anyway, in case you find useful some contributions for this small things, let us know.
Thanks for your work.

Contributor

nvdaes commented Aug 1, 2018

@leonardder, I'm seing your code on vision branch and small errors related to a tipo or aduplicate import of config are found.
I know this is a prototype. Anyway, in case you find useful some contributions for this small things, let us know.
Thanks for your work.

@leonardder

This comment has been minimized.

Show comment
Hide comment
@leonardder

leonardder Aug 1, 2018

Collaborator
Collaborator

leonardder commented Aug 1, 2018

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Aug 1, 2018

Contributor

Thanks, honored providing feedback for your work.
In vision panel of gui/settingsDialogs, comments refer to braille settings instead of vision.

Contributor

nvdaes commented Aug 1, 2018

Thanks, honored providing feedback for your work.
In vision panel of gui/settingsDialogs, comments refer to braille settings instead of vision.

@netblue44

This comment has been minimized.

Show comment
Hide comment
@netblue44

netblue44 Aug 1, 2018

netblue44 commented Aug 1, 2018

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Aug 2, 2018

Contributor

Please, if this is not a bug, ignore this comment.
I have bundled the whole vision package in an add-on for testing with alpha snapshots. For now I'm testing on Windows 7, and seems that, if screenCurtain is not in registered providers, vision is not initialized.
The plugin of the add-on just imports vision, ads vision configuration to config.conf.spec and performs vision.initialize() to add other tasks for testing.
Here is the relevant piece of NVDA's log. Later I will test on Windows 10:

ERROR - globalPluginHandler.listPlugins (14:43:12.316):
Error importing global plugin vision
Traceback (most recent call last):
File "globalPluginHandler.pyc", line 22, in listPlugins
File "C:\Users\USUARIO\AppData\Roaming\nvda\addons\vision\globalPlugins\vision_init_.py", line 26, in
File "C:\Users\USUARIO\AppData\Roaming\nvda\addons\vision\globalPlugins\vision....\vision_init_.py", line 553, in initialize
File "C:\Users\USUARIO\AppData\Roaming\nvda\addons\vision\globalPlugins\vision....\vision_init_.py", line 362, in init
AttributeError: 'module' object has no attribute 'configProfileSwitched'
ERROR - unhandled exception (14:43:12.486):
Traceback (most recent call last):
File "wx\core.pyc", line 3240, in
File "C:\Users\USUARIO\AppData\Roaming\nvda\addons\vision\globalPlugins\vision....\vision_init_.py", line 404, in setProvider
ValueError: Vision enhancement provider screenCurtain not registered

Contributor

nvdaes commented Aug 2, 2018

Please, if this is not a bug, ignore this comment.
I have bundled the whole vision package in an add-on for testing with alpha snapshots. For now I'm testing on Windows 7, and seems that, if screenCurtain is not in registered providers, vision is not initialized.
The plugin of the add-on just imports vision, ads vision configuration to config.conf.spec and performs vision.initialize() to add other tasks for testing.
Here is the relevant piece of NVDA's log. Later I will test on Windows 10:

ERROR - globalPluginHandler.listPlugins (14:43:12.316):
Error importing global plugin vision
Traceback (most recent call last):
File "globalPluginHandler.pyc", line 22, in listPlugins
File "C:\Users\USUARIO\AppData\Roaming\nvda\addons\vision\globalPlugins\vision_init_.py", line 26, in
File "C:\Users\USUARIO\AppData\Roaming\nvda\addons\vision\globalPlugins\vision....\vision_init_.py", line 553, in initialize
File "C:\Users\USUARIO\AppData\Roaming\nvda\addons\vision\globalPlugins\vision....\vision_init_.py", line 362, in init
AttributeError: 'module' object has no attribute 'configProfileSwitched'
ERROR - unhandled exception (14:43:12.486):
Traceback (most recent call last):
File "wx\core.pyc", line 3240, in
File "C:\Users\USUARIO\AppData\Roaming\nvda\addons\vision\globalPlugins\vision....\vision_init_.py", line 404, in setProvider
ValueError: Vision enhancement provider screenCurtain not registered

@leonardder

This comment has been minimized.

Show comment
Hide comment
@leonardder

leonardder Aug 2, 2018

Collaborator
Collaborator

leonardder commented Aug 2, 2018

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Aug 2, 2018

Collaborator
Collaborator

josephsl commented Aug 2, 2018

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Aug 2, 2018

Contributor

OK, then, if this is due to issues on master snapshots, I will wait to build an add-on with the complete framework. The problem seems to be configProfileSwitched, I don't know if other issues will be found, but removing this the plugin at least doesn't raise errors.
Thanks

Contributor

nvdaes commented Aug 2, 2018

OK, then, if this is due to issues on master snapshots, I will wait to build an add-on with the complete framework. The problem seems to be configProfileSwitched, I don't know if other issues will be found, but removing this the plugin at least doesn't raise errors.
Thanks

@josephsl

This comment has been minimized.

Show comment
Hide comment
@josephsl

josephsl Aug 2, 2018

Collaborator
Collaborator

josephsl commented Aug 2, 2018

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Aug 2, 2018

Contributor

OK, I realised it is affecting Emoticons. When you can, please let us know how to fix it.
Cheers

Contributor

nvdaes commented Aug 2, 2018

OK, I realised it is affecting Emoticons. When you can, please let us know how to fix it.
Cheers

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Aug 2, 2018

Contributor

Hi, hope I can write properly, since I have issues with Chrome in multiline edit controls.
As explained by Joseph on the add-ons mailing list, we need to useconfig. post_configProfileSwitch.
I have changed this in the vision package in my add-on and it seems to work fine. I have an issue in a script supposed to toggle screen curtain, and the issue applies when switching profiles. I think it's due to a wrong value set by me in config.conf["vision"["colorEnhancer"]. I change it from None and "screenCurtain". If someone can please let me know the firght value or if it's another issue. Cheers

Contributor

nvdaes commented Aug 2, 2018

Hi, hope I can write properly, since I have issues with Chrome in multiline edit controls.
As explained by Joseph on the add-ons mailing list, we need to useconfig. post_configProfileSwitch.
I have changed this in the vision package in my add-on and it seems to work fine. I have an issue in a script supposed to toggle screen curtain, and the issue applies when switching profiles. I think it's due to a wrong value set by me in config.conf["vision"["colorEnhancer"]. I change it from None and "screenCurtain". If someone can please let me know the firght value or if it's another issue. Cheers

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Aug 2, 2018

Contributor

Maybe an issue of vision package, I don't know, but I get errors using directlyvision.handler. setProvider too.
Here is the relevant part of log. Thanks:

ERROR - unhandled exception (20:48:18.342):
Traceback (most recent call last):
File "wx\core.pyc", line 3240, in
File "C:\Users\User\AppData\Roaming\nvda\addons\vision\globalPlugins\vision....\vision_init_.py", line 410, in setProvider
TypeError: argument of type 'Getter' is not iterable

Contributor

nvdaes commented Aug 2, 2018

Maybe an issue of vision package, I don't know, but I get errors using directlyvision.handler. setProvider too.
Here is the relevant part of log. Thanks:

ERROR - unhandled exception (20:48:18.342):
Traceback (most recent call last):
File "wx\core.pyc", line 3240, in
File "C:\Users\User\AppData\Roaming\nvda\addons\vision\globalPlugins\vision....\vision_init_.py", line 410, in setProvider
TypeError: argument of type 'Getter' is not iterable

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Aug 4, 2018

Contributor

More feedback based on the add-on I created locally, bundling your package, @leonardder.
I think the issue can be in the _get_supportedRoles(cls) method, not iterable.
It returns a frozenset.
I think that, if config.conf["vision"] accepts a cictionary whose keys are roles, really each class, for instance the corresponding to ScreenCurtain, just supports a role, in this case colorEnhancer.
conflicting roles make sense, but I'm not sure about supported roles.

I may public the add-on just for testing the framework and provide feedback, though maybe you aren't working on this right now, and I will wait if you prefer this.

Thanks, I'm delighted with this work. Thank you also to authors of add-ons like Windows 7 Magnifier(Dominic Canare;) and focusHighlight @nishimotz

Contributor

nvdaes commented Aug 4, 2018

More feedback based on the add-on I created locally, bundling your package, @leonardder.
I think the issue can be in the _get_supportedRoles(cls) method, not iterable.
It returns a frozenset.
I think that, if config.conf["vision"] accepts a cictionary whose keys are roles, really each class, for instance the corresponding to ScreenCurtain, just supports a role, in this case colorEnhancer.
conflicting roles make sense, but I'm not sure about supported roles.

I may public the add-on just for testing the framework and provide feedback, though maybe you aren't working on this right now, and I will wait if you prefer this.

Thanks, I'm delighted with this work. Thank you also to authors of add-ons like Windows 7 Magnifier(Dominic Canare;) and focusHighlight @nishimotz

@leonardder

This comment has been minimized.

Show comment
Hide comment
@leonardder

leonardder Aug 4, 2018

Collaborator

@nvdaes: While I'm really thankful for your assistance and exploration of the vision module, there is just too much stuff in it that relies on an implementation into core. For example, the vision framework also depends on #8393. This clarifies the "TypeError: argument of type 'Getter' is not iterable" error.

The idea behind multiple supported roles is that usually, magnifier products like ZoomText implement both magnification and color enhancing functionality. When a magnifier add-on would be created that depends on ZoomText or the Windows Magnification API, for example, using separate providers for roles would result in duplicated code and weird conflicts.

Conflicting roles do make sense because it is a waste of resource to magnify a screen or put focus highlights on it when it is black.

Collaborator

leonardder commented Aug 4, 2018

@nvdaes: While I'm really thankful for your assistance and exploration of the vision module, there is just too much stuff in it that relies on an implementation into core. For example, the vision framework also depends on #8393. This clarifies the "TypeError: argument of type 'Getter' is not iterable" error.

The idea behind multiple supported roles is that usually, magnifier products like ZoomText implement both magnification and color enhancing functionality. When a magnifier add-on would be created that depends on ZoomText or the Windows Magnification API, for example, using separate providers for roles would result in duplicated code and weird conflicts.

Conflicting roles do make sense because it is a waste of resource to magnify a screen or put focus highlights on it when it is black.

@nvdaes

This comment has been minimized.

Show comment
Hide comment
@nvdaes

nvdaes Aug 4, 2018

Contributor

Thanks for your response. Then for now I won't make the add-on public, since you know what's behind bugs. If you would like me to do it, please let me know.
This is great.

Contributor

nvdaes commented Aug 4, 2018

Thanks for your response. Then for now I won't make the add-on public, since you know what's behind bugs. If you would like me to do it, please let me know.
This is great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment