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

Braille doesn't refresh when downloading update #6862

Closed
dkager opened this issue Feb 9, 2017 · 8 comments
Closed

Braille doesn't refresh when downloading update #6862

dkager opened this issue Feb 9, 2017 · 8 comments
Assignees
Labels
p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority p4 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority
Milestone

Comments

@dkager
Copy link
Collaborator

dkager commented Feb 9, 2017

This is a specific case of braille not refreshing when the foreground dialog refreshes. Usually these dialogs have static text or a progress bar that can't be focused but that is updated at intervals.
In the case of downloading NVDA updates, braille keeps showing "Remaining: 0:00:00". Same for the time left.

@LeonarddeR
Copy link
Collaborator

LeonarddeR commented Feb 9, 2017 via email

@dkager
Copy link
Collaborator Author

dkager commented Feb 9, 2017

That is true, see #3258. But this is a scenario where it actually matters and can be reproduced.
It could also be a totally different issue, I'm not sure.

@feerrenrut feerrenrut added the p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority label Feb 9, 2017
@jcsteh
Copy link
Contributor

jcsteh commented Feb 9, 2017

This is discussed in the last few comments of #3258. I'm going to close that issue in favour of this one, since #3258 is quite confusing now.

First, if a progress bar updates, braille does get updated if that progress bar is the current focus/review object (depending on tethering).

Regarding the update dialog, here are the relevant comments from #3258:

@dkager wrote:

Another area where braille isn't refreshed is the NVDA update progress dialog.
...
for braille it makes sense to re-evaluate a dialog's contents when a progress bar changes. Scraping may be more accurate, but it seems safe to assume that an updated progress bar implies something else was also updated.

@jcsteh wrote:

Progress bars themselves don't appear in dialog text. In some cases, there can be other text associated with the progress bar (e.g. the elapsed/remaining times in the NVDA update dialog) and this is often included in dialog text. However, we have no way of knowing that this is the case. Fetching dialog text is very expensive from a performance point of view, so we want to be very sure it's necessary before we do it.

It's worth noting that we can't just recalculate part of dialog text at present; it's all or nothing.

That said, I guess it's reasonable to hope that there might be some text associated with the progress bar in a dialog. If a progress bar changes and has a dialog as a parent (or maybe we could check a small number of ancestors), I guess we could update the dialog's braille region, though we'd probably want to limit it to every 10% or every 5 seconds or something like that.

@jcsteh jcsteh added the p4 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority label Feb 9, 2017
@dkager
Copy link
Collaborator Author

dkager commented Feb 10, 2017

That said, I guess it's reasonable to hope that there might be some text associated with the progress bar in a dialog. If a progress bar changes and has a dialog as a parent (or maybe we could check a small number of ancestors), I guess we could update the dialog's braille region, though we'd probably want to limit it to every 10% or every 5 seconds or something like that.

For the delay we could use whatever the spoken progress bar messages use.
One concern I have is that updating the dialog text should ideally preserve the panning of the braille display. Otherwise you get in a situation where you are frantically scrolling to read something while the updates keep bumping you back to the top.

@dkager
Copy link
Collaborator Author

dkager commented Feb 10, 2017

Unless I'm reading this wrong, braille is already updated whenever a progress bar changes value, see NVDAObjects.behaviors.ProgressBar.event_valueChange.
But braille.braillejHandler.handleUpdate then only updates the region associated with that object rather than the whole buffer.

So should we introduce periodic updates for progress bars and then update the whole buffer if that object has a dialog as ancestor? Or do we update on every value change as is done now?
In other words, is updating less frequently only a performance consideration, or also an attempt not to refresh the braille display too often?

@jcsteh
Copy link
Contributor

jcsteh commented Feb 11, 2017 via email

@dkager
Copy link
Collaborator Author

dkager commented Feb 11, 2017

One final question: should I traverse the ancestors using obj.parent and then its parent, etc? Or is this readily available somewhere, e.g. api.getFocusAncestors()?

@feerrenrut feerrenrut assigned dkager and feerrenrut and unassigned feerrenrut Feb 12, 2017
@dkager
Copy link
Collaborator Author

dkager commented Feb 12, 2017

On second thought, the most logical solution seems to work up from the progress bar until its parent dialog is found, then request an update for that. Working up from the focus could lead to an unrelated dialog being update if a progress bar somewhere else on the screen fires. Or does that make no sense?

Is it expensive to fetch parents? My code currently doesn't have a stop criterion, so it goes all the way to desktop. That's probably not a good idea. Will open a PR for the real reviewin'. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority p4 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority
Projects
None yet
Development

No branches or pull requests

5 participants