Skip to content

Enable cancellable speech by default#11266

Merged
feerrenrut merged 17 commits intomasterfrom
makeCancellableSpeechDefault
Mar 17, 2021
Merged

Enable cancellable speech by default#11266
feerrenrut merged 17 commits intomasterfrom
makeCancellableSpeechDefault

Conversation

@feerrenrut
Copy link
Contributor

@feerrenrut feerrenrut commented Jun 17, 2020

Link to issue number:

First introduced with "Cancellable speech #10885"
Several issues fixed with "Fix several issues in speech manager #11245"

Summary of the issue:

The experimental fix for rapid focus events was included but disabled by default in 2020.2, this PR enables it for broad testing during alpha for 2020.3.

Description of how this pull request fixes the issue:

Make 'Attempt to cancel speech for expired focus events' enabled by default.

Testing performed:

Known issues with pull request:

If this causes problems for users, it can be manually disabled by setting "Attempt to cancel speech for expired focus events" to "No"

Change log entry:

Changes

- 'Attempt to cancel speech for expired focus events' option in the advanced settings panel now enabled by default.
  - This behaviour can be disabled by default with by setting this option to "No".
  - Web applications (E.G. Gmail) no longer speak outdated information when moving focus rapidly. (#10885)

@feerrenrut feerrenrut added this to the 2020.3 milestone Jun 17, 2020
@feerrenrut feerrenrut self-assigned this Jun 17, 2020
@AppVeyorBot

This comment has been minimized.

@feerrenrut feerrenrut marked this pull request as draft June 17, 2020 13:47
@codeofdusk

This comment has been minimized.

@feerrenrut

This comment has been minimized.

@codeofdusk

This comment has been minimized.

@feerrenrut

This comment has been minimized.

@codeofdusk

This comment has been minimized.

@lukaszgo1

This comment has been minimized.

@amirsol81

This comment has been minimized.

@feerrenrut feerrenrut modified the milestones: 2020.3, 2020.4 Sep 2, 2020
Many NVDAObjects derive from Window, including IAccessible.
This meant that the fix for focus changes in Gmail was bypassed!

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.
To avoid window change speech from getting cancelled
Encapsulate logic in a class, clarify ownership of obj ref.
@feerrenrut

This comment has been minimized.

@feerrenrut

This comment has been minimized.

@codeofdusk

This comment has been minimized.

@Adriani90

This comment has been minimized.

@ramchaitanyap

This comment has been minimized.

@feerrenrut
Copy link
Contributor Author

@ramchaitanyap Could you please create a new issue for this?

I have created a codepen to demonstrate this a little more clearly, see the full view and press the 'do focus changes button'.
I reproduced issue with firefox and chrome.

Although it should be noted this is not a regression due to this feature. It would be good to handle this case, I don't think it should prevent the feature from being turned on by default. There are other situations where this feature drastically improves things.

@feerrenrut
Copy link
Contributor Author

@ramchaitanyap created the following related issue: #11840

@feerrenrut

This comment has been minimized.

@lukaszgo1
Copy link
Contributor

This should also be blocked by #11736 . While it is theoretically possible that this is NVDA Remote's problem rather than an NVDA one we should not knowingly break add-ons that way especially not such popular ones as NVDA Remote.

@feerrenrut
Copy link
Contributor Author

@lukaszgo1 Yes, this is unfortunate. Hopefully there is a simple fix we can apply on the NVDA side, otherwise we will defer the Cancellable speech by default change until the 2021.1 release. It should be noted that we don't believe we have broken any public add-on API with the Cancellable speech change, and if we do break the addon-API we expect add-on authors to help us to identify the breakage. Given the open and flexible nature of how NVDA add-ons interact with NVDA, we can only provide a best-effort approach with no strong guarantees. It is frustrating that NVDA Remote is written in a fragile way, relying on implementation details, and then maintenance falls on NV Access. The intention to make this option the default has been public for quite a while, and the option itself has existed for far longer. It seems NVDA Remote could be considered abandon-ware at this stage, it does not see active maintenance. Now would be a good time for someone new to take it on. The 2021.1 release will almost certainly break it.

@dpy013
Copy link
Contributor

dpy013 commented Dec 10, 2020

hi @feerrenrut
Can we change the milestone from 2020.4 to 2021.1?

@feerrenrut feerrenrut modified the milestones: 2020.4, 2021.1 Dec 15, 2020
@feerrenrut feerrenrut marked this pull request as ready for review March 12, 2021 08:32
@AppVeyorBot

This comment has been minimized.

@michaelDCurran
Copy link
Member

It is appropriate to merge this within this release cycle, sooner rather than later, so that add-ons this affects can be updated. E.g. nvdaRemote.

@feerrenrut feerrenrut merged commit 5fbe05d into master Mar 17, 2021
@feerrenrut feerrenrut deleted the makeCancellableSpeechDefault branch March 17, 2021 10:57
@codeofdusk
Copy link
Contributor

I think it might be clearer to have the change log read:

  • Web applications (E.G. Gmail) no longer speak outdated information when moving focus rapidly. (Cancellable speech #10885)
    • 'Attempt to cancel speech for expired focus events' option in the advanced settings panel now enabled by default.
    • This behaviour can be disabled by setting this option to "No".

Keeps parity with initial cancellable speech change and puts user impact first (as main change log point).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants