From 05af4aff3f0a7e9f331928902e9881efaafcfc05 Mon Sep 17 00:00:00 2001 From: Reef Turner Date: Wed, 25 Nov 2020 15:45:05 +0800 Subject: [PATCH 1/2] Prevent infinite recursion for unspeakable chars 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. --- source/speech/manager.py | 9 +++++-- tests/unit/test_speechManager/__init__.py | 30 ++++++++++++++++++++++- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/source/speech/manager.py b/source/speech/manager.py index 42fd3622137..f31d8666d2d 100644 --- a/source/speech/manager.py +++ b/source/speech/manager.py @@ -333,7 +333,7 @@ def ensureEndUtterance(seq: SpeechSequence): else: lastOutSeq = outSeqs[-1] if outSeqs else None lastCommand = lastOutSeq[-1] if lastOutSeq else None - if not lastCommand or isinstance(lastCommand, (EndUtteranceCommand, ConfigProfileTriggerCommand)): + if lastCommand is None or isinstance(lastCommand, (EndUtteranceCommand, ConfigProfileTriggerCommand)): # It doesn't make sense to start with or repeat EndUtteranceCommands. # We also don't want an EndUtteranceCommand immediately after a ConfigProfileTriggerCommand. return @@ -458,13 +458,18 @@ def _buildNextUtterance(self): # apply any parameters changed before the preemption. params = self._curPriQueue.paramTracker.getChanged() utterance.extend(params) - for seq in self._curPriQueue.pendingSequences: + lastSequenceIndexAddedToUtterance = None + for seqIndex, seq in enumerate(self._curPriQueue.pendingSequences): if isinstance(seq[0], EndUtteranceCommand): # The utterance ends here. break utterance.extend(seq) + lastSequenceIndexAddedToUtterance = seqIndex # if any items are cancelled, cancel the whole utterance. if utterance and not self._checkForCancellations(utterance): + log.error(f"Checking for cancellations failed, cancelling sequence: {utterance}") + # Avoid infinite recursion by removing the problematic sequences: + del self._curPriQueue.pendingSequences[:lastSequenceIndexAddedToUtterance + 1] return self._buildNextUtterance() return utterance diff --git a/tests/unit/test_speechManager/__init__.py b/tests/unit/test_speechManager/__init__.py index e0563cfc237..21c3b968d75 100644 --- a/tests/unit/test_speechManager/__init__.py +++ b/tests/unit/test_speechManager/__init__.py @@ -1139,12 +1139,15 @@ def setUp(self): config.conf['featureFlag']['cancelExpiredFocusSpeech'] = 1 # yes -class Test_pr11651(unittest.TestCase): +class RegressionTests(unittest.TestCase): + """Tests to prevent regressions after issues are fixed. + """ def test_redundantSequenceAfterEndUtterance(self): """ Tests that redundant param change and index commands are not emitted as an extra utterance when the preceeding utterance contained param change commands and an EndUtterance command. + See PR #11651 E.g. speaking a character. """ smi = SpeechManagerInteractions(self) @@ -1168,3 +1171,28 @@ def test_redundantSequenceAfterEndUtterance(self): smi.indexReached(1) smi.doneSpeaking() smi.pumpAll() + + def test_nonSpokenCharacter(self): + """Test for fix to GH#11752 - NVDA Freeze with unicode value U+000B + Actually, the speech manager receives an empty string for the character U+000B. NVDA + does not have a mapping for this character. + It is questionable whether we should send anything to the synth when there is no content, however + what constitutes 'content' is currently not easy to define. + """ + smi = SpeechManagerInteractions(self) + + speechSequence = [ + CharacterModeCommand(True), + '', + smi.create_EndUtteranceCommand(expectedToBecomeIndex=1) + ] + with smi.expectation(): + seqIndexes = smi.speak(speechSequence) + smi.expect_synthSpeak(seqIndexes) + + +class RegressionTests_withCancellableSpeechEnabled(RegressionTests): + """Note, while cancellable speech is configurable test with and without it enabled.""" + def setUp(self): + super().setUp() + config.conf['featureFlag']['cancelExpiredFocusSpeech'] = 1 # yes From b4b3ec6e50df5b90202e2c1ce98f2af1571b3604 Mon Sep 17 00:00:00 2001 From: Reef Turner Date: Wed, 25 Nov 2020 15:48:25 +0800 Subject: [PATCH 2/2] Index may be zero zero is a might be a valid index. Only no index (None) shoudl take this path --- source/speech/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/speech/manager.py b/source/speech/manager.py index f31d8666d2d..ea4d72a2b8f 100644 --- a/source/speech/manager.py +++ b/source/speech/manager.py @@ -547,7 +547,7 @@ def _getMostRecentlyCancelledUtterance(self) -> Optional[_IndexT]: ) for index in cancelledIndexes: if ( - not latestCancelledUtteranceIndex + latestCancelledUtteranceIndex is None or self._isIndexABeforeIndexB(latestCancelledUtteranceIndex, index) ): latestCancelledUtteranceIndex = index