Prepare for selective UIA event registration by default #13297

codeofdusk · 2022-02-01T19:02:11Z

Link to issue number:

None

Summary of the issue:

We would like to switch to selective UIA event registration by default.

Description of how this pull request fixes the issue:

Convert the selective UIA registration option to a ternary setting and factor out logic to determine whether to register selectively into a new function.

Testing strategy:

N/A

Known issues with pull request:

None known

Change log entries:

Not user-facing

Code Review Checklist:

Pull Request description:
- description is up to date
- change log entries
Testing:
- Unit tests
- System (end to end) tests
- Manual testing
API is compatible with existing add-ons.
Documentation:
- User Documentation
- Developer / Technical Documentation
- Context sensitive help for GUI changes
UX of all users considered:
- Speech
- Braille
- Low Vision
- Different web browsers
- Localization in other languages / culture than English

LeonarddeR · 2022-02-03T08:35:35Z

There is a known issue that task manager is misbehaving when selective registration is on. I guess we'll have to take that for ok or fix it somehow.

seanbudd · 2022-02-09T03:39:55Z

@LeonarddeR - you are referring to #11599, correct?

I searched NVDA issues for "selective registration"

@codeofdusk - do you mind investigating how this PR effects #11599, #11354, #11109, #8624?

It looks like this will improve the latter two, but make behaviour worse for the first two.

josephsl · 2022-02-09T03:48:45Z

Hi, note that enabling selective UIA event registration will break Windows 10 emoji panel support (does not affect Windows 11) due to the fact that emoji panel interface is an overlay that does not take system focus at all. Thanks.

codeofdusk · 2022-02-09T07:44:28Z

When selective UIA event registration is enabled, NVDA is too verbose in Task Manager #11599: Can still reproduce.
Context menus in Firefox read inconsistently with selective UIA handling enabled #11354: cannot reproduce.
NVDA becomes sluggish with WPF TreeView UIAutomation Events #11109 and Sluggishness with Visual Studio 2017 #8624: more specific instances of UIA freezes after a large number of events is received #11002 (which is improved).

seanbudd · 2022-02-09T23:31:27Z

Hi, note that enabling selective UIA event registration will break Windows 10 emoji panel support (does not affect Windows 11) due to the fact that emoji panel interface is an overlay that does not take system focus at all. Thanks.

@codeofdusk - merging this is definitely blocked by the above, and if a fix for #11599 is possible.

I suggest we try and find a mechanism to disable this for the Windows 10 emoji panel.
If we can disable this on an appModule level, the same could be done for task manager (and Firefox, if it is still necessary).

Another path is to create a third state for the setting, "Windows 11 only" which only enables this on Windows 11.
We can probably suppress the Task Manager verbosity depending on if the setting is enabled or disabled.

codeofdusk · 2022-07-20T07:08:45Z

Another path is to create a third state for the setting, "Windows 11 only" which only enables this on Windows 11.

@seanbudd This would require a config spec schema upgrade. If we can fix #11599 I think that could be reasonable.

codeofdusk · 2022-07-20T07:11:57Z

Actually, it looks like I can no longer reproduce #11599 at least on Nickel. I'll proceed with the config spec upgrade.

LeonarddeR · 2022-07-20T07:12:48Z

Can we use the new feature flag approach while at it?

codeofdusk · 2022-07-20T07:27:15Z

Can we use the new feature flag approach while at it?

I'm not sure. This seems closer to the "diff algorithm" or "Windows Console support" option (choice of two approaches or an "automatic" option that lets NVDA decide based on runtime environment) than an opt-in/opt-out feature with set NVDA developer default.

AppVeyorBot · 2022-07-20T09:14:19Z

PASS: Translation comments check.
PASS: Unit tests.
PASS: Lint check.
FAIL: System tests. See test results for more information.
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/h8vib6yqhoc9k0wt/artifacts/output/nvda_snapshot_pr13297-25948,f2926f5a.exe

See test results for failed build of commit f2926f5ac2

Qchristensen

User Guide change looks good

seanbudd · 2022-08-16T00:06:11Z

@codeofdusk can you update the PR description? it appears out of date

codeofdusk · 2022-08-16T00:07:16Z

@seanbudd What do you mean? It looks up-to-date to me.

source/config/profileUpgradeSteps.py

seanbudd · 2022-08-16T00:14:48Z

source/config/profileUpgradeSteps.py

+	"""
+	Selective UIA registration check box has been replaced with event registration multi choice.
+	"""


What does profile['UIA']['eventRegistration'] become as the result of upgrading when:

the user has not changed the default value of profile['UIA']['selectiveEventRegistration'] (e.g. it is set to False)

the user has explicitly set the value to False

I currently only understand the case where:

the user has explicitly set the value to True

Could you add a summary of what happens for the 3 cases to the docstring?

Co-authored-by: Sean Budd <[email protected]>

seanbudd · 2022-08-16T00:17:41Z

@codeofdusk the summary of the issue reads as if the bugs are still an issue. Can you add that these bugs are fixed in Windows 11 SV2?

source/config/profileUpgradeSteps.py

codeofdusk · 2022-08-16T00:28:33Z

source/config/profileUpgradeSteps.py

@@ -101,7 +101,7 @@ def upgradeConfigFrom_5_to_6(profile: dict):
 		profile['UIA']['allowInMSWord'] = AllowUiaInMSWord.ALWAYS.value


-def upgradeConfigFrom_6_to_7(profile: dict):
+def upgradeConfigFrom_6_to_7(profile: Dict[str, str]) -> None:


For some reason, this change prevents NVDA from starting on my system.

You need to import Dict from typing

codeofdusk · 2022-08-16T00:30:02Z

@seanbudd For some reason, adding your type hints to the config upgrade step seems to prevent NVDA from starting on my machine.

seanbudd · 2022-08-16T00:31:23Z

source/config/profileUpgradeSteps.py

@@ -1,6 +1,6 @@
 # -*- coding: UTF-8 -*-
 #A part of NonVisual Desktop Access (NVDA)
-#Copyright (C) 2016 NV Access Limited
+#Copyright (C) 2016–2022 NV Access Limited, Bill Dengler


To fix linting you will probably have to add a space after the # for each line

source/config/profileUpgradeSteps.py

seanbudd

Thanks @codeofdusk LGTM once these minor things get fixed!

Co-authored-by: Sean Budd <[email protected]>

codeofdusk · 2022-08-16T00:36:33Z

@seanbudd Do you think it's alright to keep the config spec upgrade and behaviour change in one PR, and in the event of revert follow the process outlined in the known issues? Or should I separate them similar to UIA console?

seanbudd · 2022-08-16T00:41:32Z

@codeofdusk - that would be ideal, but I believe there is less risk of reverting for this PR

codeofdusk · 2022-08-16T00:49:28Z

@seanbudd Done. I'll make a follow-up once this is merged.

AppVeyorBot · 2022-08-16T01:26:03Z

PASS: Translation comments check.
PASS: Unit tests.
FAIL: Lint check. See test results for more information.
PASS: System tests.
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/b5ua8o0phn02c83v/artifacts/output/nvda_snapshot_pr13297-26179,5caf543b.exe

See test results for failed build of commit 5caf543b4c

codeofdusk · 2022-08-16T03:31:33Z

Thanks @seanbudd. I've opened #14018.

Link to issue number: Partial mitigation for #11002. Follow-up of #11214 and #13297. Summary of the issue: NVDA 2020.3 introduced support for selective registration for UIA events based on the currently-focused provider, but this is disabled by default due to bugs in the Windows 10 task manager and emoji panel. These bugs have been fixed in Windows 11 SV2. Description of how this pull request fixes the issue: By default, enable selective UIA event registration on Windows 11 Sun Valley 2 (22H2). Testing strategy: Enable by default in master snapshots to allow for wider testing.

…in the selective registration UI, for consistency with the config spec (#14107) Follow-up of #13297. Summary of the issue: In the advanced settings panel, the config options were referred to as "selectively" and "globally", but the config spec uses "selective" and "global", which makes more sense ("selective registration" vs "selectively registration"). Description of how this pull request fixes the issue: Update the UI. No other functional changes.

codeofdusk requested a review from a team as a code owner February 1, 2022 19:02

codeofdusk requested a review from seanbudd February 1, 2022 19:02

seanbudd added the blocked label Feb 11, 2022

seanbudd marked this pull request as draft February 18, 2022 05:36

codeofdusk force-pushed the selective-uia-by-default branch 6 times, most recently from df5c845 to 7ce1e20 Compare July 20, 2022 08:33

codeofdusk changed the title ~~Selectively register for UIA events by default~~ Selectively register for UIA events by default on Windows 11 Jul 20, 2022

codeofdusk marked this pull request as ready for review July 20, 2022 09:15

codeofdusk requested a review from a team as a code owner July 20, 2022 09:15

codeofdusk requested a review from Qchristensen July 20, 2022 09:15

Qchristensen approved these changes Jul 20, 2022

View reviewed changes

seanbudd added merge-early Merge Early in a developer cycle blocked/needs-code-review and removed blocked labels Jul 21, 2022

seanbudd added this to the 2022.4 milestone Jul 25, 2022

This comment was marked as outdated.

Sign in to view

Syntax

00a521e

seanbudd reviewed Aug 16, 2022

View reviewed changes

Update source/config/profileUpgradeSteps.py

cfd4179

Co-authored-by: Sean Budd <[email protected]>

Update docstring

2d9dd53

seanbudd reviewed Aug 16, 2022

View reviewed changes

source/config/profileUpgradeSteps.py Show resolved Hide resolved

Copyright headers

f2ea367

codeofdusk commented Aug 16, 2022

View reviewed changes

Import

d9d51bd

seanbudd reviewed Aug 16, 2022

View reviewed changes

Style

0a7e4c3

seanbudd reviewed Aug 16, 2022

View reviewed changes

source/config/profileUpgradeSteps.py Outdated Show resolved Hide resolved

seanbudd approved these changes Aug 16, 2022

View reviewed changes

Update source/config/profileUpgradeSteps.py

6b55818

Co-authored-by: Sean Budd <[email protected]>

Remove changes to behaviour

bac8204

codeofdusk changed the title ~~Selectively register for UIA events by default on SV2~~ Prepare for selective UIA event registration by default Aug 16, 2022

Style

3d9be93

seanbudd merged commit f47dd5c into nvaccess:master Aug 16, 2022

codeofdusk mentioned this pull request Aug 16, 2022

Selectively register for UIA events by default on SV2 #14018

Merged

5 tasks

codeofdusk mentioned this pull request Sep 5, 2022

Change selectively and globally to selective and global respectively in the selective registration UI #14107

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepare for selective UIA event registration by default #13297

Prepare for selective UIA event registration by default #13297

codeofdusk commented Feb 1, 2022 •

edited

Loading

LeonarddeR commented Feb 3, 2022

seanbudd commented Feb 9, 2022

josephsl commented Feb 9, 2022 via email

codeofdusk commented Feb 9, 2022

seanbudd commented Feb 9, 2022

codeofdusk commented Jul 20, 2022

codeofdusk commented Jul 20, 2022

LeonarddeR commented Jul 20, 2022

codeofdusk commented Jul 20, 2022

AppVeyorBot commented Jul 20, 2022

Qchristensen left a comment

This comment was marked as outdated.

seanbudd commented Aug 16, 2022

codeofdusk commented Aug 16, 2022

seanbudd Aug 16, 2022

seanbudd commented Aug 16, 2022

codeofdusk Aug 16, 2022

seanbudd Aug 16, 2022

codeofdusk commented Aug 16, 2022 •

edited

Loading

seanbudd Aug 16, 2022

seanbudd left a comment •

edited

Loading

codeofdusk commented Aug 16, 2022

seanbudd commented Aug 16, 2022

codeofdusk commented Aug 16, 2022

AppVeyorBot commented Aug 16, 2022

codeofdusk commented Aug 16, 2022

Prepare for selective UIA event registration by default #13297

Prepare for selective UIA event registration by default #13297

Conversation

codeofdusk commented Feb 1, 2022 • edited Loading

Link to issue number:

Summary of the issue:

Description of how this pull request fixes the issue:

Testing strategy:

Known issues with pull request:

Change log entries:

Code Review Checklist:

LeonarddeR commented Feb 3, 2022

seanbudd commented Feb 9, 2022

josephsl commented Feb 9, 2022 via email

codeofdusk commented Feb 9, 2022

seanbudd commented Feb 9, 2022

codeofdusk commented Jul 20, 2022

codeofdusk commented Jul 20, 2022

LeonarddeR commented Jul 20, 2022

codeofdusk commented Jul 20, 2022

AppVeyorBot commented Jul 20, 2022

Qchristensen left a comment

Choose a reason for hiding this comment

This comment was marked as outdated.

seanbudd commented Aug 16, 2022

codeofdusk commented Aug 16, 2022

seanbudd Aug 16, 2022

Choose a reason for hiding this comment

seanbudd commented Aug 16, 2022

codeofdusk Aug 16, 2022

Choose a reason for hiding this comment

seanbudd Aug 16, 2022

Choose a reason for hiding this comment

codeofdusk commented Aug 16, 2022 • edited Loading

seanbudd Aug 16, 2022

Choose a reason for hiding this comment

seanbudd left a comment • edited Loading

Choose a reason for hiding this comment

codeofdusk commented Aug 16, 2022

seanbudd commented Aug 16, 2022

codeofdusk commented Aug 16, 2022

AppVeyorBot commented Aug 16, 2022

codeofdusk commented Aug 16, 2022

codeofdusk commented Feb 1, 2022 •

edited

Loading

codeofdusk commented Aug 16, 2022 •

edited

Loading

seanbudd left a comment •

edited

Loading