Skip to content

Fix completed WISP touch promotion across modal dialogs - #12034

Open
amarinov-msft wants to merge 3 commits into
dotnet:mainfrom
amarinov-msft:fix/2491-modal-touch-promotion
Open

amarinov-msft wants to merge 3 commits into
dotnet:mainfrom
amarinov-msft:fix/2491-modal-touch-promotion

Conversation

@amarinov-msft

@amarinov-msft amarinov-msft commented Oct 14, 2026 •

Copy link
Copy Markdown

Fixes #2491

Main PR: Not applicable; this is the main-branch fix, not a backport.

Description

Fix WISP touch-to-mouse promotion when a cancelled manipulation replays a mouse-down handler that synchronously opens a modal dialog.

With a panning ScrollViewer, a tap can be buffered until touch-up cancels manipulation. Replaying the buffered mouse down from inside touch-up can enter ShowDialog() before the original touch device deactivates. That device retains mouse-promotion ownership, preventing subsequent dialog touches from generating mouse events until WISP reuses its cursor ID. The reproduction on this machine ignored ten touches and first clicked on touch eleven; that count is device/environment dependent.

This change:

  • Defers completed-touch cancellation replay until touch-up and deactivation finish, including cancellation triggered by capture release during deactivation. Handled PreviewStylusUp and StylusUp use the same completion scope without reporting a suppressed TouchUp. Mid-contact cancellation remains immediate.
  • Detaches the replay queue and checks contact generations and queue revisions so reentrant callbacks cannot consume or clear newer input.
  • Tracks the replay's actual mouse source and per-button ownership. Disabling or disposing that source retires only its still-owned press, without delivering a synthetic mouse-up to a disabled/disposed source.
  • Resolves replay eligibility and press receipts across nested operations and prevents stale cached-down writes after outbound callbacks.
  • Retires stale replay bookkeeping when real mouse input takes over its button, and tracks that affected button through physical release even after the replay returns. A held physical press survives old replay cleanup; source re-enable resynchronizes it from system state. Unrelated legacy mouse/stylus button handling is unchanged.

Adds 46 deterministic bookkeeping/ownership cases in PresentationCore.Tests and 14 opt-in native cases in PresentationFramework.Tests, plus the project references needed by the exercised input/UI paths.

The product change is WISP-only: no public API or pointer-stack changes. The separate non-panning, live-contact modal variant is outside this fix's scope.

Related: #194 and #11999. I also tested the unchanged native regressions with only the product patch from #11999 at 5f54e6051b42fa8f27ca1c1b5a67cdcc183570da, on the same upstream baseline and without this PR's product changes. Its Release build improved the first successful dialog click from touch eleven to touch one, but still missed the Click on touch ten. A fresh process running only the three modal cases reproduced the same failure; both Click-based controls passed. Its full Release native suite had 9 passes and 3 failures (the modal case, handled-TouchUp completion, and throwing-handler recovery). Its Debug suite aborted on a WISP assertion during the modal case. This is evidence of overlapping behavior, not a claim that every scenario reported under the two issues is identical or that the other failures were introduced by that PR.

Customer Impact

Touch-operated applications opening modal dialogs from promoted mouse-down handlers can appear unresponsive: touch events reach the dialog, but Buttons and other mouse-dependent controls do not receive the corresponding mouse events. Customers may need repeated taps before controls start responding.

The regression verifies that every one of 36 dialog touches clicks across three open/close cycles, rather than checking only eventual recovery.

Regression

No known recent-release regression. The issue is longstanding and has also been reported on .NET Framework.

Testing

Validated locally on Windows, x64 Debug, at commit 98a75a9bf310853d185635b75a86e1bdbb64a922, based on upstream 5edf2b293. Tests target net11.0-windows and ran on .NET 12.0.0-alpha.1.26509.101. The commands/results below were rerun after addressing the two review findings.

Before the corrections, seven new managed cases failed on the original PR product code: handled-preview-up deactivation, left/right physical press/release followed by another touch, and held physical-button cleanup. All pass after correction. Additional coverage checks release after the replay has left the active stack, nested left/right takeover, release without an observed press, and preservation of unrelated legacy button behavior.

Builds:

$env:LC_ALL = 'C'; $env:LANG = 'C'; .\build.cmd -projects 'src\Microsoft.DotNet.Wpf\tests\UnitTests\PresentationCore.Tests\PresentationCore.Tests.csproj' -platform x64 -msbuildEngine vs -verbosity quiet -binaryLogName Issue2491Review-PresentationCore.Tests.binlog

Result: passed, 0 warnings, 0 errors; exit 0.

$env:LC_ALL = 'C'; $env:LANG = 'C'; .\build.cmd -projects 'src\Microsoft.DotNet.Wpf\tests\UnitTests\PresentationFramework.Tests\PresentationFramework.Tests.csproj' -platform x64 -msbuildEngine vs -verbosity quiet -binaryLogName Issue2491Review-PresentationFramework.Tests.binlog

Result: passed, 0 warnings, 0 errors; exit 0. Builds used existing installed tools/artifacts; clean-machine bootstrap is not validated.

Core input compatibility:

$env:DOTNET_ROOT = (Resolve-Path .dotnet).Path; & .\artifacts\bin\PresentationCore.Tests\x64\Debug\net11.0-windows\PresentationCore.Tests.exe --filter-class 'System.Windows.Input.*' --no-progress --report-xunit-xml --report-xunit-xml-filename Issue2491ReviewInput.xml --timeout 2m

Result: 249 passed, 0 failed, 0 skipped; exit 0. Includes all 46 new WISP cases, including nested-operation coverage. These managed ownership/lifetime models are not physical-device tests.

Native desktop touch regressions:

$env:DOTNET_ROOT = (Resolve-Path .dotnet).Path; $env:WPF_TEST_DESKTOP_TOUCH = '1'; try { & .\artifacts\bin\PresentationFramework.Tests\x64\Debug\net11.0-windows\PresentationFramework.Tests.exe --filter-class System.Windows.Input.WispModalTouchTests --no-progress --report-xunit-xml --report-xunit-xml-filename Issue2491ReviewNative.xml --timeout 2m; $testExit = $LASTEXITCODE } finally { Remove-Item Env:WPF_TEST_DESKTOP_TOUCH }; exit $testExit

Result: 14 passed, 0 failed, 0 skipped; exit 0. Covers the modal/control matrix, handled touch input, successful panning/inertia, mid-contact cancellation, source closure, throwing-handler recovery, simultaneous contacts, and cross-window mouse capture transfer. The two new modal cases handle PreviewStylusUp and StylusUp; both require deactivation before opening the dialog, no promoted TouchUp, and every one of 36 dialog touches clicking across three cycles.

This is guarded native touch injection on an explicitly idle interactive desktop. The tests assert locally loaded WPF assemblies and observed WISP touch devices; foreground, process-ownership and unobscured-window guards stay enabled. The modal regression failed before the fix with first successful dialog touch eleven and passes with this fix for every touch across all three cycles.

Default opt-in gating:

$env:DOTNET_ROOT = (Resolve-Path .dotnet).Path; if (Test-Path Env:WPF_TEST_DESKTOP_TOUCH) { Remove-Item Env:WPF_TEST_DESKTOP_TOUCH }; & .\artifacts\bin\PresentationFramework.Tests\x64\Debug\net11.0-windows\PresentationFramework.Tests.exe --filter-class System.Windows.Input.WispModalTouchTests --no-progress --report-xunit-xml --report-xunit-xml-filename Issue2491ReviewDefault.xml --timeout 1m

Result: 14 skipped, 0 executed; expected all-skipped exit 8. Do not enable native injection on a shared/unattended desktop or run another desktop test executable concurrently.

Neighboring MessageBox coverage:

$env:DOTNET_ROOT = (Resolve-Path .dotnet).Path; & .\artifacts\bin\PresentationFramework.Tests\x64\Debug\net11.0-windows\PresentationFramework.Tests.exe --filter-class '*MessageBoxTests' --no-progress --report-xunit-xml --report-xunit-xml-filename Issue2491ReviewMessageBox.xml --timeout 1m

Result: 58 passed, 0 failed, 0 skipped; exit 0.

git diff --check

Result: passed, exit 0. The staged review-correction diff was also checked with git diff --cached --check: passed.

Pending / not run: physical touch-device validation by a teammate; physical mouse/pen and monitor/DPI combinations; native nested contact reuse/hot-unplug and source closure specifically during a pre-press move; full OLE drag/drop resynchronization; pointer-stack runtime testing; the full repository suite. Targeted DRT binaries were unavailable, so DRTs were not run. The original non-panning live-contact variant was not revalidated on the final binaries.

Risk

Medium. This changes reentrant WISP touch-up/manipulation-cancellation ordering and shared mouse-state bookkeeping. The completed-touch path intentionally finishes touch-up and deactivation before replaying buffered mouse input.

Risk is constrained by leaving immediate mid-contact cancellation and the pointer implementation unchanged, checking source identity/contact generation/queue revision, and releasing only a provably owned mouse press. Deterministic and native tests cover nested operations, primary/secondary contacts, source lifetime and capture transfer, but do not replace hardware validation.

This PR is a draft pending physical-device testing and review of the input-lifecycle changes.

Microsoft Reviewers: Open in CodeFlow

Defer cancelled manipulation replay until touch-up and deactivation finish. Track contact generations and owned mouse presses across nested replay, source disable/disposal and contact reuse. Add guarded native regressions and managed ownership coverage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1fe51848-46fe-4323-8746-5267d21e7788
@dotnet-policy-service dotnet-policy-service Bot added the PR metadata: Label to tag PRs, to facilitate with triage label Oct 14, 2026
@amarinov-msft
amarinov-msft requested a balanced review from Copilot October 14, 2026 09:00

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.

🟡 Changes recommended

Physical mouse input and handled StylusUp paths can still leave promotion state inconsistent or replay before deactivation completes.

2 open findings
What changed in this PR

Fixes #2491 by deferring WISP touch-to-mouse replay until touch-up deactivation completes.

Changes:

  • Adds generation-aware replay and mouse-button ownership tracking.
  • Handles source disposal, nested input, and reentrancy.
  • Adds managed and opt-in native touch regression coverage.
File Description
WispLogic.cs Implements deferred replay and ownership bookkeeping.
WispStylusDevice.cs Integrates replay lifecycle with mouse promotion.
WispStylusTouchDevice.cs Tracks contact generations and deferred promotion.
WispStylusTouchDeviceTests.cs Tests replay bookkeeping and ownership.
WispModalTouchTests.cs Adds native modal-touch regressions.
PresentationCore.Tests.csproj Adds required test dependencies.
PresentationFramework.Tests.csproj Adds required UI test dependencies.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Retire completed replay bookkeeping before real mouse takeover and reconcile the affected button through its physical release. Share the completed-touch deactivation scope with handled PreviewStylusUp and StylusUp without reporting TouchUp. Add managed ownership and native modal regressions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 1fe51848-46fe-4323-8746-5267d21e7788

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.

🟡 Changes recommended

Drag/drop resynchronization can invalidate an active replay receipt without clearing its stylus mouse-down state.

1 open finding
2 resolved since last review

🧠 Review effort: Balanced

Clear matching replay device state before invalidating button ownership on drag/drop exit and source re-enable. Preserve physically held buttons and newer contacts, and cover both resynchronization paths with managed regressions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 466a2938-008f-4030-912a-904ed8296d05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-Touch Included in test pass PR metadata: Label to tag PRs, to facilitate with triage Status:Completed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Touch bug/issue with ShowDialog and TextBox

2 participants