Repository navigation
Add Mouse Button Lock (ClickLock for left, right, and middle mouse buttons) - #49279
Niels Laute (niels9001) merged 35 commits into
Conversation
New Mouse Utilities sub-module: hold the right or middle mouse button past a configurable threshold and release, and the OS keeps perceiving the button as held (ClickLock semantics). Tap the same button again to release. Includes a move-cancel dead-zone so a drag is not mistaken for a lock, and releases any held button on disable or shutdown so nothing gets stranded. Native C++ in-process module (WH_MOUSE_LL hook on a dedicated thread, suppresses the button-up and injects a tagged synthetic release via SendInput), modeled on the other Mouse Utilities. Settings surface as a new section on the shared Mouse Utilities page; default off, no activation hotkey. Ported from https://github.com/owenpkent/windows-right-click-lock and generalized to cover both the right and middle buttons. Proposal: microsoft#48302
Addresses the actionable findings from a manual + multi-agent review of the module. No behavior change for valid input; these are reliability and defense-in-depth fixes in the authored files. - parse_settings: catch(...) so a winrt::hresult_error from the JSON accessors (which does not derive from std::exception) cannot escape and abort settings-apply or module construction. - readInt: clamp hold-duration to [50, 60000] ms and move-cancel to [0, 10000] px before the double-to-int cast. Closes the out-of-range float-to-int UB, and prevents a 0 ms hold from latching every click. - enable(): bail if CreateEventW fails instead of starting a hook thread that would busy-spin on a null wait handle; also break the wait loop on WAIT_FAILED. - Tap-to-release now claims the lock with exchange() and clears it even when the synthetic up injection fails, so the state machine cannot disagree with the OS and strand a later click. - g_instance and m_enabled are atomic; the hook proc loads the singleton once. - ReleaseButton logs when a synthetic up injection fails. - Settings viewmodel repairs null MouseButtonLock sub-properties to defaults before reading, so a hand-edited settings.json with explicit nulls cannot crash the Mouse Utilities page. - Docs: surface the WH_MOUSE_LL raw/exclusive-input limitation to users, and track session-lock release as a parity follow-up.
- dllmain.cpp: cast the move-cancel delta operands to 64-bit before the subtraction so the C++ Core Guidelines analysis (C26451, enforced as an error in PowerToys) is satisfied. - Dev doc: add a "Building and debugging" section. Verified with a local x64 Release build: version/logger/SettingsAPI and the module all compile, the module links into PowerToys.MouseButtonLock.dll exporting powertoy_create, and C++ code analysis is clean.
- Extract the per-button state machine into MouseButtonLockCore.h: a Win32-free Engine where the clock is passed in and synthetic-up injection is behind an IButtonUpInjector interface. dllmain.cpp becomes a thin Win32 adapter over it (no behavior change). - Add MouseButtonLock.UnitTests (CppUnitTest): 15 tests covering hold-to-lock, exact/under threshold, tap-to-release including injection failure, move-cancel (in/out of dead-zone, after threshold, disabled), RMB/MMB independence, disabled-button, enforce-enabled release, release-all, and stale-hold reset. Registered in PowerToys.slnx; runs in CI via the solution Build;Test target (no pipeline edit needed for a CppUnitTest project). - Add a 36x36 MouseButtonLock.png settings/dashboard icon. Verified locally: the module rebuilds and links cleanly with the engine (clean code analysis); the test project builds and all 15 tests pass.
Add MouseButtonLock.FuzzTests, a C++ libFuzzer target over the Win32-free Engine, satisfying the PowerToys requirement that user-input modules implement fuzzing. Registered in the solution under MouseUtils/Tests with ARM64 disabled; the OneFuzz pipeline's existing tests/*.FuzzTests glob picks it up, so no pipeline edit is needed. OneFuzzConfig.json follows the repo's v3 schema. Writing the target surfaced a latent signed overflow in CheckMoveCancel (dx*dx + dy*dy computed in long long for extreme coordinates). Compute the squared distance in double instead. It is unreachable in production (the cursor is screen-bounded) and behavior-preserving for in-range inputs; the 15 unit tests still pass. Local run: 815,727 inputs, zero crashes, no ASan findings. Document the fuzzing setup and the remaining parity gaps in the dev doc.
…scope Add a Mouse Button Lock section to DATA_AND_PRIVACY.md documenting the MouseButtonLock_EnableMouseButtonLock TraceLogging event the module raises on enable/disable, matching the sibling MouseUtils entries. Include MouseButtonLock in the MouseUtils.UITests restart scope, matching the level of integration CursorWrap has. Record both, plus the Command Palette and dedicated-UI-test follow-ups, in the dev doc.
Add a Settings UI section with an ASCII mockup of the Mouse Button Lock group on the Mouse utilities page, a symbol legend, and a control-to-key/ViewModel/default mapping. Derived from MouseUtilsPage.xaml and the en-us resources so the layout, labels, control ranges, and automation ids match the shipping UI.
…status Mark the Command Palette toggle as decided (skip: the module has no activation to bind and settings are already reachable via Open in Settings), and update the AGENTS.md cross-check note now that the work is committed.
Extend the ClickLock engine and Win32 hook to cover the left (primary) mouse button alongside right and middle, so all three lock in one place instead of left ClickLock living in Windows settings and right/middle in PowerToys. Left-button locking is off by default: it is the primary interaction button and Windows already ships ClickLock for it. - Engine (MouseButtonLockCore.h): add MouseButton::Left and lmbEnabled; extend the per-button state machine, move-cancel, EnforceEnabled, ReleaseAll, and ResetTransient to the third button. - Hook (dllmain.cpp): handle WM_LBUTTONDOWN/WM_LBUTTONUP and inject MOUSEEVENTF_LEFTUP; parse the new lmb_lock_enabled setting. - Settings: add LmbLockEnabled property, the "Lock the left (primary) mouse button" checkbox, strings, and MouseUtilsViewModel wiring. - Tests: 3 new engine tests (off-by-default, locks-when-enabled, independent of right/middle); fuzz target now drives all three buttons. - Docs and spell-check dictionary updated.
github-actions
Bot
added
the
Product-Mouse Utilities
|
Thank you for contributing to PowerToys. We've detected that this PR might include a new or modified telemetry event. After this PR is merged, please follow these next steps:
|
|
@microsoft-github-policy-service agree |
|
Hi Niels Laute (@niels9001), thanks again for chatting about this idea. The PR is up: it adds Mouse Button Lock, a ClickLock equivalent for the left, right, and middle mouse buttons (implements #48302). It follows the passive MouseUtils pattern (CursorWrap-style, native C++ in-process module), with 18 unit tests over the engine and a libFuzzer target. Since it's my first PR to the repo, the CI workflows are waiting on a maintainer to approve them to run. Would you be able to kick those off, or point me to the right reviewer? Thank you! |
There was a problem hiding this comment.
Pull request overview
Adds a new Mouse Utilities sub-module, Mouse Button Lock, implemented as an in-process native C++ module with Settings UI integration, GPO wiring, telemetry event logging, and supporting tests/docs.
Changes:
- Introduces
PowerToys.MouseButtonLock.dll(WH_MOUSE_LL hook + SendInput) and a Win32-free engine with C++ unit tests + libFuzzer target. - Wires Settings UI (toggle + per-button options), serialization contexts, module enablement plumbing, and Runner module loading.
- Adds GPO policy + wrapper APIs, telemetry documentation entry, and developer docs for the new module.
Reviewed changes
Copilot reviewed 49 out of 50 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/settings-ui/Settings.UI/ViewModels/MouseUtilsViewModel.cs | Adds Mouse Button Lock settings binding, IPC send, persistence, and GPO-aware enable state. |
| src/settings-ui/Settings.UI/Strings/en-us/Resources.resw | Adds localized strings for the new settings section. |
| src/settings-ui/Settings.UI/SettingsXAML/Views/MouseUtilsPage.xaml.cs | Injects MouseButtonLock settings repository into the page ViewModel. |
| src/settings-ui/Settings.UI/SettingsXAML/Views/MouseUtilsPage.xaml | Adds UI section (toggle + expander) for Mouse Button Lock options. |
| src/settings-ui/Settings.UI/SerializationContext/SourceGenerationContextContext.cs | Registers MouseButtonLockSettings for JSON source generation. |
| src/settings-ui/Settings.UI/Helpers/ModuleGpoHelper.cs | Adds GPO lookup + page routing for the new module type. |
| src/settings-ui/Settings.UI.Library/SndMouseButtonLockSettings.cs | Adds IPC wrapper for sending MouseButtonLock settings to Runner/module. |
| src/settings-ui/Settings.UI.Library/SettingsSerializationContext.cs | Adds JSON serialization metadata for MouseButtonLock settings/properties and IPC wrapper types. |
| src/settings-ui/Settings.UI.Library/MouseButtonLockSettings.cs | Adds module settings model and module type mapping for Settings UI. |
| src/settings-ui/Settings.UI.Library/MouseButtonLockProperties.cs | Defines per-button enable flags and behavior parameters with defaults. |
| src/settings-ui/Settings.UI.Library/Helpers/ModuleHelper.cs | Adds label resource name, enable/disable mapping, and module key for MouseButtonLock. |
| src/settings-ui/Settings.UI.Library/EnabledModules.cs | Adds global module enablement flag for MouseButtonLock. |
| src/settings-ui/QuickAccess.UI/Helpers/ModuleGpoHelper.cs | Adds GPO lookup for QuickAccess surface. |
| src/runner/main.cpp | Adds MouseButtonLock DLL to Runner’s known module list. |
| src/modules/MouseUtils/MouseUtils.UITests/FindMyMouseTests.cs | Ensures MouseButtonLock is included in the UI test restart scope. |
| src/modules/MouseUtils/MouseButtonLock/trace.h | Declares telemetry events for enable/disable. |
| src/modules/MouseUtils/MouseButtonLock/trace.cpp | Implements telemetry event write for enable/disable. |
| src/modules/MouseUtils/MouseButtonLock/resource.h | Adds resource ID(s) for module strings. |
| src/modules/MouseUtils/MouseButtonLock/pch.h | Adds module PCH header for common includes. |
| src/modules/MouseUtils/MouseButtonLock/pch.cpp | Builds the module PCH. |
| src/modules/MouseUtils/MouseButtonLock/packages.config | Adds CppWinRT package reference for native build. |
| src/modules/MouseUtils/MouseButtonLock/MouseButtonLockCore.h | Adds Win32-free per-button ClickLock state machine (engine). |
| src/modules/MouseUtils/MouseButtonLock/MouseButtonLock.vcxproj.filters | Adds VS filters for the new native project. |
| src/modules/MouseUtils/MouseButtonLock/MouseButtonLock.vcxproj | Adds native DLL project emitting PowerToys.MouseButtonLock.dll. |
| src/modules/MouseUtils/MouseButtonLock/MouseButtonLock.rc | Adds version info + display name resources. |
| src/modules/MouseUtils/MouseButtonLock/dllmain.cpp | Implements PowertoyModuleIface, hook thread, settings parse, and input suppression/injection. |
| src/modules/MouseUtils/MouseButtonLock.UnitTests/pch.h | Adds unit test PCH header. |
| src/modules/MouseUtils/MouseButtonLock.UnitTests/pch.cpp | Builds the unit test PCH. |
| src/modules/MouseUtils/MouseButtonLock.UnitTests/MouseButtonLock.UnitTests.vcxproj.filters | Adds VS filters for the unit test project. |
| src/modules/MouseUtils/MouseButtonLock.UnitTests/MouseButtonLock.UnitTests.vcxproj | Adds C++ unit test project for the engine. |
| src/modules/MouseUtils/MouseButtonLock.UnitTests/EngineTests.cpp | Adds unit tests covering lock, move-cancel, enable enforcement, and lifecycle resets. |
| src/modules/MouseUtils/MouseButtonLock.FuzzingTest/OneFuzzConfig.json | Adds OneFuzz job configuration for fuzz target. |
| src/modules/MouseUtils/MouseButtonLock.FuzzingTest/MouseButtonLock.FuzzingTest.vcxproj | Adds libFuzzer/ASan fuzz target project. |
| src/modules/MouseUtils/MouseButtonLock.FuzzingTest/MouseButtonLock.FuzzingTest.filters | Adds VS filters for fuzz project. |
| src/modules/MouseUtils/MouseButtonLock.FuzzingTest/MouseButtonLock.FuzzingTest.cpp | Adds fuzz harness that decodes bytes into engine events/settings mutations. |
| src/gpo/assets/PowerToys.admx | Adds GPO policy entry for configuring Mouse Button Lock enabled state. |
| src/gpo/assets/en-US/PowerToys.adml | Adds localized GPO policy string for Mouse Button Lock. |
| src/common/utils/gpo.h | Adds policy constant and helper accessor for MouseButtonLock enabled state. |
| src/common/ManagedCommon/ModuleType.cs | Adds new module type enum value for MouseButtonLock. |
| src/common/logger/logger_settings.h | Adds logger channel name for MouseButtonLock. |
| src/common/GPOWrapper/GPOWrapper.idl | Exposes MouseButtonLock GPO getter to managed callers. |
| src/common/GPOWrapper/GPOWrapper.h | Declares MouseButtonLock GPO getter in wrapper. |
| src/common/GPOWrapper/GPOWrapper.cpp | Implements MouseButtonLock GPO getter via powertoys_gpo helper. |
| PowerToys.slnx | Adds MouseButtonLock module, unit test, and fuzz test projects to solution. |
| doc/devdocs/modules/mouseutils/readme.md | Updates Mouse Utilities devdoc overview to include Mouse Button Lock. |
| doc/devdocs/modules/mouseutils/mousebuttonlock.md | Adds detailed devdoc for Mouse Button Lock behavior, architecture, and settings. |
| DATA_AND_PRIVACY.md | Registers the MouseButtonLock enable/disable telemetry event in privacy docs. |
| .pipelines/ESRPSigning_core.json | Adds PowerToys.MouseButtonLock.dll to ESRP signing list. |
| .github/actions/spell-check/expect.txt | Adds new tokens to spell-check allowlist. |
| - **[Mouse Highlighter](mousehighlighter.md)**: Visualizes mouse clicks with customizable highlights | ||
| - **[Mouse Jump](mousejump.md)**: Allows quick cursor movement to specific screen locations | ||
| - **[Mouse Pointer Crosshairs](mousepointer.md)**: Displays crosshair lines that follow the mouse cursor | ||
| - **[Mouse Button Lock](mousebuttonlock.md)**: A ClickLock equivalent for the right and middle mouse buttons - hold the button to lock it down, tap to release |
There was a problem hiding this comment.
Fixed in a2d97a2: the Mouse Utilities overview now states the module covers the left, right, and middle buttons (left optional and off by default), matching the implementation and the module doc page.
Justine Spencer (queenjustice1990-web)
left a comment
Justine Spencer (queenjustice1990-web)
left a comment
There was a problem hiding this comment.
noemi
…fixes Hold-duration control: - Replace the NumberBox with a Slider that snaps in 200 ms steps (SnapsTo=StepValues) across 200-2200 ms, matching Windows' built-in ClickLock slider, with a live "N ms" label (MillisecondsLabelConverter). - Change the default from 300 ms to 1200 ms (the Windows ClickLock default and the 200-2200 midpoint) across every mirrored site: C# properties, the ViewModel fallback, C++ DEFAULT_HOLD_DURATION_MS, and the engine Settings struct. - Raise the hand-edit clamp floor from 50 ms to 200 ms; 50 ms sat inside the ordinary-click duration band and would latch on normal clicks. - Pin the engine test baseline to 300 ms in DefaultSettings() so the hold-mechanics tests stay valid against the new 1200 ms struct default. PR review (microsoft#49279): - readInt now rejects non-finite JSON numbers: a NaN slipped past the min/max clamp and made static_cast<int> undefined. - Drop the redundant deep-relative TraceBase.h include from trace.cpp (trace.h already includes it via the angle-bracket form). - Fix the Mouse Utilities readme to state the module covers the left, right, and middle buttons (left optional, off by default). Dev doc updated to match.
Extends the ClickLock-style hold-to-lock into a usable hands-free drag for all three buttons (Windows' built-in ClickLock covers the left only): hold a button past the threshold to lock it held, move to drag (text selection, window/file drag, CAD/3D camera), and click to release. Settings: - Hold-duration slider now steps in 100 ms increments, so 300 ms is reachable. - A single "Allow a held drag to lock" toggle (default off) plus a drag-threshold pixel value replaces the contradictory "cancel on move" checkbox: by default any drag cancels the pending lock; enable the toggle to let a sustained hold-and-drag lock (gaming camera pan). Engine / hook: - Lock by suppressing the physical button-up, so the button stays held without the original click completing (no context menu or paste at lock time). - Defer synthetic injection to the hook thread's message loop (WM_MBL_INJECT): calling SendInput inside the low-level hook callback leaked the suppressed event to apps and collapsed text selections on release. Deferring it until after the callback returns fixes that; a selection made during the hold now survives release. - Release every held button on ANY button-down, not only a same-button tap. A held button keeps mouse capture, so other clicks were dead until the exact same button was tapped; now a left-click frees a right-lock, etc. - After a right-button release, inject Esc to dismiss the context menu the release up necessarily opens, keeping hands-free right-drag usable. Normal quick right-clicks never lock and still open menus. Tests/docs: update EngineTests (18 pass) and the fuzz injector for the up-only IButtonUpInjector, and rewrite the dev doc for the final mechanism.
The Mouse Utilities readme still described releasing the lock as "tap to release"; any button press now releases a held button. Also note the hands-free dragging purpose.
…ument elevated/UIAccess limits - Remove the "Allow a held drag to lock" (drag_locks_enabled) setting across the engine, native adapter, C# settings, ViewModel, XAML, resw, unit test, and fuzz target. Cancel-on-drag is now the only behavior; the drag-threshold dead-zone stays exposed and always applies. - Add a recommendation InfoBar to the "Buttons and behavior" expander: recommends the right and middle buttons and points to Windows' built-in ClickLock (Control Panel > Mouse > Buttons) for the left. It renders as a sibling below the expander (an InfoBar cannot be hosted inside SettingsExpander.Items) with visibility bound to the expander's IsExpanded. - Document the elevated / UIAccess window limitation (Device Manager, on-screen keyboards) and the run-as-admin workaround in the dev doc; note it in the settings description too. - Fix a stale OnButtonUp comment; add osk/UIPI spell-check tokens.
|
Hi Niels Laute (@niels9001), quick update: I think I've worked out all the bugs now, so this should be ready for a proper look whenever you have a chance. Since my last note I've:
Engine still has full unit-test and libFuzzer coverage. Thanks again, happy to make any changes you'd like. |
|
Hi Niels Laute (@niels9001), gentle nudge on this one. It's been about a week, so just checking in whenever you have a moment. All of Copilot's review comments are resolved, CI is green, and the PR is mergeable. Happy to make any changes you'd like. Thanks! |
|
Hi Niels Laute (@niels9001), following up on our chat. cc Clint Rutkas (@crutkas) Kai Tao (@vanzue) The main blocker is mechanical: as a first-time contributor, the build workflows have never run on this PR. Only Otherwise ready: Copilot comments resolved, no conflicts with Thanks! |
Documentation sweep against the current code: - Dev doc: slider has 21 steps, not 11 (200-2200 ms at 100 ms StepFrequency); unit tests number 17, not 18 (the drag-lock removal in a2dd5f1 cut one, in two places). - Dev doc: the verification harness and fuzzer build script were ad hoc in the gitignored x64\Release\ and are not in the repo, so the "rebuild with build_harness.cmd" and build_mbl_fuzzer.cmd instructions were not actionable from a clean clone. Say so, and keep the reproducible part (the cl.exe ASan invocation). - Settings description still read "Tap the same button again to release"; any button-down releases (ReleaseAllExcept), which was the fix for the stuck-left-click report. - tools/module_loader/SHARING.md: add the missing MouseButtonLock entry alongside the sibling Mouse Utilities modules.
Register MouseButtonLock with the DSC settings resource so the module can be configured declaratively alongside the other utilities, and document it. MouseButtonLockSettings already satisfied the resource's `ISettingsConfig, new()` constraint, so no change to the settings types was needed. The build's manifest generator iterates the resource dictionary, so registration alone produces microsoft.powertoys.MouseButtonLock.settings.dsc.resource.json with no pipeline or installer edit. - SettingsResource.cs: register the module. - SettingsResourceCommandTest: add it to the expected supported-module list (the assertion compares against an exact list). - New SettingsResourceMouseButtonLockModuleTest covering get/set/test/export, following the existing per-module test pattern. - New doc/dsc/modules/MouseButtonLock.md reference page; add the module to the overview table, the settings-resource list, and all three App.md enabled-module lists; cross-link it from the sibling Mouse Utilities pages; add it to the enable/disableAllModules winget examples. Verified locally: full PowerToys.DSC.UnitTests suite passes 47/47 (6 of them the new MouseButtonLock cases), and the DSC resource JSON is emitted.
|
Merged main again (3a52b54); the PR is mergeable. Two conflicts this time:
The other files both modules touch (GPO wrapper and ADMX/ADML, One follow-up commit (47ddcf3): the Mouse Button Lock policy still said Built Release|x64 locally: Settings, runner, the MouseButtonLock module, and the MouseButtonLock, Common.Utils, and DSC test projects. MouseButtonLock engine tests 17/17, DSC tests 106/106, Common.Utils GPO tests 28/28. |
|
Quick round-up of what this PR is waiting on, now that it's mergeable again:
The build workflows also still need a maintainer's approval to run CI on this branch. Thanks all! |
|
Niels Laute (@niels9001) the new conflicts with main are resolved and the PR is mergeable again (summary a couple of comments up). Only the reviews and sign-offs above are left, plus approval for the build workflows. Thanks for keeping this moving! |
Port the legacy MouseButtonLockTests to the .Next suite, following the Auto Hide Cursor tests from microsoft#50271, and remove the legacy copy and its now-unused helper constants. The new tests assert effects rather than control round-trips: the options expander follows the module toggle, the lock checkboxes and the hold duration and move-cancel boundary values persist to settings.json and survive a restart, and a held button actually locks past the configured duration, releases on a same-button tap, and does not lock under the threshold or when its lock is disabled. Lock state is read with GetAsyncKeyState, since the module runs inside the runner with no worker process or named event to observe. The gestures use the left and middle buttons so an unlocked release never opens a context menu, and enable/disable checks retry until the runner has applied the toggle. Add AutomationIds for the module toggle and the three lock checkboxes, and record the coverage in the migration checklist.
|
Gleb Khmyznikov (@khmyznikov) following up on the UI tests question: I went ahead and moved them to What changed:
Both test projects build clean locally (Release|x64). I haven't run the .Next suite end to end yet, so a pipeline or local-VM run would be the real check. |
The Mouse Button Lock UI tests now live in MouseUtils.UITests.Next, so the legacy FindMyMouseTests restart scope no longer needs to enable it. Enabling it there also turned on the right-button lock (on by default) during the legacy Find My Mouse tests. Restore main's module list.
Describe the MouseButtonLockSettingsTests coverage and how it observes lock state, list the new toggle and checkbox AutomationIds, note that the GPO policy targets PowerToys 0.102.0, and point out that the lock test now reproduces the core of the untracked harness verification.
Two fixes carried over from Copilot review comments on PR microsoft#49279, which this module inherited by following the same template: - readInt now rejects non-finite values outright. NaN compares false against both bounds, so it slipped past the clamp into an undefined static_cast<int>. The default-action reader was already safe: its short-circuit range check keeps NaN and out-of-range values away from the cast. - trace.cpp drops the deep-relative TraceBase.h include; trace.h already includes it via the angle-bracket form the sibling modules use.
|
Niels Laute (@niels9001) Gentle follow-up, since it has been ten days without movement. The PR is still mergeable against main and the intake checks pass. What is left is the same three items from the round-up above: Muyuan Li (@MuyuanMS)'s engineering sign-off you asked for on 08-31, a re-review from Boliang Zhang (@LegendaryBlair) (all four requested changes were addressed on 08-11), and approval for the build workflows so CI can run on the branch. Is there anything else you need from my side, or anyone else I should loop in to get this over the line? Happy to merge main again if it drifts before then. |
Muyuan Li (MuyuanMS)
left a comment
Muyuan Li (MuyuanMS)
left a comment
There was a problem hiding this comment.
Please address the release-completion race, refresh the Mouse Button Lock GPO binding state, and correct the DSC examples as noted inline.
| { | ||
| const WPARAM packed = (static_cast<WPARAM>(button) << 1) | (dismissContextMenu ? 1 : 0); | ||
| const DWORD threadId = m_threadId.load(); | ||
| if (threadId != 0 && PostThreadMessageW(threadId, WM_MOUSEBUTTONLOCK_INJECT, packed, 0)) |
There was a problem hiding this comment.
Complete deferred releases before committing engine state
Severity: high
PostThreadMessageW succeeding here only means that the synthetic release was queued. PerformDeferred later discards the actual SendInput result, after the engine has already cleared locked and may have set swallowNextRealUp. If the deferred injection is rejected—for example across a UIPI boundary—the physical down has been suppressed, its paired up can be swallowed, and the OS can remain in a pressed state while the engine believes the lock was released.
Please make this a coordinated change across MouseButtonLockCore.h, dllmain.cpp, and EngineTests.cpp:
- Add a deferred-failure callback to
IButtonUpInjector; register it fromEngineand clear it in the engine destructor. - Have
WinInjector::PerformDeferredreport a failedInjectUpNowthrough that callback while preserving the existing context-menu-dismiss intent. - On deferred failure, clear
swallowNextRealUpfor a same-button release tap; for another release path, restore the logical lock so cleanup can be retried. - Serialize engine operations and serialize
set_configparsing/enforcement withHandleMouseMessage, so disabling a button cannot race with a staleOnButtonUpthat latches afterward. - Add regression tests for both deferred failure paths.
Verification: Build Runner, Settings, Mouse Button Lock, its unit-test project, and its fuzz target; run MouseButtonLock.UnitTests; then perform an elevated-window/UIPI pass and confirm a failed synthetic up cannot leave a button stuck.
There was a problem hiding this comment.
Addressed across 1617bd6 and 7c748ea, following your five points, plus one finding that changed the shape of the fix.
- New
IDeferredFailureSinkinMouseButtonLockCore.h.Engineregisters itself with the injector in its constructor and clears the registration in its destructor.IButtonUpInjector::SetDeferredFailureSinkdefaults to a no-op, so synchronous injectors (the fuzz target's) are unaffected. WinInjector::PerformDeferredis now an instance method. A failedInjectUpNowis reported through the sink with the original button anddismissContextMenu, so the engine sees the release exactly as it requested it; the Esc dismiss still only runs after a successful up.- On a reported failure the engine clears
swallowNextRealUpwhen a release tap's paired up is still pending (that up now performs the release the injection could not deliver). For any other path it restores the logical lock so the next press, settings change or shutdown retries the cleanup.ReleaseButtontreats a synchronous rejection the same way instead of dropping the result. - Every
Engineoperation takes the engine mutex. Indllmain.cpp,set_configholdsm_settingsMutexacrossparse_settings+EnforceEnabled, andHandleMouseMessageholds it across the snapshot + engine call, so a disable cannot interleave with a staleOnButtonUp. - Regression tests in a new
DeferredInjectionFailureclass: sink lifecycle, both tap-release orderings, settings release, cross-button release, synchronous rejection, and a failure report arriving mid-press.
The finding: while verifying the UIPI case I measured that Windows drops a blocked injection silently. With input blocked by an elevated process, the runner's SendInput returned 1, GetLastError said nothing, and the button stayed down; the SendInput docs say the same for UIPI. So a return-value check alone never fires for the scenario you described. 7c748ea adds a second detection: after a deferred SendInput reports success, the injector arms a 50 ms thread timer and reads the button's physical state with GetAsyncKeyState (undoing a primary/secondary swap). Still down means the up never landed, and it reports through the same sink. To keep a fast re-press inside that window from restoring a stale lock, the engine tracks a press even for a disabled button and ignores a report while a physical press of that button is in progress (its own up completes the release).
Verification: module, unit tests and fuzz target build; MouseButtonLock.UnitTests passes 24/24. Against the running runner: a release whose SendInput was forced to fail released the button cleanly through the paired physical up; with input blocked by an elevated process, the settings-driven release logged the dropped injection, restored the lock, and the next tap released the button; the ordinary lock/tap flow logs nothing. Dev doc updated with the repair path, the two detections and the locking.
| OnPropertyChanged(nameof(IsMouseJumpEnabled)); | ||
| OnPropertyChanged(nameof(IsMousePointerCrosshairsEnabled)); | ||
| OnPropertyChanged(nameof(IsCursorWrapEnabled)); | ||
| OnPropertyChanged(nameof(IsMouseButtonLockEnabled)); |
There was a problem hiding this comment.
Refresh the GPO-managed warning state
Severity: medium
InitializeEnabledValues() updates both the effective enabled value and _mouseButtonLockEnabledStateIsGPOConfigured, but this refresh only notifies the former. If policy is added or retracted while Settings is open, the enable card and GPOInfoControl can retain their previous policy-managed state until the page is recreated.
| OnPropertyChanged(nameof(IsMouseButtonLockEnabled)); | |
| OnPropertyChanged(nameof(IsMouseButtonLockEnabled)); | |
| OnPropertyChanged(nameof(IsMouseButtonLockEnabledGpoConfigured)); |
There was a problem hiding this comment.
Good catch, applied as suggested in 0220be4.
The DSC settings resource deserializes input with the module's JsonPropertyName keys, so the examples' C# member names (LmbLockEnabled, HoldDurationMs, ...) with bare values were silently ignored. Switch every example to the snake_case keys with BoolProperty/IntProperty value wrappers, and name the property headings by the same keys.
7d0a17b to
a5d2bfa
Compare
InitializeEnabledValues() recomputes both the effective enabled value and whether policy manages it, but the refresh only notified the former, so adding or retracting the policy while Settings was open left the enable card and GPOInfoControl in their previous state until the page was recreated.
WinInjector posts the release SendInput back to the hook thread, so a true return from InjectUp only meant the injection was queued. If the deferred SendInput was then rejected (for example a UIPI block while an elevated window owns the foreground), the engine had already cleared the lock and, for a release tap, armed the swallow of the paired physical up, so the OS could keep the button pressed while the module believed it was released. - Add IDeferredFailureSink to the core; the engine registers itself with the injector in its constructor and clears the registration in its destructor. WinInjector::PerformDeferred reports a failed InjectUpNow with the original button and dismiss intent. - On a deferred failure the engine stops swallowing a still-pending release-tap up (that up now performs the release) or, for any other release path, restores the logical lock so the next press, settings change or shutdown retries the cleanup. A synchronous rejection in ReleaseButton keeps the lock the same way. - Serialize every engine operation behind a mutex, and hold a module mutex across set_config's parse + EnforceEnabled and across each hook event's snapshot + engine call, so disabling a button cannot interleave with a stale event that latches it again. - Add regression tests for the sink lifecycle, both tap-release failure orderings, settings and cross-button release failures, and the synchronous rejection; document the repair path and the locking.
Windows drops an injected button-up without reporting it when input is blocked (BlockInput) or a higher-integrity window owns the foreground (UIPI): SendInput returns success and GetLastError says nothing, so the return-value check added earlier never fires for exactly the case the review raised. Measured directly: with input blocked, the runner's SendInput returned 1 and the button stayed down with no warning logged. - After a deferred SendInput reports success, WinInjector arms a 50 ms thread timer and then reads the button's physical state with GetAsyncKeyState (undoing a primary/secondary swap). Still down means the up never landed, so it reports through the same IDeferredFailureSink as a hard rejection. - The engine now tracks a press even for a disabled button and ignores a failure report while a physical press of that button is in progress: the press explains the held state and its own up completes the release, so a fast re-press inside the verification window cannot restore a stale lock. OnButtonDown no longer reads the settings snapshot (OnButtonUp already gates locking on Enabled). - Regression test for the mid-press report; dev doc updated. Verified against the running runner: with input blocked by an elevated process, the settings-driven release logged the dropped injection, restored the lock, and the next tap released the button; the ordinary lock/tap flow logs nothing.
|
Status after the 09-29 review, now at 7c748ea (mergeable against main, intake checks green): Muyuan Li (@MuyuanMS) all three items are addressed and answered in their threads. DSC examples use the serialized snake_case/ Boliang Zhang (@LegendaryBlair) your 08-11 review is still marked changes requested; all four items were addressed that day (details in my reply there). A re-review would clear it. Niels Laute (@niels9001) the only remaining blocker besides the two re-reviews is approval for the build workflows, which have not yet run on this branch. |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
Thanks for addressing my previous comments. I reviewed the latest changes and tested the feature locally; the PR looks good to me from a code and behavior perspective. The current CI run is still red for two reasons:
For the ARM64 failure, the test successfully persisted |
Verify persisted values immediately after restarting Settings and assert the rendered duration label instead of relying on an unsupported slider value read. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 90130a9f-d9f9-43b2-8beb-badf4f25bdf8
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
Fixed Mouse Button Lock test, but seems there's still unrelated CmdPal test failures - I'll try to look into it |
141e374
into
microsoft:main
Summary of the Pull Request
As discussed with Niels:
Adds Mouse Button Lock, a new Mouse Utilities utility: a ClickLock equivalent for the left, right, and middle mouse buttons. Hold a button past a configurable threshold and release it, and the OS keeps perceiving the button as held; tap the same button again to release. Windows ships ClickLock for the left button only, so this closes the gap for the secondary and middle buttons and optionally covers the left (primary) button too, putting all three in one place. Right-button lock is on by default; left is off by default (Windows already provides left ClickLock, and it is the primary interaction button).
Visual evidence
Right-button lock in File Explorer, which is the default-on button. The right button is held past the Hold duration (ms) threshold and then physically released; it stays latched, so the marquee selection keeps extending across the file list to 14 items with no button held. The clip ends with the lock released and Explorer showing the context menu for the completed selection.
PR Checklist
PowerToys.MouseButtonLock.dlladded toESRPSigning_core.json$(Platform)\Release\*.dllglob, like the other MouseUtils DLLsBuild;Testtarget, the fuzz project is picked up by the existing OneFuzz**/tests/*.FuzzTests/**glob, and the UI tests are part of the existingMouseUtils.UITests.NextprojectDetailed Description of the Pull Request / Additional comments
Native C++ in-process module, like the other passive Mouse Utilities (Find My Mouse, Mouse Highlighter, Mouse Pointer Crosshairs, CursorWrap). No separate window: activation is the physical hold, and settings live on the shared Mouse Utilities page.
MouseButtonLockCore.h): a Win32-free, unit-tested per-button ClickLock state machine. The clock is caller-supplied and synthetic button-up injection is behind an interface, so it is fully testable and fuzzable.dllmain.cpp): thin Win32 adapter. A dedicated thread installsWH_MOUSE_LL, suppresses the matching button-up by returning1, and injects the synthetic release viaSendInput, tagged withdwExtraInfoso the hook ignores its own events.MouseButtonLockPropertiesand the C++parse_settings.disable()and on graceful hook-thread shutdown, so toggling off or exiting PowerToys never leaves a button stuck.DATA_AND_PRIVACY.md.Dev docs:
doc/devdocs/modules/mouseutils/mousebuttonlock.md.Validation Steps Performed
EnforceEnabled/ReleaseAll/ResetTransient. All pass.MouseUtils.UITests.Next,MouseButtonLockSettingsTests.cs): the options expander follows the module toggle; the lock checkboxes, hold duration, and move-cancel distance persist tosettings.json(including bounds) and survive a restart; a held button locks past the configured duration, releases on a same-button tap, and does not lock under the threshold or when its lock is off. Builds clean; not yet run end to end (the suite drives the real desktop), so the pipeline run is the first full execution.AGENTS.mdfor user-input modules); it drives all three buttons with adversarial ticks/coordinates/settings.knownModules, and the Settings UI shows the Mouse Button Lock controls including the left-button checkbox.