-
-
Notifications
You must be signed in to change notification settings - Fork 662
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
Say all, move caret per word instead of per line/sentence, taken over after abandoned 9937 #11658
Say all, move caret per word instead of per line/sentence, taken over after abandoned 9937 #11658
Conversation
…ultiple sentences
Fixed typo Co-Authored-By: Derek Riemer <[email protected]>
Co-Authored-By: Derek Riemer <[email protected]>
…is code from 2017 and later
Could you elaborate on what was changed since #9937? |
Nothing yet. I have created this PR to notice that I take it over, it is currently only the copy of #9937. I will try to create a commit in a day or two. |
I have updated the description with a link to the diff with BabbageCom/sayAll |
@dawidpieper In the Orca screen reader we intent to do it too but we don't implement it like this because we thought it could lead to mistake. If you move the caret on a word you couldn't avoid to have an impact on the webpage, maybe it could trigger an unpredictable action. For the magnifier part, we use the speech progress notifications given by the speech synthesizer to track words. As we mimic IA2 for it I assume we could do the same into NVDA on Windows. |
@alexarnaud I have been looking a bit into speech synthesizers and their events, specifically for this problem. Maybe I am overlooking something. Does NVDA have an universal event for speech progress? |
@FalkoBabbage I think having synth support for this wouldn't add much additional value. It's actually more that changing the caret's position too often results in performance issues that themselves cause stuttering audio, at least for espeak. |
I haven't looked into this very carefully, but based just on a PR description it seems to me the weak point of this and the main reason why this is being made configurable is the fact that updating the caret position can be expensive or even undesirable in some situations. Has it been considered to just change location of the highlight based on the location of the word currently being read (something similar can be probably done for braille though I'm completely unfamiliar with this part of the code) and change positionn of the caret only when say all is stopped to move it to the current word being read for speech users? |
@lukaszgo1 For magnification extensions it should be sufficient to have some event / notification that a new word is spoken, so the caret does not necessarily have to move for each word. I was hoping to get such an event from the synthesizer. Whenever that events fires I can simply move my "magnification rectangle" to the next word. As far as I understand, the caret is moved based on a time guess in this PR. If we could instead couple this to an event of the synthesizer then we would have a better indication of what word is spoken as well. However, I am very inexperienced with speech synthesizers and events so I am not sure what I just suggested is a good idea. |
This will be severely broken by #12251 as soon as it is merged. |
Most of the changes are just code being moved and renamed. I'm happy to help merge in any of the changes. |
Closing this PR as abandoned. Note: no action has been taken since #9937, which was the initial PR this attempted to take over. |
Note
This pull request is taken over by me after #9937 was abandoned. Diff with BabbageCom/sayAll
Below I paste the original description by @LeonarddeR
Link to issue number:
Fixes #9179
Fixes #3287
Summary of the issue:
When using say all, the caret moves per sentence or per line, not per word. This is not very helpful when using magnification or braille to track the current position of say all. More importantly though, when reviewing text with say all, exiting say all makes you end up at the start of the line or sentence, not at the current word's position.
Say all also doesn't follow in browse mode.
Description of how this pull request fixes the issue:
Find white space using regex, and bring the caret to it accordingly. This avoids fetching text info per word, which can be pretty expensive.
This option can be found in the speech settings: "Increase caret updates during say all"
Testing performed:
Tested in Word, Notepad and browse mode that the caret follows read words. Note that there is a minimal 0.5 seconds interval between every caret update to avoid the caret going mad on higher speechrates.
Known issues with pull request:
Change log entry: