Re-Introduce Table SayAll commands#14070
Merged
Merged
Conversation
Contributor
Author
|
cc: @lukaszgo1, @Mohamed00 |
5 tasks
seanbudd
approved these changes
Aug 26, 2022
|
Can confirm that Bookworm's page turning works properly. |
feerrenrut
approved these changes
Aug 29, 2022
Contributor
@mltony I think there are a couple of things going wrong here. The example assumes the existence of a test named "starts", since wildcards are accepted this could also be "starts*". The ellipsis was intended to mean "and any other arguments". Happy for a PR to clarify. |
This was referenced Aug 31, 2022
CyrilleB79
added a commit
to CyrilleB79/nvda
that referenced
this pull request
Oct 25, 2022
6 tasks
seanbudd
pushed a commit
that referenced
this pull request
Oct 26, 2022
Summary of the issue: In MSHtml container, e.g. NVDA's browseable message, the table edge is not reported. The following error is found instead: ERROR - scriptHandler.executeScript (23:32:09.385) - MainThread (6444): error executing script: <bound method DocumentWithTableNavigation.script_nextColumn of <virtualBuffers.MSHTML.MSHTML object at 0x0953F690>> with gesture 'alt+contrôle+flèche droite' Traceback (most recent call last): File "scriptHandler.pyc", line 289, in executeScript File "documentBase.pyc", line 480, in script_nextColumn File "documentBase.pyc", line 417, in _tableMovementScriptHelper File "documentBase.pyc", line 362, in _tableFindNewCell File "virtualBuffers\__init__.pyc", line 674, in _getNearestTableCell TypeError: cannot unpack non-iterable _TableCell object Description of user facing changes No more error reported at edge and the edge is reported. Description of development approach The dataclass documentBase._TableCell was added in #14070. More specifically, DocumentWithTableNavigation._getTableCellCoords now returns _TableCell. But some usages of the return of this function have been forgotten. This PR fixes this.
6 tasks
michaelDCurran
pushed a commit
that referenced
this pull request
Jan 29, 2023
6 tasks
seanbudd
pushed a commit
that referenced
this pull request
Mar 22, 2023
Fixes #14390 Summary of the issue: With the merging of pr #14070 which added sayall in tables, SayAll in Kindle for PC would fail to continue reading after turning the page. Technical: Some of the code in the nextLine method in sayAll was refacted into a nextLineImpl method. However, if nextLineImpl returned false, finish would be called and nextLIne would return. This would be correct for handling the case where there was no more text, or table cells, but not the case where the page has just been turned. In fact, text sayAll already called finish when there was no more text, so calling finish again was useless in the base case, but for page turns caused sayAll to abort prematurely after the page was turned. Description of user facing changes NVDA no longer fails to keep reading with sayAll after crossing a page boundary. Description of development approach In sayAll's nextLine method, removed the call to self.finish when nextLineImpl returns False, but ensured that table sayAll's nextLineImpl does call finish itself if there is no more table cells. In other words, the self.finish call has been moved into the specific nextLineImpl method where it is needed, rather than running more broadly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to issue number:
Fixes #13469.
Fixes #13927.
Summary of the issue:
Previous PR #13956 broke sayAll functionality in BookWorm (#13927) and therefore was reverted.
This PR undoes reverting, in other words it reintroduces table sayAll commands. It also contains a minor change that fixes sayAll in BookWorm.
Description of user facing changes
Reintroduces table sayAll commands.
Description of development approach
In my original PR #13956 I did a minor refactoring of SayAll classes, in order to avoid code reduplication and have a nice class inheritance. In order to achieve this I slightly rearranged calls to textInfo. This didn't affect functionality anywhere except for page turn detector in BookWorm.
Reverting to the original order of TextInfo calls at the cost of slightly less elegant code.
Testing strategy:
Tested manually with BookWorm and confirmed that pages turn successfully.
Tested on tables in browsers.
Known issues with pull request:
No system testing was performed as
runsystemtests.batscript appears to be broken. When trying to run command mentioned as example from https://github.com/nvaccess/nvda/tree/master/tests/system on clean master:Change log entries:
New features
NVDA+control+alt+downArrowNVDA+control+alt+rightArrowNVDA+control+alt+upArrowNVDA+control+alt+leftArrowChanges
Bug fixes
For Developers
Code Review Checklist: