nvda
nvda copied to clipboard
Improve Paragraph Navigation -- Add Normal and Block Style
Link to issue number:
Closes #13797
Summary of the issue:
New feature: add support for both normal and block style paragraph navigation for editors which do not support such navigation natively. Also allows paragraph navigation in web browsers.
Description of how this pull request fixes the issue:
Offers improved paragraph navigation, as well as the option to use an editor's native paragraph navigation as before.
Testing strategy:
You'll find settings to control this feature in a new settings panel titled Document Navigation. The first option, Handled by application, causes NVDA to perform exactly as it does without this patch. The second option, Normal style, adds normal paragraph navigation (paragraphs delimited by Enter) to editors which do not support this functionality. The third option, Block style, is similar to the second option, but moves to paragraphs delimited by at least one blank line. The default is the first option, Handled by application, which acts as before, preventing users from experiencing unexpected changes in functionality. If it is impossible to find or move to the next or previous paragraph when either of the latter two options is selected, NVDA acts as if the Handled by application option is selected, and performs as before.
Known issues with pull request:
Microsoft Word and Outlook are only supported when using UIA. Only searches forward or backward a maximum of 250 lines to keep performance reasonable.
Change log entries:
New features
- Adds support for both normal and block style paragraph navigation for use with editors that do not support paragraph navigation natively.
Changes Bug fixes For Developers
Code Review Checklist:
- [x] Pull Request description:
- description is up to date
- change log entries
- [x] Testing:
- Unit tests
- System (end to end) tests
- Manual testing
- [x] API is compatible with existing add-ons.
- [ ] Documentation:
- User Documentation
- Developer / Technical Documentation
- Context sensitive help for GUI changes
- [x] UX of all users considered:
- Speech
- Braille
- Low Vision
- Different web browsers
- Localization in other languages / culture than English
@rob-aph - what feedback are you seeking specifically?
I tested this and it looks good, For 'Block Style': If in an editor with a lot of text NVDA takes some time to retrieve empty lines, NVDA will hang during the process.
Is this an acceptable feature? Is it ready to go? These kinds of things.
Both normal and block styles give up after searching 250 lines. Can you tell me which editor you were using which made NVDA hang?
@rob-aph Hi, I opened an epub book using bookworm, The following is the repo of bookworm: https://github.com/blindpandas/bookworm
Here is the book file used for testing(You need to unzip the file to get the epub file): test.zip
Can you please try again with my latest commit? I optimized a bit, and stopped trying to read more than the first 250 lines of paragraphs. I believe the paragraphs I detected in your book were much larger than I expected, and this caused the problem.
Hi, @rob-aph
I tested the latest commit and there is a noticeable improvement in NVDA hang time, but there is still a lag in "block navigation". I would also like to hear other people's opinions, Is this acceptable?
Thanks
This PR will also need to include a description of the option in the user guide. https://www.nvaccess.org/files/nvda/releases/2022.1/documentation/userGuide.html#NVDASettings
@rob-aph Can you add an unassigned gesture to this feature to toggle these styles?
Sean,
I have tested with Notepad, Wordpad, Visual Studio Code, Notepad++, and TextPad. It doesn’t work with TextPad, but NVDA doesn’t work so well with TextPad anyway. There is an issue with VSCode where if using normal paragraph style, VSCode will wrap around to the top of the file if you attempt to move twice from the second to the last paragraph in the file to the next paragraph. I suspect this has something to do with the TextInfo used by VSCode, but I plan to see if I can improve the situation.
As I recall, the issue with Word was a sluggishness issue – something would take too long, even in small files, and NVDA would eventually fail the call. Actually, it was more Outlook’s reading pane I was trying to get to work, but I believe Outlook and Word use the same TextInfo. There may be more I can do with Word when I become more knowledgeable on the peculiarities of it’s TextInfo – I haven’t entirely given up.
I also recall this PR working in Visual Studio 2019 in the past, but I see now it doesn’t. I believe the issue may be that I require State.MULTILINE, and this value is not set in the focus object for VS 2019. Basically, I’m trying to avoid these commands getting run in single line edit fields, etc.
I would be happy to advertise this PR and let users test it, but I have no idea how to get the URL for it’s AppVeyor build, or how long the build remains available. Can you point me in the right direction?
Sure, no problem. Good idea.
I would be happy to advertise this PR and let users test it, but I have no idea how to get the URL for it’s AppVeyor build, or how long the build remains available. Can you point me in the right direction?
You can check the AppVeyor CI/CD build from the NVDA AppVeyor history or from the Status Checks at the bottom of the PR.
You will find the exe build in the artefacts of the build for d2eb59d.
https://ci.appveyor.com/api/buildjobs/aln761e4f3adcufl/artifacts/output%2Fnvda_snapshot_pr13798-25707%2C489f8c97.exe
@seanbudd Thanks for the AppVeyor info. I'll advertize this after a few more itterations -- attempt at paragraph support in Outlook, etc.
@seanbudd I have tried harder to get this to work in Word / Outlook. You asked if anything was blocking this, and I can now respond yes. It appears very inefficient to move by line in the TextInfo used by Word / Outlook. In fact, it can sometimes hang for reasons I currently do not understand. I worked around one issue, the fact that attempting to move to the next line when there is not another line returns 1 indicating that movement occurred, when in fact, nothing changed -- it should return 0.
I should add that I only have access to Windows 10, so I have no idea of the performance on Windows 11. Perhaps it is better.
I have updated my branch to add a demo of Word / Outlook support. I find that even when it doesn't hang, navigating by block paragraphs is extremely slow, even in very short messages.
If you wish to try this in Word, you will have to switch to Browse mode, as I have not hooked up scripts for Focus mode yet, as this is really a test / demo. It works right out of the box in read-only Outlook messages.
- PASS: Translation comments check.
- PASS: Unit tests.
- FAIL: Lint check. See test results for more information.
- FAIL: System tests. See test results for more information.
- Build (for testing PR): https://ci.appveyor.com/api/buildjobs/tas67ulu006hbbf6/artifacts/output/nvda_snapshot_pr13798-25846,f3ab6400.exe
See test results for failed build of commit f3ab640057
@seanbudd Thanks. I think I know how to proceed now.
@cary-rowen Unassigned gesture to toggle through paragraph navigation styles has been added.
I tested this feature. pretty good.
- PASS: Translation comments check.
- PASS: Unit tests.
- PASS: Lint check.
- FAIL: System tests. See test results for more information.
- Build (for testing PR): https://ci.appveyor.com/api/buildjobs/t5et50j3n1maqj0u/artifacts/output/nvda_snapshot_pr13798-26047,2466e5d4.exe
See test results for failed build of commit 2466e5d4b9
@seanbudd Let me ask, if I rebase/squash my commits from the command line, does that invalidate the pull request? I would like to clean things up before requesting a review. Similarly, does rebaseing on to the latest master branch cause issues?
- PASS: Translation comments check.
- PASS: Unit tests.
- PASS: Lint check.
- FAIL: System tests. See test results for more information.
- Build (for testing PR): https://ci.appveyor.com/api/buildjobs/i5na2b45gcot6v7g/artifacts/output/nvda_snapshot_pr13798-26056,e9d77ca3.exe
See test results for failed build of commit e9d77ca368
Squashing/rebasing will reset the review - currently I have the option to review the changes since my last review. If the last commit which was reviewed is lost, then I will have to re-review for scratch. If you keep the commit 34b118e and only change newer commits this shouldn't be an issue. The pull request will eventually be squash merged into master.
Merging in the latest master won't cause issues.
- PASS: Translation comments check.
- PASS: Unit tests.
- PASS: Lint check.
- FAIL: System tests. See test results for more information.
- Build (for testing PR): https://ci.appveyor.com/api/buildjobs/55nyvng03ce5xwlk/artifacts/output/nvda_snapshot_pr13798-26101,053ac179.exe
See test results for failed build of commit 053ac179de
@rob-aph Could you also consider this comment
- FAIL: System tests. See test results for more information.
- PASS: Lint check.
- PASS: Unit tests.
- PASS: Translation comments check.
- Build (for testing PR): https://ci.appveyor.com/api/buildjobs/u64w2prdkq9x2vfh/artifacts/output/nvda_snapshot_pr13798-26392,a82b905e.exe
See test results for failed build of commit a82b905e6d
@seanbudd Continuing, it appears that _validateConfig_featureFlag
is not expecting anything besides a BoolFlag
. I get this error when attempting to update the config setting, and when NVDA switches app profiles. Error follows:
Traceback (most recent call last):
File "scriptHandler.py", line 289, in executeScript
script(gesture)
File "globalCommands.py", line 3687, in script_toggleParagraphStyle
config.conf["documentNavigation"]["paragraphStyle"] = newFlag
File "config\__init__.py", line 1237, in __setitem__
val = self.manager.validator.check(spec, val)
File "c:\src\nvda\.venv\lib\site-packages\configobj\validate.py", line 615, in check
return self._check_value(value, fun_name, fun_args, fun_kwargs)
File "c:\src\nvda\.venv\lib\site-packages\configobj\validate.py", line 647, in _check_value
return fun(value, *fun_args, **fun_kwargs)
File "config\featureFlag.py", line 127, in _validateConfig_featureFlag
'Expected a featureFlag value in the form of a string. EG "disabled", "enabled", or "default".'
configobj.validate.ValidateError: Expected a featureFlag value in the form of a string. EG "disabled", "enabled", or "default". Got <enum 'ParagraphNavigationFlag'> with value: ParagraphNavigationFlag.DEFAULT instead.
- PASS: Translation comments check.
- PASS: Unit tests.
- PASS: Lint check.
- FAIL: System tests. See test results for more information.
- Build (for testing PR): https://ci.appveyor.com/api/buildjobs/tnap1x83ar5qt6p4/artifacts/output/nvda_snapshot_pr13798-26429,7612a6e6.exe
See test results for failed build of commit 7612a6e6e5
This still needs a section in the user guide
- 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/51uf9uwi71bn9gpn/artifacts/output/nvda_snapshot_pr13798-26687,7e790efa.exe
- CI timing (mins): INIT 0.0, INSTALL_START 0.9, INSTALL_END 0.9, BUILD_START 0.0, BUILD_END 22.8, TESTSETUP_START 0.0, TESTSETUP_END 0.3, TEST_START 0.0, TEST_END 13.8, FINISH_END 0.1
See test results for failed build of commit 7e790efa89
- PASS: Translation comments check.
- PASS: Unit tests.
- PASS: Lint check.
- FAIL: System tests (tags: installer NVDA). See test results for more information.
- Build (for testing PR): https://ci.appveyor.com/api/buildjobs/a73p32bqtlvl7oui/artifacts/output/nvda_snapshot_pr13798-26692,bdf34e71.exe
- CI timing (mins): INIT 0.0, INSTALL_START 0.8, INSTALL_END 0.8, BUILD_START 0.0, BUILD_END 22.9, TESTSETUP_START 0.0, TESTSETUP_END 0.2, TEST_START 0.0, TEST_END 12.9, FINISH_END 0.1
See test results for failed build of commit bdf34e71bd