Skip to content

Block touch input while the magnifier is running#20357

Open
Boumtchack wants to merge 10 commits into
nvaccess:betafrom
France-Travail:feature/disableTouchscreen
Open

Block touch input while the magnifier is running#20357
Boumtchack wants to merge 10 commits into
nvaccess:betafrom
France-Travail:feature/disableTouchscreen

Conversation

@Boumtchack

@Boumtchack Boumtchack commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Link to issue number:

Closes #20038

Summary of the issue:

Touch screen input causes incorrect mappings when the NVDA magnifier is running. Touch input must be disabled while the magnifier is active to prevent this conflict.

Description of user facing changes:

On installed copies of NVDA, touch gestures are silently blocked while the magnifier is running. An earcon plays on each blocked gesture so the user knows the touch was received but suppressed. On portable copies, copies running from source, or without UI access, a warning dialog appears when the magnifier starts to inform the user that touch input cannot be intercepted and may not behave as expected.

Description of developer facing changes:

A blockTouchInput: bool flag is added to touchHandler. In TouchHandler.pump(), when the flag is True, gestures are skipped and an earcon is played instead of executing the gesture. Magnifier._startMagnifier sets touchHandler.blockTouchInput = True if the handler is active, or schedules a gui.messageBox warning via wx.CallAfter otherwise. Magnifier._stopMagnifier unconditionally resets the flag to False.

Description of development approach:

Keeps the touch handler running and blocks only at the gesture execution layer in pump(), which runs on the main thread and avoids any threading concerns with audio feedback.

Testing strategy:

unit

Known issues with pull request:

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@Boumtchack Boumtchack marked this pull request as ready for review June 17, 2026 15:08
@Boumtchack Boumtchack requested a review from a team as a code owner June 17, 2026 15:08
@Boumtchack Boumtchack requested a review from seanbudd June 17, 2026 15:08
@SaschaCowley

Copy link
Copy Markdown
Member

@Boumtchack this approach will not work. Disabling NVDA's touch support does *NOT disable touchscreen input, it simply disables NVDA's processing of touch events. Arguably this makes the situation worse, as touching the screen in this state will silently perform actions. While we have no way of preventing this in portable copies (we can't intercept touch inputs in portables), we already intercept all touch events when touch support is enabled in installed copies.

What we need to do is continue to block touch inputs but alert the user that touch support is unavailable. Of course, if NVDA's touch support is disabled, or NVDA is running from source, portable, or (I think) without UI access, we still need to pop up a dialog alerting the user that the touch screen will not behave as expected while the magnifier is running.

@SaschaCowley SaschaCowley marked this pull request as draft June 17, 2026 23:33
@Boumtchack Boumtchack marked this pull request as ready for review June 22, 2026 13:32
@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Jun 23, 2026
@seanbudd seanbudd requested a review from SaschaCowley June 23, 2026 02:17
@seanbudd

Copy link
Copy Markdown
Member

@SaschaCowley - can you check the approach now?

@SaschaCowley SaschaCowley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the logic is flawed, but this is the right direction.

Comment thread source/_magnifier/magnifier.py Outdated
Comment thread source/_magnifier/magnifier.py Outdated
Comment thread source/touchHandler.py Outdated
Comment thread tests/unit/test_magnifier/test_magnifier.py
@SaschaCowley SaschaCowley marked this pull request as draft June 23, 2026 06:14
@Boumtchack Boumtchack marked this pull request as ready for review June 24, 2026 06:28
@seanbudd seanbudd requested a review from SaschaCowley June 25, 2026 04:12
Comment thread tests/unit/test_magnifier/test_magnifier.py Outdated
Comment thread source/_magnifier/magnifier.py Outdated
Comment thread source/_magnifier/magnifier.py Outdated
@seanbudd seanbudd added this to the 2026.2 milestone Jun 25, 2026
@seanbudd seanbudd marked this pull request as draft June 29, 2026 05:06
@Boumtchack Boumtchack marked this pull request as ready for review June 29, 2026 14:03
Comment thread tests/unit/test_magnifier/test_magnifierTouchscreen.py Outdated
Comment thread tests/unit/test_magnifier/test_magnifierTouchscreen.py Outdated
@hwf1324

hwf1324 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Please change the title of this PR to avoid confusion.

@Boumtchack

Copy link
Copy Markdown
Contributor Author

how would you change the title? it seems ok to me

@hwf1324

hwf1324 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

For example: “Block touch input while the magnifier is running”?

@Boumtchack Boumtchack changed the title touchscreen and magnifier support Block touch input while the magnifier is running Jul 1, 2026
Copilot AI review requested due to automatic review settings July 7, 2026 15:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to prevent mis-mapped touchscreen behavior while NVDA’s magnifier is active by blocking NVDA touch gesture execution when touch interception is available, and warning the user when interception isn’t possible (portable/source/no UI access scenarios).

Changes:

  • Add a touchHandler.blockTouchInput flag and skip gesture execution in TouchHandler.pump() while the flag is set.
  • Update magnifier start/stop logic to enable/disable touch blocking, or show a warning dialog when touch input can’t be intercepted.
  • Add/adjust unit tests covering magnifier + touchscreen interactions.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File Description
tests/unit/test_magnifier/test_magnifierTouchscreen.py New unit tests for touch blocking / warning behavior when magnifier starts/stops.
tests/unit/test_magnifier/test_magnifier.py Test setup/teardown adjustments and focus coordinate mocking to support magnifier behavior in tests.
source/touchHandler.py Add global blockTouchInput and block gesture execution (with feedback) during pump.
source/_magnifier/magnifier.py Toggle touch blocking on magnifier start/stop; warn when touch interception isn’t available.

Comment thread source/_magnifier/magnifier.py
Comment thread source/_magnifier/magnifier.py
Comment on lines +204 to +208
reason = pgettext(
"magnifier",
# Translators: Warning when the magnifier starts on a portable/source copy with a touchscreen.
"Touch screen input cannot be used because NVDA is not installed. ",
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
reason = pgettext(
"magnifier",
# Translators: Warning when the magnifier starts on a portable/source copy with a touchscreen.
"Touch screen input cannot be used because NVDA is not installed. ",
)
reason = pgettext(
"magnifier",
# Translators: Warning when the magnifier starts on a portable/source copy with a touchscreen.
"Touch screen input is not supported in this version of NVDA. ",
)

Comment thread source/_magnifier/magnifier.py
Comment on lines 292 to +295
self._stopTimer()
self._isActive = False
if touchHandler.handler is not None:
touchHandler.blockTouchInput = False
Comment thread source/touchHandler.py
Comment on lines 395 to +398
for preheldTracker, tracker in self.trackerManager.emitTrackers():
if blockTouchInput:
winsound.MessageBeep(winsound.MB_ICONASTERISK)
continue

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think this might be too noisy.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for preheldTracker, tracker in self.trackerManager.emitTrackers():
if blockTouchInput:
winsound.MessageBeep(winsound.MB_ICONASTERISK)
continue
if blockTouchInput:
winsound.MessageBeep(winsound.MB_ICONASTERISK)
return
for preheldTracker, tracker in self.trackerManager.emitTrackers():

Comment on lines +58 to +68
def testStopMagnifierUnblocksTouchInput(self):
"""blockTouchInput is reset to False when the magnifier stops with the touch handler active."""
self.magnifier._stopTimer = MagicMock()
self.magnifier._isActive = True
touchHandler.blockTouchInput = True
self.addCleanup(setattr, touchHandler, "blockTouchInput", False)

with patch("touchHandler.handler", new=MagicMock()):
self.magnifier._stopMagnifier()

self.assertFalse(touchHandler.blockTouchInput)
seanbudd
seanbudd previously approved these changes Jul 8, 2026

@seanbudd seanbudd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Boumtchack

"magnifier",
# Translators: Warning when magnifier is started on a device with a touch screen but NVDA's touch support is disabled.
"Touch screen input cannot be used because NVDA touch support is disabled. ",
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't this message also appear if touch support is enabled?

also maybe I missed it but do we have a warning message for when we block touch input

@seanbudd seanbudd self-requested a review July 8, 2026 02:13
@seanbudd seanbudd dismissed their stale review July 8, 2026 02:13

stale

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

Labels

conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants