Skip to content

Disable UIA text change events outside of Word, Windows Console, and Windows Terminal#14067

Merged
seanbudd merged 8 commits into
nvaccess:masterfrom
codeofdusk:drop-uia-textchange
Aug 29, 2022
Merged

Disable UIA text change events outside of Word, Windows Console, and Windows Terminal#14067
seanbudd merged 8 commits into
nvaccess:masterfrom
codeofdusk:drop-uia-textchange

Conversation

@codeofdusk

@codeofdusk codeofdusk commented Aug 25, 2022

Copy link
Copy Markdown
Contributor

Link to issue number:

Mitigation for #11002.
Blocking #14047.

Summary of the issue:

UIA textChange NVDA events are seldom (if ever) used outside of a few specific situations, but have an extreme performance impact (see #11002).

Description of user facing changes

Improved performance/less chance of NVDA hanging in UIA applications.

Description of development approach

Explicitly do not process UIA textChange events outside of Windows Console, Terminal, and Word. The eventual end goal is to remove TermControl/TermControl2 from UIAHandler.textChangeUIAClassNames in #14047, which will very greatly improve performance in Windows Terminal. (conhost will remain, as there don't seem to be any plans to add notifications, especially as wt is becoming the default).

I'm very reluctant to add a mechanism by which add-ons/app modules can request textChange events unless someone requests it, especially given #11002.

Testing strategy:

  • Tested in a self-signed build of NVDA over a few days.
  • Tested that conhost and wt remain functional.
  • Removed TermControl from textChangeUIAClassNames and verified that textChange events are not received.

Known issues with pull request:

None known.

Change log entries:

== Changes for Developers ==

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
  • Security precautions taken.

@codeofdusk codeofdusk requested a review from a team as a code owner August 25, 2022 10:13
@codeofdusk codeofdusk requested a review from seanbudd August 25, 2022 10:13
@codeofdusk codeofdusk force-pushed the drop-uia-textchange branch from ec92a87 to 4762bd3 Compare August 25, 2022 10:17
@codeofdusk codeofdusk marked this pull request as draft August 25, 2022 10:36
@codeofdusk codeofdusk closed this Aug 25, 2022
@LeonarddeR

Copy link
Copy Markdown
Collaborator

Note that you need to add "_WwG" as well since word also relies on text change events

@codeofdusk codeofdusk reopened this Aug 25, 2022
@codeofdusk

Copy link
Copy Markdown
Contributor Author

@LeonarddeR Done. For some reason, start menu suggestions aren't reading with this PR.

@LeonarddeR

Copy link
Copy Markdown
Collaborator

May be @josephsl knows a reason for this, though I don't see how textChange events are used there.

@codeofdusk

Copy link
Copy Markdown
Contributor Author

Never mind, even when running master from source they don't read. I think it's a build environment issue.

@codeofdusk codeofdusk marked this pull request as ready for review August 25, 2022 11:12
@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 820eb60520

Comment thread source/UIAHandler/__init__.py Outdated
seanbudd
seanbudd previously approved these changes Aug 26, 2022
@codeofdusk

Copy link
Copy Markdown
Contributor Author

@seanbudd Why was the build cancelled for this PR?

@codeofdusk codeofdusk force-pushed the drop-uia-textchange branch from e93544d to 6bf9745 Compare August 26, 2022 06:53
@seanbudd

Copy link
Copy Markdown
Member

@codeofdusk - please don't force push to the branch after approval

@seanbudd seanbudd dismissed their stale review August 26, 2022 06:55

stale after force push

@codeofdusk

codeofdusk commented Aug 26, 2022

Copy link
Copy Markdown
Contributor Author

@seanbudd OK. I just tried doing that to see if it'd re-trigger the build. There are no code changes.

@seanbudd

Copy link
Copy Markdown
Member

@codeofdusk - the build was cancelled intentionally. Releases take priority for builds.
You can build and run tests locally.
The build checks are for final checks before merging.

@codeofdusk

codeofdusk commented Aug 26, 2022

Copy link
Copy Markdown
Contributor Author

Oh OK. Will you restart the build for this PR after the release?

seanbudd
seanbudd previously approved these changes Aug 26, 2022
@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 5c330d899c

@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 5c330d899c

@codeofdusk codeofdusk changed the title Disable UIA text change events outside Windows Console and Terminal Disable UIA text change events outside Word, Windows Console, and Windows Terminal Aug 28, 2022
Comment thread source/UIAHandler/__init__.py Outdated
@seanbudd seanbudd dismissed their stale review August 29, 2022 01:45

stale, changes pushed

Co-authored-by: Sean Budd <seanbudd123@gmail.com>
@codeofdusk codeofdusk changed the title Disable UIA text change events outside Word, Windows Console, and Windows Terminal Disable UIA text change events outside of Word, Windows Console, and Windows Terminal Aug 29, 2022
@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 04337168d0

@seanbudd seanbudd merged commit 6fdabfe into nvaccess:master Aug 29, 2022
@nvaccessAuto nvaccessAuto added this to the 2022.4 milestone Aug 29, 2022
seanbudd pushed a commit that referenced this pull request May 26, 2026
Fixes #6291. Partially addresses #15786, #15850, #11002, and #14189. All of these describe the same general class of "NVDA stops responding during console flood" problem but also include symptoms (UIA event flooding, NVDA crashes destroying the console window, etc.) that are out of scope for this PR and were partly addressed in #14888 and #14067 anyway. Related to #2977 and #875.

Most of these are already closed, but the underlying root cause (which is the main thread blocking during line announcement) was never actually addressed.
Summary of the issue:

When a live-text region (such as a terminal) produces a large burst of new text in a short window, NVDA's main thread blocks announcing every line sequentially, becoming unresponsive to keyboard input until the burst finishes. In some cases this lasts tens of seconds. In other (more drastic) cases, it can last so long that the user has to sign out, restart, or force-kill NVDA.
Description of user facing changes:

NVDA no longer freezes when large bursts of text are reported in a live region.
Description of developer facing changes:

LiveText._reportNewLines is no longer guaranteed to report lines synchronously. Internally, it now registers a generator with queueHandler which yields between batches of lines so the main thread can service input during long announcements.

A new class attribute LiveText.MAX_LINES caps the number of lines announced per burst. We drop everything before that on the floor.

When a new burst arrives while a previous one is still being announced, the previous generator is cancelled and the new burst supersedes it.
Description of development approach:

Live-text regions announce new text via LiveText._reportNewLines, which was invoked synchronously from the main thread's event queue. For workloads producing substantial amounts of new lines in a short window, every line traverses the full speech pipeline sequentially without yielding, which can leave the main thread unresponsive to keyboard input for tens of seconds or longer.

This PR addresses the responsiveness problem at the announcement layer rather than at the diff or event-source layer, which were previously addressed by #14888 and #14067. Specifically:

(1) Bursts larger than MAX_LINES are truncated to the most recent N lines. Skipping the oldest lines was chosen over skipping the newest because most flooding scenarios carry their actionable content at the end.

(2) Reporting now happens via a generator registered with queueHandler.registerGeneratorObject, yielding every 5 lines. This is the same primitive previously used by speech.speakSpelling and the original sayAll. Between yields, the main thread services the event queue, which keeps NVDA responsive.

(3) A handler is registered with pre_speechCanceled so that pressing control (or any other speech-cancel trigger) also cancels the in-flight generator. Perceptibly this may not cancel immediately and up to 5 lines may still be announced past the cancel, but internally the generator won't proceed past that iteration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants