From 3426905e49ca13183943b2f39b7134574f84480e Mon Sep 17 00:00:00 2001 From: Reef Turner Date: Thu, 8 Oct 2020 16:59:22 +0200 Subject: [PATCH] Fix nvda find title cancellation (PR #11632) Essentially this came down to a mismatch of the timing for when focus ancestors are set (in api.setFocusObject() called in doPreGainFocus) and when the lastQueuedFocusObject is set (in _trackFocusObject called via queueEvent). If the check for "cancelled" happened after a new focus event had been queued, but before that event was executed then the dialog object was no longer the lastQueuedFocusObject, but was not yet added to the focus ancestors. To resolve this mismatch, api.getFocusObject() is used to compare the object to NVDA's concept of the current focus, this is updated at the same time as focus ancestors. The state with inline functions was becoming confusing and hard to read, it has been replaced with a class. --- source/NVDAObjects/__init__.py | 3 +- source/api.py | 12 ++- source/eventHandler.py | 99 ++++++++++++++--------- source/globalVars.py | 2 +- source/speech/commands.py | 35 ++++---- tests/unit/test_speechManager/__init__.py | 41 ++++++++-- user_docs/en/changes.t2t | 1 + 7 files changed, 127 insertions(+), 66 deletions(-) diff --git a/source/NVDAObjects/__init__.py b/source/NVDAObjects/__init__.py index 96237882e3c..611f5531469 100644 --- a/source/NVDAObjects/__init__.py +++ b/source/NVDAObjects/__init__.py @@ -308,7 +308,8 @@ def objectWithFocus(): @staticmethod def objectInForeground(): - """Retrieves the object representing the current foreground control according to the Operating System. This differes from NVDA's foreground object as this object is the real foreground object according to the Operating System, not according to NVDA. + """Retrieves the object representing the current foreground control according to the + Operating System. This may differ from NVDA's cached foreground object. @return: the foreground object @rtype: L{NVDAObject} """ diff --git a/source/api.py b/source/api.py index 361cdd259db..fbf8ae2fc73 100644 --- a/source/api.py +++ b/source/api.py @@ -39,13 +39,23 @@ def getFocusObject(): def getForegroundObject(): """Gets the current foreground object. + This (cached) object is the (effective) top-level "window" (hwnd). + EG a Dialog rather than the focused control within the dialog. + The cache is updated as queued events are processed, as such there will be a delay between the winEvent + and this function matching. However, within NVDA this should be used in order to be in sync with other + functions such as "getFocusAncestors". @returns: the current foreground object @rtype: L{NVDAObjects.NVDAObject} """ return globalVars.foregroundObject def setForegroundObject(obj): - """Stores the given object as the current foreground object. (Note: it does not physically change the operating system foreground window, but only allows NVDA to keep track of what it is). + """Stores the given object as the current foreground object. + Note: does not cause the operating system to change the foreground window, + but simply allows NVDA to keep track of what the foreground window is. + Alternative names for this function may have been: + - setLastForegroundWindow + - setLastForegroundEventObject @param obj: the object that will be stored as the current foreground object @type obj: NVDAObjects.NVDAObject """ diff --git a/source/eventHandler.py b/source/eventHandler.py index d0e1a86dd76..1d722135dd4 100755 --- a/source/eventHandler.py +++ b/source/eventHandler.py @@ -152,11 +152,62 @@ def _trackFocusObject(eventName, obj) -> None: if eventName == "gainFocus": lastQueuedFocusObject = obj - setattr(obj, WAS_GAIN_FOCUS_OBJ_ATTR_NAME, True) - if speech.manager._shouldDoSpeechManagerLogging(): - log.debug(f"Changing last queued focus object: {obj!r}") - elif not hasattr(obj, WAS_GAIN_FOCUS_OBJ_ATTR_NAME): - setattr(obj, WAS_GAIN_FOCUS_OBJ_ATTR_NAME, False) + + +class FocusLossCancellableSpeechCommand(_CancellableSpeechCommand): + def __init__(self, obj, reportDevInfo: bool): + from NVDAObjects import NVDAObject + if not isinstance(obj, NVDAObject): + log.warning("Unhandled object type. Expected all objects to be descendant from NVDAObject") + raise TypeError(f"Unhandled object type: {obj!r}") + self._obj = obj + super(FocusLossCancellableSpeechCommand, self).__init__(reportDevInfo=reportDevInfo) + + if self.isLastFocusObj(): + # Objects may be re-used. + # WAS_GAIN_FOCUS_OBJ_ATTR_NAME state should be cleared at some point? + # perhaps instead keep a weak ref list of obj that had focus, clear on keypress? + + # Assumption: we only process one focus event at a time, so even if several focus events are queued, + # all focused objects will still gain this tracking attribute. Otherwise, this may need to be set via + # api.setFocusObject when globalVars.focusObject is set. + setattr(obj, WAS_GAIN_FOCUS_OBJ_ATTR_NAME, True) + elif not hasattr(obj, WAS_GAIN_FOCUS_OBJ_ATTR_NAME): + setattr(obj, WAS_GAIN_FOCUS_OBJ_ATTR_NAME, False) + + def _checkIfValid(self): + stillValid = ( + self.isLastFocusObj() + or not self.previouslyHadFocus() + or self.isAncestorOfCurrentFocus() + # Ensure titles for dialogs gaining focus are reported, EG NVDA Find dialog + or self.isForegroundObject() + ) + return stillValid + + def _getDevInfo(self): + return ( + f"isLast: {self.isLastFocusObj()}" + f", previouslyHad: {self.previouslyHadFocus()}" + f", isAncestorOfCurrentFocus: {self.isAncestorOfCurrentFocus()}" + f", is foreground obj {self.isForegroundObject()}" + ) + + def isLastFocusObj(self): + # Use '==' rather than 'is' because obj may have been created multiple times + # pointing to the same underlying object. + return self._obj == api.getFocusObject() + + def previouslyHadFocus(self): + return getattr(self._obj, WAS_GAIN_FOCUS_OBJ_ATTR_NAME, False) + + def isAncestorOfCurrentFocus(self): + return self._obj in api.getFocusAncestors() + + def isForegroundObject(self): + foreground = api.getForegroundObject() + return self._obj is foreground or self._obj == foreground + def _getFocusLossCancellableSpeechCommand( obj, @@ -169,34 +220,9 @@ def _getFocusLossCancellableSpeechCommand( log.warning("Unhandled object type. Expected all objects to be descendant from NVDAObject") return None - def previouslyHadFocus(): - return getattr(obj, WAS_GAIN_FOCUS_OBJ_ATTR_NAME, False) - - def isLastFocusObj(): - # Obj may have been created multiple times pointing to the same underlying object. - return obj is lastQueuedFocusObject or obj == lastQueuedFocusObject - - def isAncestorOfCurrentFocus(): - return obj in api.getFocusAncestors() + shouldReportDevInfo = speech.manager._shouldDoSpeechManagerLogging() + return FocusLossCancellableSpeechCommand(obj, reportDevInfo=shouldReportDevInfo) - def isSpeechStillValid(): - stillValid = ( - isLastFocusObj() - or not previouslyHadFocus() - or isAncestorOfCurrentFocus() - ) - return stillValid - - if not speech.manager._shouldDoSpeechManagerLogging(): - return _CancellableSpeechCommand(isSpeechStillValid) - - def getDevInfo(): - return ( - f"isLast: {isLastFocusObj()}" - f", previouslyHad: {previouslyHadFocus()}" - f", isAncestorOfCurrentFocus: {isAncestorOfCurrentFocus()}" - ) - return _CancellableSpeechCommand(isSpeechStillValid, getDevInfo) def executeEvent(eventName, obj, **kwargs): """Executes an NVDA event. @@ -232,13 +258,12 @@ def doPreGainFocus(obj,sleepMode=False): # ask speechManager to check if any of it's queued utterances should be cancelled # Note: Removing cancelled speech commands should happen after all dependencies for the isValid check # have been updated: - # - lastQueuedFocusObject # - obj.WAS_GAIN_FOCUS_OBJ_ATTR_NAME + # - api.setFocusObject() # - api.getFocusAncestors() - # These are updated: - # - lastQueuedFocusObject & obj.WAS_GAIN_FOCUS_OBJ_ATTR_NAME - # - Set in stack: _trackFocusObject, eventHandler.queueEvent - # - Which results in executeEvent being called, then doPreGainFocus + # When these are updated: + # - obj.WAS_GAIN_FOCUS_OBJ_ATTR_NAME + # - Set during creation of the _CancellableSpeechCommand. # - api.getFocusAncestors() via api.setFocusObject() called in doPreGainFocus speech._manager.removeCancelledSpeechCommands() diff --git a/source/globalVars.py b/source/globalVars.py index 5a93fec7ba6..f371093caa4 100644 --- a/source/globalVars.py +++ b/source/globalVars.py @@ -4,7 +4,7 @@ #This file is covered by the GNU General Public License. #See the file COPYING for more details. """global variables module -@var foregroundObject: holds the current foreground object +@var foregroundObject: holds the current foreground object. The object for the last foreground event received. @type foregroundObject: L{NVDAObjects.NVDAObject} @var focusObject: holds the current focus object @type focusObject: L{NVDAObjects.NVDAObject} diff --git a/source/speech/commands.py b/source/speech/commands.py index b0caad9fef4..1561dca40cc 100644 --- a/source/speech/commands.py +++ b/source/speech/commands.py @@ -33,18 +33,22 @@ class _CancellableSpeechCommand(SpeechCommand): def __init__( self, - checkIfValid: Optional[Callable[[], bool]] = None, - getDevInfo: Optional[Callable[[], str]] = None, + reportDevInfo=False ): """ - @param checkIfValid: Callable that returns True if the utterance is still valid. - @param getDevInfo: An optional used to supply more information when __repr__ is called. + @param reportDevInfo: If true, developer info is reported for repr implementation. """ self._isCancelled = False - if checkIfValid: - self._checkIfValid = checkIfValid - self._getCheckIfValidDevInfo = getDevInfo self._utteranceIndex = None + self._reportDevInfo = reportDevInfo + + @abstractmethod + def _checkIfValid(self): + raise NotImplementedError() + + @abstractmethod + def _getDevInfo(self): + raise NotImplementedError() def _checkIfCancelled(self): if self._isCancelled: @@ -60,26 +64,21 @@ def isCancelled(self): def cancelUtterance(self): self._isCancelled = True - @staticmethod - def _checkIfValid() -> bool: - """Overridable behavior.""" - return True + def _getFormattedDevInfo(self): - def _getDevInfo(self): - isValidCallbackDevInfo = "" if not self._getCheckIfValidDevInfo else self._getCheckIfValidDevInfo() - return ( - f"devInfo(" + return "" if not self._reportDevInfo else ( + f", devInfo<" f" isCanceledCache: {self._isCancelled}" f", isValidCallback: {self._checkIfValid()}" - f", isValidCallbackDevInfo: {isValidCallbackDevInfo}" + f", isValidCallbackDevInfo: {self._getDevInfo()} >" ) def __repr__(self): return ( f"CancellableSpeech (" f"{ 'cancelled' if self._checkIfCancelled() else 'still valid' }" - f" {self._getDevInfo()}" - f" )" + f"{self._getFormattedDevInfo()}" + f")" ) diff --git a/tests/unit/test_speechManager/__init__.py b/tests/unit/test_speechManager/__init__.py index dc5d526005a..e0563cfc237 100644 --- a/tests/unit/test_speechManager/__init__.py +++ b/tests/unit/test_speechManager/__init__.py @@ -6,6 +6,8 @@ """Unit tests for speech/manager module """ import unittest +from typing import Optional, Callable + import config import speech from unittest import mock @@ -152,6 +154,29 @@ def test_expectedProsodyNotMatching(self): self.assertNotEqual(p, e) +class _CancellableSpeechCommand_withLamda(_CancellableSpeechCommand): + """ + A test helper to control when speech gets cancelled. + """ + + def _getDevInfo(self): + return "" + + def _checkIfValid(self): + return True + + def __init__( + self, + checkIfValid: Optional[Callable[[], bool]] = None, + getDevInfo: Optional[Callable[[], str]] = None, + ): + if checkIfValid is not None: + self._checkIfValid = checkIfValid + if getDevInfo is not None: + self._getDevInfo = getDevInfo + super().__init__(reportDevInfo=True) + + class CancellableSpeechTests(unittest.TestCase): """Tests behaviour related to CancellableSpeech""" @@ -166,7 +191,7 @@ def test_validSpeechSpoken(self): with smi.expectation(): text = "This stays valid" - smi.speak([text, _CancellableSpeechCommand(lambda: True)]) + smi.speak([text, _CancellableSpeechCommand_withLamda(lambda: True)]) smi.expect_synthSpeak(sequence=[text, smi.create_ExpectedIndex(expectedToBecomeIndex=1)]) def test_invalidSpeechNotSpoken(self): @@ -175,7 +200,7 @@ def test_invalidSpeechNotSpoken(self): with smi.expectation(): text = "This stays invalid" - smi.speak([text, _CancellableSpeechCommand(lambda: False)]) + smi.speak([text, _CancellableSpeechCommand_withLamda(lambda: False)]) def test_invalidated_indexHit(self): """Hitting an index should cause a cancellation of speech that has become @@ -195,7 +220,7 @@ def _checkIfValid(speechNumber): smi.create_CallBackCommand(expectedToBecomeIndex=1), "text 2", smi.create_ExpectedIndex(expectedToBecomeIndex=2), - _CancellableSpeechCommand(lambda: _checkIfValid(1)), + _CancellableSpeechCommand_withLamda(lambda: _checkIfValid(1)), ] isSpeechNumberValid[1] = True # initially valid smi.speak(initiallyValidSequence) @@ -226,7 +251,7 @@ def _checkIfValid(speechNumber): smi.create_CallBackCommand(expectedToBecomeIndex=1), "text 2", smi.create_ExpectedIndex(expectedToBecomeIndex=2), - _CancellableSpeechCommand(lambda: _checkIfValid(1)), + _CancellableSpeechCommand_withLamda(lambda: _checkIfValid(1)), ] isSpeechNumberValid[1] = True # initially valid smi.speak(initiallyValidSequence) @@ -263,7 +288,7 @@ def _checkIfValid(speechNumber): smi.create_CallBackCommand(expectedToBecomeIndex=1), "text 2", smi.create_ExpectedIndex(expectedToBecomeIndex=2), - _CancellableSpeechCommand(lambda: _checkIfValid(1)), + _CancellableSpeechCommand_withLamda(lambda: _checkIfValid(1)), ] isSpeechNumberValid[1] = True # initially valid smi.speak(initiallyValidSequence) @@ -277,7 +302,7 @@ def _checkIfValid(speechNumber): smi.create_CallBackCommand(expectedToBecomeIndex=3), "text 4", smi.create_ExpectedIndex(expectedToBecomeIndex=4), - _CancellableSpeechCommand(lambda: _checkIfValid(2)), + _CancellableSpeechCommand_withLamda(lambda: _checkIfValid(2)), ] smi.speak(newValidSequence) smi.expect_synthCancel() @@ -292,12 +317,12 @@ def test_validSpeechAfterInvalid(self): with smi.expectation(): smi.speak([ "Stays invalid", - _CancellableSpeechCommand(lambda: False), + _CancellableSpeechCommand_withLamda(lambda: False), smi.create_ExpectedIndex(expectedToBecomeIndex=1) ]) with smi.expectation(): - smi.speak(["Stays valid", _CancellableSpeechCommand(lambda: True)]) + smi.speak(["Stays valid", _CancellableSpeechCommand_withLamda(lambda: True)]) smi.expect_synthSpeak(sequence=[ "Stays valid", smi.create_ExpectedIndex(expectedToBecomeIndex=2) ]) diff --git a/user_docs/en/changes.t2t b/user_docs/en/changes.t2t index c87af95a3fe..27767d66623 100644 --- a/user_docs/en/changes.t2t +++ b/user_docs/en/changes.t2t @@ -34,6 +34,7 @@ What's New in NVDA - When using the SAPI5 Ivona voices from harposoftware.com, NvDA is now able to save configuration, switch synthesizers, and no longer will stay silent after restarting. (#11650) - It is now possible to enter number 6 in computer braille from a braille keyboard on HIMS displays. (#11710) - Major performance improvements in Azure Data Studio. (#11533, #11715) +- With "Attempt to Cancel speech for expired focus events" enabled the title of the NVDA Find dialog is announced again. (#11632) == Changes for Developers ==