Skip to content

Prevent infinite recursion for unspeakable chars#11865

Merged
feerrenrut merged 2 commits into
masterfrom
freezeWithCharacter-i11752
Nov 26, 2020
Merged

Prevent infinite recursion for unspeakable chars#11865
feerrenrut merged 2 commits into
masterfrom
freezeWithCharacter-i11752

Conversation

@feerrenrut

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #11752

Summary of the issue:

When reading characters that do not have any description in NVDA, a 'no content' utterance can be produced. In this case the empty string is the last 'command' in the sequence passed to ensureEndUtterance, resulting in the sequence not being given an indexCommand or an EndUtteranceCommand. The indexCommand is required for the cancellable speech processing, to
be able to track which utterance a cancellableSpeechCommand belongs to.

Description of how this pull request fixes the issue:

  • Adds tests for this case.
  • Prevents an empty string being interpreted as a lack of speech commands.
  • Fixes a similar possible error for an optional index.

Testing performed:

Known issues with pull request:

  • This is allowing the speech module to continue to send the speech sequences with no useful speech. Fixing this would be more complicated / risky.

Change log entry:

None

Fixes #11752
When reading characters that do not have any description in NVDA, a 'no content'
utterance can be produced. In this case the empty string is the last
'command' in the sequence passed to 'ensureEndUtterance', resulting in
the sequence not being given an indexCommand or an EndUtteranceCommand.
The indexCommand is required for the cancellable speech processing, to
be able to track which utterance a cancellableSpeechCommand belongs to.
zero is a might be a valid index. Only no index (None) shoudl take this path

@michaelDCurran michaelDCurran left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am guessing without this fix, some kind of exception (perhaps infinite recursion) would have been raised within the unit test? That is, not just perhaps a missing expected index. Fix makes sense though.

@feerrenrut

Copy link
Copy Markdown
Contributor Author

some kind of exception (perhaps infinite recursion) would have been raised within the unit test?

Yes, I wrote the test first to reproduce the issue. It failed with something like "stack limit reached".
I then fixed the infinite recursion part of this (by removing the problematic sequence that was missing an indexCommand).
Then I wanted to understand why it was missing the indexCommand, and found I could fix that easily.

@feerrenrut feerrenrut merged commit 82c6835 into master Nov 26, 2020
@feerrenrut feerrenrut deleted the freezeWithCharacter-i11752 branch November 26, 2020 05:10
@nvaccessAuto nvaccessAuto added this to the 2020.4 milestone Nov 26, 2020
feerrenrut added a commit that referenced this pull request Dec 1, 2020
Fix an issue introduced with #11865, which incorrectly used the return value of _checkForCancellations
to indicate error/success, rather than utterance canceled (correctly) or no cancellations found.

Instead, an exception is raised when the method can not complete, and the error is logged in this case.
feerrenrut added a commit that referenced this pull request Dec 4, 2020
Fix an issue introduced with #11865, which incorrectly used the return value of _checkForCancellations
to indicate error/success, rather than utterance canceled (correctly) or no cancellations found.

Instead, an exception is raised when the method can not complete, and the error is logged in this case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cancelable speech. NVDA freezes with this character

3 participants