nvda
nvda copied to clipboard
Sentence navigation
Link to issue number:
Closes #8518.
Summary of the issue:
Sentence navigation.
Description of user facing changes
-
Alt+Down/UpArrow
now navigates by sentence in editables and in browse mode. - Sentence reconstruction option added to document navigation panel in settings. This defines to what extent sentence navigation can look into adjacent paragraphs when trying to reconstruct a sentence.
- Alsoadded an option to specify non-breaking prefixes for current language.
Description of development approach
- Sentence splitting algorithm is implemented in
documentNavigation\sentenceNavigation.py
. It is generic enough and works with different types of textInfos. I left some comments throughout to explain inner workings, but LMK if more clarification/ explanation is needed. I ended up rewriting the algorithm (compared to SentenceNav add-on) from scratch - now the code should be cleaner and more elegant. - In
editableText.py
I addedspeakCurrentSentence
script - not assigned any default keystroke. Hope that's ok - since SentenceNav users requested that in the past. - In
OffsetsTextInfo
updated_getUnitOffsets()
function to deal withUNIT_SENTENCE
. - In
UIATextInfo
updatedexpand()
andmove()
methods to work withUNIT_SENTENCE
. - In
browseMode.py
updatedscript_collapseOrExpandControl
: if current item is not collapsable, then treat alt+Up/Down as sentence navigation commands. - Existing sentence navigation logic for MSWord is not affected.
Testing strategy:
- Manual testing in:
- Notepad
- Notepad++
- Visual Studio
- VSCode
- Chrome - browse mode and textArea
- Firefox browse mode and TextArea
- Unit tests
Known issues with pull request:
Ready for review. ~~This is a draft, but fully working. Feel free to test or even start reviewing. Things I plan to address in the coming few days:~~
- ~~Figure out how to deal with language-specific non-breaking prefixes.~~
- ~~Add docuemntation~~
- ~~Add unit tests~~
Code Review Checklist:
- [x] Documentation:
- Change log entry
- User Documentation
- Developer / Technical Documentation
- Context sensitive help for GUI changes
- [x] Testing:
- Unit tests
- System (end to end) tests
- Manual testing
- [x] UX of all users considered:
- Speech
- Braille
- Low Vision
- Different web browsers
- Localization in other languages / culture than English
- [x] API is compatible with existing add-ons.
- [x] Security precautions taken.
- PASS: Unit tests.
- Build (for testing PR): https://ci.appveyor.com/api/buildjobs/n0j629p6l7j97var/artifacts/output/nvda_snapshot_pr16384-31584,a1f18e45.exe
- PASS: Translation comments check.
- FAIL: Lint check. See test results for more information.
- PASS: System tests (tags: installer NVDA).
- CI timing (mins): INIT 0.0, INSTALL_START 1.0, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 30.0, TESTSETUP_START 0.0, TESTSETUP_END 0.4, TEST_START 0.0, TEST_END 2.4, FINISH_END 0.2
See test results for failed build of commit a1f18e45a7
I have a technical question regarding how to deal with non-breaking prefixes. Define non-breaking prefix is a word that goes right before period, but that suggests that the period doesn't serve as a sentence breaker. Here are some examples in English:
- Titles like Dr., Mr, Ms, Prof.
- Some abbreviations like i.e., s.t.
- Single capital letters. E.g. "George H. W. Bush"
- As a counterexample, not all abbreviations should be included. For example, "etc." should not be included, since a sentence can end with "etc.". In this case we should prefer to have a false positive rather than a false negative.
Non-breaking prefixes are obviously language-dependent. So far I just hardcoded English non-breaking prefixes, but that obviously needs to be changed before merging. I think we should make non-breaking prefixes to go through translation system to have it translated to all NVDA languages. But I see a few issues here to call out:
- This is not a typical string to translate and it would require some research of the language. I will write a long #translators message explaining what is non-breaking prefixes and what's the expected format.
- That string must be
|
-separated list of valid fixed-length regular expressions. Fixed length requirement is coming from the fact that each regex in the list is packed into a negative look-behind clause, which can only accept fixed length regex. So for exampleS*
would be invalid for NLB. I expect some translators will not be familiar either with regexes in general or this specific requirement, so mistakes will be made. I haven't dealt with NVDA translation system before, so wondering, is there a way to set up a custom validator for a translateable string? If not, then can it be arranged to manually check all incoming translations of the string and invalidate those that will turn out to be invalid? - Another issue is identifying current language. Often times people use NVDA to read context in multiple languages, so perhaps a good idea would be to take the current language of synthesizer and retrieve non-breaking prefixes in that language, instead of default NVDA language. There are two problems here:
- How to map current synth language onto NVDA languages? In the past I remember for Russian some synthesizers return "ru_ru", whereas in NVDA I only see "ru". An obvious solution would be to try to identify language by the first two letters, but then I see some country-specific languages in
user_docs
directory, such asde_CH
. How big deal would it be to ignore such country-specific languages? - What is API call to retrieve a string in another language? I bet it must be possible, but I looked through NVDA source code and didn't find any examples - would be great if someone knows.
- How to map current synth language onto NVDA languages? In the past I remember for Russian some synthesizers return "ru_ru", whereas in NVDA I only see "ru". An obvious solution would be to try to identify language by the first two letters, but then I see some country-specific languages in
- Do we need to create a setting to specify non-breaking prefixes? In the past I've been given feedback that I tend to expose too many settings and it's confusing to the users - so asking here to get community's opinion.
Thanks @mltony,
Based on Sentence-Nav I want to know:
- Will phrase navigation be included here?
- Will there also be a user-defined regular expression for sentence splitting?
- How do we deal with MSWord? There is an option in the Sentence-Nav add-on to override the default sentence navigation of MSWord. It is very useful not only for MSWord but also for Bookworm.
- Will phrase navigation be included here?
No. But it should be very straightforward to add it later provided NVAccess is aligned.
- Will there also be a user-defined regular expression for sentence splitting?
It currently uses the same regex as for text paragraph navigation, which is configurable. But its only a single regex.
- How do we deal with MSWord? There is an option in the Sentence-Nav add-on to override the default sentence navigation of MSWord. It is very useful not only for MSWord but also for Bookworm.
I don't override MSWord sentence navigation here. It should be easy to override later - again as long as NVAccess is aligned.
In general I think getting this PR approved and merged is going to be somewhat challenging because there are many unknowns regarding non-breaking prefixes. So at this point I would like to focus on core sentence navigation, and postpone bells and whistels to later PRs.
@mltony wrote:
Also, not sure how to fix lint error in configSpec.py - line is indeed to long, but trying to break it up leads to invalid config.
At end of line, after two spaces:
# noqa: E501 Breaking this line makes invalid config
@mltony wrote:
2. That string must be `|`-separated list of valid fixed-length regular expressions. [.] I expect some translators will not be familiar either with regexes in general or this specific requirement, so mistakes will be made.
Can you use a tuple of strings, then join them with or bars at runtime?
sentenceSeparators = "|".join((
"string1",
"string2",
"etc.",
))
I don't know how translation for this would work--I imagine you couldn't use normal translatable strings, but I'm not sure. Regardless you must solve that question either way, and this is probably better than trying to make translators juggle a large regex.
Can you use a tuple of strings
How can we send that tuple to translators? If translation system supports tuples of variable length then should be doable, but please show me an example.
@mltony I have no idea. Nor do I know how you'll move a long and fragile regular expression string through the translation process.
Especially with the translation system the way it is now, I know even less about it than I did before.
Apologies for the inapplicable idea.
This is now ready for review.
Hi @mltony I'm getting the following error when trying to do sentence navigation using the latest build.
IO - speech.speech.speak (07:12:27.736) - MainThread (11956):
Speaking ['日志片段开始点已标记,再按一次复制到剪贴板']
IO - inputCore.InputManager.executeGesture (07:12:28.564) - winInputHook (3860):
Input: kb(laptop):alt+downArrow
ERROR - scriptHandler.executeScript (07:12:28.582) - MainThread (11956):
error executing script: <bound method BrowseModeDocumentTreeInterceptor.script_collapseOrExpandControl of <NVDAObjects.IAccessible.chromium.ChromeVBuf object at 0x0B435150>> with gesture 'Alt+下光标'
Traceback (most recent call last):
File "scriptHandler.pyc", line 295, in executeScript
File "browseMode.pyc", line 1689, in script_collapseOrExpandControl
File "cursorManager.pyc", line 157, in _caretMovementScriptHelper
File "textInfos\offsets.pyc", line 538, in expand
File "virtualBuffers\__init__.pyc", line 405, in _getUnitOffsets
File "textInfos\offsets.pyc", line 512, in _getUnitOffsets
File "documentNavigation\sentenceNavigation.pyc", line 210, in __init__
File "documentNavigation\sentenceNavigation.pyc", line 145, in getSentenceStopRegex
KeyError: '1,3'
IO - inputCore.InputManager.executeGesture (07:12:29.599) - winInputHook (3860):
Input: kb(laptop):control+shift+NVDA+f1
There seems to be no language difference, I used the English and Chinese versions of the NVDA User Guide for testing.
@cary-rowen, try resetting your value for text paragraph regex. It should work with the default value.
- PASS: Translation comments check.
- PASS: Unit tests.
- FAIL: Lint check. See test results for more information.
- FAIL: System tests (tags: installer NVDA). See test results for more information.
- Build (for testing PR): https://ci.appveyor.com/api/buildjobs/c5vaeu214msswiba/artifacts/output/nvda_snapshot_pr16384-31654,24be7c84.exe
- CI timing (mins): INIT 0.0, INSTALL_START 1.1, INSTALL_END 1.0, BUILD_START 0.0, BUILD_END 31.1, TESTSETUP_START 0.0, TESTSETUP_END 0.4, TEST_START 0.0, TEST_END 2.4, FINISH_END 0.2
See test results for failed build of commit 24be7c8425
@mltony did you test this in browse mode in pdf documents, with Adobe reader, outlook messages in MS outlook and browse mode in MS Word? Does it give an error in browse mode in MS Excel? Or did you implement a message that this is not supported there?
@mltony Re non breaking prefixes, it might be worth to add all so called "period words" in a database first and import them from there. Maybe there could be a file similar to symbols.dic called periodwords.dic. At least with such a file you could pass it to the translation system and we would get all of the international prefixes translated while translators can also add their language specific period words to their own language's periodwords.dic file. Ofcourse the correct sentence navigation would depend on translation work.
@Adriani90
- Browse mode in browsers: tested, works fine
- PDF reader browse mode: tested, works fine
- MSWord and Outlook: legacy behavior overrides functionality oof my PR, so no change here
- In MS Excel alt+Down arrow seems to trigger expand action, so seems no regressions here. Re symbols file - that's a reasonable idea. I guess I'll just wait for a few days before working on that just in case anyone else has objections or better ideas.
- PASS: Translation comments check.
- PASS: Unit tests.
- FAIL: Lint check. See test results for more information.
- PASS: System tests (tags: installer NVDA).
- Build (for testing PR): https://ci.appveyor.com/api/buildjobs/4rd4tj4u94poj6kt/artifacts/output/nvda_snapshot_pr16384-31656,ee7ed21a.exe
- CI timing (mins): INIT 0.0, INSTALL_START 1.1, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 29.2, TESTSETUP_START 0.0, TESTSETUP_END 0.4, TEST_START 0.0, TEST_END 2.3, FINISH_END 0.2
See test results for failed build of commit ee7ed21aad
Hi @mltony
Thank you for your efforts on this. I will continue to pay attention to this feature and provide the following test work for your reference.
Test in Windows 10 Notepad:
Steps
- Write the following Chinese content : "今天天气很好。我想去爬山。" These are two sentences containing two periods.
- Press Alt+Up/DownArrow.
- Add a space after the first period.
- Place the caret at the beginning of the text and press Alt+Downarrow repeatedly.
Actual behavior:
- Unable to navigate by sentence when performing step (2).
- NVDA reports the second sentence when performing step (4).
expected behavior
- Perform step 2 to navigate by sentence. No matter whether there is a space after the period, because in Chinese writing, there is generally no space after the period.
- When performing step (4), "blank" should be reported, consistent with the behavior in MS Word.
Test in NVDA Speech Viewer or Bookworm:
Sentence navigation is not working. If you use the Sentence-nav add-on, you need to enable overriding MS Word's sentence navigation behavior in settings. So, as far as this PR is concerned, I think we should have a solution for similar situations like the Bookworm rich text control.
Thanks
@cary-rowen
- Re step 2: fixed regexp, try again.
- Also did I get Chinese question mark and exclamation mark correctly in the regexp?
- Re step 4: This one is complicated. So I implement in this PR functions like
UIATextInfo.expand(UNIT_SENTENCE)
. If you want NVDA to report blank in this case, that means that when the cursor is at the last position of the document, it should expand into empty sentence. I actually designed sentences to be non-empty. Trying to redesign just to account for this case is going to be difficult and any attempt to redesign is likely to cause bugs in many other edge cases. Trying to analyze behavior with different textInfo units, I played withUNIT_WORD
. If you type: "Hello world" without quotes into Notepad on Windows 11 and try to press control+rightArrow repeatedly, the cursor would stay at the word "world" and not find a blank word in the end. Although in notepad++ it does find the blank word. So behavior is not consistent here. So given the fact that there is already some inconsistency present with other textInfo units and given how hard and hacky it's going to be to add fake empty sentence at the end, I would ask whether it's even worth it to tackle this issue. WDYT? - Re NVDA Speech Viewer : acctually it works on my side. It doesn't appear to be using MSWord control inside.
- Re Bookworm: I prefer to postpone this until after this PR is merged. I'm worried that the more features we add here, the more likely we'll get bogged down with discussions on implementation details.
- PASS: Translation comments check.
- PASS: Unit tests.
- FAIL: Lint check. See test results for more information.
- PASS: System tests (tags: installer NVDA).
- Build (for testing PR): https://ci.appveyor.com/api/buildjobs/3n1p8rlwadg4r911/artifacts/output/nvda_snapshot_pr16384-31657,cf90f526.exe
- CI timing (mins): INIT 0.0, INSTALL_START 1.1, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 28.9, TESTSETUP_START 0.0, TESTSETUP_END 0.4, TEST_START 0.0, TEST_END 2.3, FINISH_END 0.2
See test results for failed build of commit cf90f52694
Re step 2: fixed regexp, try again. Yes, sentence navigation now works as expected.
Also did I get Chinese question mark and exclamation mark correctly in the regexp?
Yes.
So given the fact that there is already some inconsistency present with other textInfo units and given how hard and hacky it's going to be to add fake empty sentence at the end, I would ask whether it's even worth it to tackle this issue. WDYT?
OK, regarding this behavior, if it is very difficult then I don't think there is a need to put too much effort into it.
If you type: "Hello world" without quotes into Notepad on Windows 11 and try to press control+rightArrow repeatedly, the cursor would stay at the word "world" and not find a blank word in the end.
Yes, I reproduce this on Notepad on Windows 10 as well.
As an improvement on step 4, I think another behavior might be better: NVDA will report the last sentence repeatedly when we press Alt+DownArrow repeatedly, And the caret has been moved to the end of the last sentence. This means that if the user presses Alt+UpArrow once you will hear NVDA continue reporting the last sentence.
Personally, I would like it to report the first sentence, ie: when there are no more sentences our caret should be placed at the beginning of the last sentence, no matter how many times I press Alt+DownArrow.
This is consistent with the paragraph navigation pattern in document navigation. This is also consistent with the behavior of the sentence-nav add-on, and users of the sentence navigation add-on will have a consistent experience as before.
Re NVDA Speech Viewer : acctually it works on my side. It doesn't appear to be using MSWord control inside.
That's weird, can you confirm this again, it seems to me that Speech Viewer behaves the same as Bookworm, i.e. sentence navigation doesn't work at all.
The test I performed was with Speech Viewer open: I asked NVDA to report the Chinese sentences given in the comments above. Then try sentence navigation in Speech Viewer. NVDA seems to treat the entire Speech Viewer as one sentence.
Hi @mltony
While reminding you not to forget the above comment, I thought of another question.
I'm wondering, is it necessary that we share the regex option with paragraph navigation? The current regular expression options are very complex and may be completely uncustomizable by novice users. Even someone like me who can read regular expressions may need to spend a lot of time understanding it.
Is there a way we can make the regex options more understandable? If not, that’s no problem. Our goal is to provide users with an experience that they can use right out of the box, rather than requiring users to modify it before they can use it.
Thanks
I have not reviewed this PR.
But, on contrary to what I wrote earlier, I understand now why the regexp for text paragraph navigation and the one for sentence navigation may be separately configurable. For now, the criterion to identify a text paragraph is if it contains a sentence. However, we may imagine in the future that we use a different criterion (even if I do not have it in mind now).
Of course, since we do not have another criterion now, we may also wait for a real need before duplicating these fields. @cary-rowen, have you a real use case / interest to configure differently text paragraph navigation and sentence navigation?
Re the possibility to better understand the regexp, we may use a verbose regexp in the field, rather than a normal one. Verbose regexps ignore comments, spaces and newline characters (unless escaped). We should however take into account that verbose regexps are less known than normal ones; I have discovered this option only some weeks ago...
I agree in case of sentence navigation, the carret should always stay at the beginning of the sentence as it does with paragraph navigation when the paragraph is handled by NVDA in document navigation settings, no matter if it is the first or the last sentence or any senence in the middle. Instead of repeating the first or the last sentence, there could be a message reporting "no next sentence" or "no previous sentence". This is also consistent with NVDA paragraph navigation.
@CyrilleB79
Have you a real use case / interest to configure differently text paragraph navigation and sentence navigation?
No, I haven't yet. I just have the above thoughts from the perspective of ease of understanding. However, we can focus on the core implementation of this PR, and put other things into separate PRs to discuss or implement.
Instead of repeating the first or the last sentence, there could be a message reporting "no next sentence" or "no previous sentence". This is also consistent with NVDA paragraph navigation.
To be honest, I don’t really like using speech to indicate boundaries. At this point, NVDA's paragraph navigation is inconsistent with other behaviors, such as paragraph navigation in Word.
I prefer to use beeps or play sound to indicate boundaries.
A similar request has already been made by #13612.
By the way, sentence-nav add-on is better and there are also beep when navigating from one paragraph to another.
There is no consistency in paragraph navigation between apps. Libreoffice behaves differently compared to ms Word for example. Nvda paragraph navigation is the most consistent paragraph handling we have so far.Von meinem iPhone gesendetAm 24.04.2024 um 17:36 schrieb Rowen @.***>:
Instead of repeating the first or the last sentence, there could be a message reporting "no next sentence" or "no previous sentence". This is also consistent with NVDA paragraph navigation.
To be honest, I don’t really like using speech to indicate boundaries. At this point, NVDA's paragraph navigation is inconsistent with other behaviors, such as paragraph navigation in Word. I prefer to use beeps or play sound to indicate boundaries. A similar request has already been made by #13612.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>
Indicating boundaries by sounds is a valid issue but out of scope for this PR I guess.Von meinem iPhone gesendetAm 24.04.2024 um 17:36 schrieb Rowen @.***>:
Instead of repeating the first or the last sentence, there could be a message reporting "no next sentence" or "no previous sentence". This is also consistent with NVDA paragraph navigation.
To be honest, I don’t really like using speech to indicate boundaries. At this point, NVDA's paragraph navigation is inconsistent with other behaviors, such as paragraph navigation in Word. I prefer to use beeps or play sound to indicate boundaries. A similar request has already been made by #13612.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>
Nvda paragraph navigation is the most consistent paragraph handling we have so far.
You confuse me that this scheme has never been done elsewhere except for what NVDA implemented in #13798 - using speech to indicate the last and first paragraphs. Where does your so-called "consistency" come from in this case?
Indicating boundaries by sounds is a valid issue but out of scope for this PR I guess.
What we have been discussing is how to prompt the user when navigating to the first and last sentences during sentence navigation. Speech or play sound are the two options under this issue.
Like it was done in #13798, so I don't understand even more what you mean by out of scope.
What I wanted to say is that when paragraphs are handled by applications, in some applications NVDA repeats the first or last paragraph, in some applications NVDA is silent, in some applications a windows bing sound is heared, etc. NVDA has a good basis for reporting the boundaries, and this can ofcourse be improved with sounds or what ever, but this does not affect only paragraphs and is in my view at least still out of scope for this PR.
For now The boundary reporting for paragraphs in NVDA is consistent all over the place when paragraphs are handled by NVDA. Tony can make use of the internal paragraph handling to design the boundaries for documents and if we improve the boundary reporting, then this should be done for both in a separate PR. But this affects also other navigation paterns handled by NVDA such as line by line navigation, every kind of review cursor or object navigation that reaches the beginning or the end of an element etc.