Fix nvda find title cancellation

[feerrenrut]

Link to issue number:

#11397 (comment)

Summary of the issue:

When Control+Insert+F is used to bring up the Find dialog, NVDA doesn't announce the title of the dialog, "Find dialog", in most instances. This also applies to NVDA settings dialogs.

Description of how this pull request fixes the issue:

I initially tried by not cancelling speech for NVDA Objects that were Window objects, while this certainly fixed the issue it also made the cancellable speech feature useless. This is because all IAccessible NVDA objects are Window objects.

Specifically this meant that the cancellable speech would no longer fix fast focus changes in Gmail.

Looking at the log, after opening NVDA Find dialog the last
queued focus object is set to the following:

• NVDAObjects.Dynamic_NvdaDialogDialogIAccessibleWindowNVDAObject
• NVDAObjects.Dynamic_IAccessibleEditWindowNVDAObject

The first is the dialog, the second the edit field.

The full MRO for the dialog is:

• class 'NVDAObjects.Dynamic_NvdaDialogDialogIAccessibleWindowNVDAObject'
• class 'appModules.nvda.NvdaDialog'
• class 'NVDAObjects.behaviors.Dialog'
• class 'NVDAObjects.IAccessible.IAccessible'
• class 'NVDAObjects.window.Window'
• class 'NVDAObjects.NVDAObject'
• class 'documentBase.TextContainerObject'
• class 'baseObject.ScriptableObject'
• class 'baseObject.AutoPropertyObject'
• class 'garbageHandler.TrackedObject'
• class 'object'

Excluding speech cancellation on 'NVDAObjects.behaviors.Dialog' seems to
generally make sense, resolves the NVDA Find dialog speech for focus
cancellation, and does not appear to interfere with the Gmail fix.

However further investigation raised the question: Why is this dialog not part of the focus ancestors?

It turns out that using api.getForegroundObject() also resolves the issue, leading to the question: How is this no longer the focused object, but is the foregroundObject?

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.

Testing performed:

• Tested actions described in summary of the issue.
• Tested moving focus quickly (in focus mode) in Gmail with Chrome.

Known issues with pull request:

Change log entry:

Bug:

• With Attempt to Cancel speech for expired focus events enabled the title of the NVDA Find dialog is announced again.

[AppVeyorBot]

• Build (for testing PR)
• System test failure

See test results for failed build of commit e7d1ca190c

[lukaszgo1]

While fix proposed here certainly makes dialog titles to be read again I'd like to propose more general approach which would probably also fix #11492.
In general we never want to cancel speech if it occurs as a result of the new window gaining focus, so in addition to not cancelling speech for dialogs we should not cancel speech for the current foreground object. We can either compare the object to result of api.getForegroundObject or even better exclude speech resulting from foreground events from being cancelled altogether.

[michaelDCurran]

This bug is wider than just the Find dialog. For instance NVDA+control+g to bring up the NVDA settings dialog set to the General pannel: The dialog is not announced, only the "General property page".
A bit of debugging shows me that there is a focus event for the dialog itself and then a focus event for the actual child item with focus. However, I think there is a timing issue here in that by the time speech is validated for the focus on the dialog, things have moved on such that eventHandler.lastQueuedFocusObject is set to the child item, but, api.getFocusAncestors has not yet been set for the child item, thus we end up getting wasLast: False, wasPrevious: True, isInAncestors: False.
We really want inAncestors to be true in this case.

[feerrenrut]

@michaelDCurran That's right, there are two focus events received. Actually, I didn't check if there the "dialog" object is actually an ancestor of the new control, theoretically it should be, the timing issue of updating focus ancestors is an interesting angle to investigate.

I hoped that all dialogs would get the dialog class added, I guess that is not true for the settings dialog?

[feerrenrut]

@lukaszgo1 Do you mean not cancel any speech for objects in the foreground window? The main use case for the cancellable speech feature was to fix multiple focus events from "queuing up" speech, such as when navigating through items in Gmail.

[lukaszgo1]

@feerrenrut wrote:

@lukaszgo1 Do you mean not cancel any speech for objects in the foreground window?

No, that is not what I mend. Currently speech is often cancelled for window titles in case of these dialogs the missing info is the window title and its role. Therefore I've suggested to not cancel speech resulting from foreground events as change of a foreground window should always be announced to the user.

[feerrenrut]

Therefore I've suggested to not cancel speech resulting from foreground events as change of a foreground window should always be announced to the user.

Yes, we are in the process of working out how to do that. Foreground events are treated as focus events in NVDA, so we can't currently differentiate on that.

[michaelDCurran]

We could just compare the object against api.getForegroundObject() and never cancel if it is equal.

[feerrenrut]

I believe this is ready for review.

[michaelDCurran]

Is this still necessary after the change from lastQueuedFocusObject to api.getFocusObject()? I wouldn't have thought so, unless the bug is different to what we hypothesized.

[feerrenrut]

Your are right, checking for self.isForegroundObject() is not required to fix this particular bug, and I have confirmed that. However, it doesn't harm the core use case of cancellable speech (rapid focus changes in Gmail), and I can't think of a reason why we would want to cancel speech for the foreground object if it wasn't the focus ancestor for some reason.

What do you think?

[michaelDCurran]

I think it is redundant and should be removed, as getforegroundObject always returns a focus ancestor, which is checked already.

[feerrenrut]

I'm not sure I follow this. First, I want to confirm you want the self.isForgroundObject() check removed?

This may not be set by the focus ancestor, usually it is from api.getDesktopObject().objectInForeground(), see the following code from doPreGainFocus

		newForeground=api.getDesktopObject().objectInForeground()
		if not newForeground:
			log.debugWarning("Can not get real foreground, resorting to focus ancestors")
			ancestors=api.getFocusAncestors()
			if len(ancestors)>1:
				newForeground=ancestors[1]
			else:
				newForeground=obj
		api.setForegroundObject(newForeground)

[michaelDCurran]

A part from removing the now redundant foreground check, I'm good with this.

[michaelDCurran]

Hmm, I forgot about the first way we set the foreground object. Never mind.  It is best to leave it in then :) So I'm good with all of this now.