Ensure screen stays on during say all #17651
Conversation
|
@LeonarddeR |
|
The only sleep state I have on this system is hybernate, since I unfortunately don't have S3 here. I once disabled s0 in the bios and was too lazy to re-enable it. |
|
btw, on some computers, it is not available to turn off the s0.. |
user_docs/en/userGuide.md
Outdated
| |Default |Disabled| | ||
|
|
||
| This option ensures that the screen stays on when reading with say all or with braille (e.g. when pressing scroll buttons). | ||
| This avoids the situation where the screen unexpectedly locks during a say all. |
There was a problem hiding this comment.
The option name talks about system lock whereas here, you talk about screen lock. I have not reviewed this PR in detail, but reading the UG does not clarify things for me.
Are we talking about system lock, I guess session lock?) or the screen turning off (black)?
I think that you should describe both the use cases for enabled and for disabled here. Thanks.
There was a problem hiding this comment.
More specifically and reading #17651 (comment) by @btman16, I have concerns regarding braille.
I'd say that systems using modern standby should never enter standby, thus never lock, when reading braille.
|
Hi,
Based on the description, prevent system lock is ok, because the behavior
here actually depends on the system.
If the system supports the s3 sleep state, this simply refers to the screen
turning off while at least in a say all, whereas on a system supporting
modern standby, you would notice the system locking.
Does this help to clarify this?
Thanks,
Brandon
…On Thu, Jan 30, 2025 at 2:05 AM Cyrille Bougot ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In user_docs/en/userGuide.md
<#17651 (comment)>:
> @@ -3333,6 +3333,18 @@ Some GDI applications will highlight text with a background color, NVDA (via dis
In some situations, the text background may be entirely transparent, with the text layered on some other GUI element.
With several historically popular GUI APIs, the text may be rendered with a transparent background, but visually the background color is accurate.
+##### Prevent system lock during say all or reading with Braille {#preventSystemLock}
+
+| . {.hideHeaderRow} |.|
+|---|---|
+|Options |Default (Enabled), Disabled, Enabled|
+|Default |Disabled|
+
+This option ensures that the screen stays on when reading with say all or with braille (e.g. when pressing scroll buttons).
+This avoids the situation where the screen unexpectedly locks during a say all.
The option name talks about system lock whereas here, you talk about
screen lock. I have not reviewed this PR in detail, but reading the UG does
not clarify things for me.
Are we talking about system lock, I guess session lock?) or the screen
turning off (black)?
I think that you should describe both the use cases for enabled and for
disabled here. Thanks.
—
Reply to this email directly, view it on GitHub
<#17651 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AD6UHRVUFFSZZULYF7UQPH32NHFK7AVCNFSM6AAAAABVZXTRPGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKOBTGA3DMOBRGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
“Be what you are. This is the first step towards becoming better than you
are.”
– J. C. Hare & A. W. Hare
|
|
@CyrilleB79 I think you are raising valid concerns here.
I'm not sure how s3/s0 standby is related here. The problem is not the system idle timer, but the display idle timer. I don't think that's related to system power state, but to display power state, which is a different aspect. |
seanbudd
added
the
conceptApproved
source/braille.py
Outdated
| @@ -1769,7 +1769,7 @@ def __init__(self, handler): | |||
| #: The translated braille representation of the entire buffer. | |||
| #: @type: [int, ...] | |||
| self.brailleCells = [] | |||
| self._windowRowBufferOffsets: list[tuple[int, int]] = [(0, 1)] | |||
| self._windowRowBufferOffsets: list[tuple[int, int]] = [(0, 0)] | |||
There was a problem hiding this comment.
there are appears to be merge conflicts here
There was a problem hiding this comment.
I think this is fixed now.
LeonarddeR
force-pushed
the
avoidSLeep
branch
from
c9f9b9a to
25d7063
Compare
|
There was something wrong in the merge process, I rebased on master to get rid of it. |
|
Frankly, the UX of this new option still seems strange and I am not sure to understand it if I only read the User Guide (i.e. not the PR). The option is called "Prevent system lock during say all or reading with braille". As a simple user, I would wonder: As a user, I would prefer an option "Allow the screen to sleep / become black during say all or while reading braille" or something similar. And NVDA should evaluate itself if the options is relevant or not depending on the standby capabilities of the system. The option should be greyed out on systems where screen standby = session lock because there is no point in enabling session lock while reading braille. |
|
What exactly is the purpose of this setting? Is it that users using say all want to have longer batery time while using NVDA? When I use NVDA actively with the keyboard, the screen doesn't get black either. So why should this be optional when using say all? |
|
I initially thought this needn't be an option, but then @btman16 chimed in, saying that it was negatively impacting his battery life. SO that's why it is an option now. |
LeonarddeR
changed the title
When NVDA is reading with review cursor say all, the physical cursor isn't moving so there's basically nothing happening from a visible point of view. It makes sense not to prevent the display from turning off in that case. |
| This option ensures that the display stays on when reading with say all or with braille (e.g. when pressing scroll buttons). | ||
| This avoids the situation where the screen unexpectedly locks during a say all. | ||
| This option is enabled by default. | ||
| Consider disabling this option if you are suffering from a shorter battery life. |
There was a problem hiding this comment.
Or maybe:
| Consider disabling this option if you are suffering from a shorter battery life. | |
| Consider disabling this option if you are suffering from a shorter battery life and you do not need to use the screen. |
Just a suggestion. Feel free to accept, rephrase or decline depending on if you find it suitable or not.
There was a problem hiding this comment.
Not sure if this is relevant as locking may occur which would affect more than just screen usage
source/gui/settingsDialogs.py
Outdated
| # Translators: This is the label for a group of advanced options in the | ||
| # Advanced settings panel | ||
| label = _("System") | ||
| systemSizer = wx.StaticBoxSizer(wx.VERTICAL, self, label=label) | ||
| systemGroup = guiHelper.BoxSizerHelper(self, sizer=systemSizer) | ||
| sHelper.addItem(systemGroup) |
| @@ -36,6 +36,9 @@ As a consequence, announcement of first line indent is now supported for LibreOf | |||
| * In Word, the selection update is now reported when using Word commands to extend or reduce the selection (`f8` or `shift+f8`). (#3293, @CyrilleB79) | |||
| * In Microsoft Word 16.0.18226 and higher or when using Word object model, NVDA will now report if a heading is collapsed in both speech and braille. (#17499) | |||
| * NVDA is now able to report caret changes when pressing `alt+upArrow` or `alt+downArrow` gestures, for example in Visual Studio. (#17652, @LeonarddeR) | |||
| * Added a general setting to prevent the display to turn off during say all or reading with braille. | |||
There was a problem hiding this comment.
| * Added a general setting to prevent the display to turn off during say all or reading with braille. | |
| * Added a general setting to prevent the display turning off during say all or reading with braille. |
| @@ -36,6 +36,9 @@ As a consequence, announcement of first line indent is now supported for LibreOf | |||
| * In Word, the selection update is now reported when using Word commands to extend or reduce the selection (`f8` or `shift+f8`). (#3293, @CyrilleB79) | |||
| * In Microsoft Word 16.0.18226 and higher or when using Word object model, NVDA will now report if a heading is collapsed in both speech and braille. (#17499) | |||
| * NVDA is now able to report caret changes when pressing `alt+upArrow` or `alt+downArrow` gestures, for example in Visual Studio. (#17652, @LeonarddeR) | |||
| * Added a general setting to prevent the display to turn off during say all or reading with braille. | |||
| This option is enabled by default, but can possibly result in shorter battery life. | |||
| If you suspect this option to negatively impact your battery life, you're advised to disable it. (#17649, @LeonarddeR) | |||
There was a problem hiding this comment.
| If you suspect this option to negatively impact your battery life, you're advised to disable it. (#17649, @LeonarddeR) | |
| If you suspect this option is negatively impacting your battery life, you're advised to disable it. (#17649, @LeonarddeR) |
| @@ -1846,6 +1846,18 @@ If you wish to change the update mirror, press the "Change..." button to open th | |||
| Please note that when using an update mirror, the operator of the mirror has access to all [information sent with update checks](#GeneralSettingsCheckForUpdates). | |||
| Contact the operator of the update mirror for details of their data handling policies to ensure you are comfortable with the way your information will be handled before setting an update mirror. | |||
|
|
|||
| ##### Prevent display from turning off during say all or reading with braille {#preventDisplayTurnOff} | |||
There was a problem hiding this comment.
Please capitalie the first letter of anchors
| ##### Prevent display from turning off during say all or reading with braille {#preventDisplayTurnOff} | |
| ##### Prevent display from turning off during say all or reading with braille {#PreventDisplayTurnOff} |
| conf=config.conf, | ||
| ) | ||
| ) | ||
| self.bindHelpEvent("preventDisplayTurnOff", self.preventDisplayTurnOffCombo) |
There was a problem hiding this comment.
| self.bindHelpEvent("preventDisplayTurnOff", self.preventDisplayTurnOffCombo) | |
| self.bindHelpEvent("PreventDisplayTurnOff", self.preventDisplayTurnOffCombo) |
| | . {.hideHeaderRow} |.| | ||
| |---|---| | ||
| |Options |Default (Enabled), Disabled, Enabled| | ||
| |Default |Enabled| |
There was a problem hiding this comment.
please move this to the bottom of the section as per the latest coding standards for more information.
| This option ensures that the display stays on when reading with say all or with braille (e.g. when pressing scroll buttons). | ||
| This avoids the situation where the screen unexpectedly locks during a say all. | ||
| This option is enabled by default. | ||
| Consider disabling this option if you are suffering from a shorter battery life. |
There was a problem hiding this comment.
Not sure if this is relevant as locking may occur which would affect more than just screen usage
Link to issue number:
Fixes #17649
Partially reverts #11118
Follow up of #10111, #10643,
Summary of the issue:
On some systems (mine, for example 😉) the screen still locks during a say all even though we reset the system idle timer. We also reset the display timer in the past, but this was reverted in #11118, particularly on request of @btman16
Description of user facing changes
Added a new advanced setting that, when enabled, also resets the display timer.
Description of development approach
ES_CONTINUOUSflag. This should avoid an edge case where reading a line takes more then a minute and the sleep timer is a minute, in which case the system will still lock.Testing strategy:
Tested on my system. Noted that with the feature flag enabled, the system doesn't lock, whereas when disabled, it does lock.
Known issues with pull request:
None known
Code Review Checklist:
@coderabbitai summary