Fix: Mouse bug -> switch getKeyState with getAsyncKeyState #20052
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses mis-targeted mouse clicks when the full-screen magnifier is running in Center mode by switching from message-queue-based key state to GetAsyncKeyState, so the magnifier can reliably detect mouse button presses immediately and avoid repositioning the cursor mid-click.
Changes:
- Replace mouse-button detection with
winUser.getAsyncKeyStatein magnifier focus tracking (FocusManager.getCurrentFocusCoordinates). - Replace
winUser.getKeyStatewithwinUser.getAsyncKeyStatein_keepMouseCenteredto avoid interfering with active clicks. - Update unit tests to mock the new API usage.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
source/_magnifier/utils/focusManager.py |
Use getAsyncKeyState for left-button detection; broaden exception handling for review point retrieval. |
source/_magnifier/fullscreenMagnifier.py |
Use getAsyncKeyState for left/right/middle button checks before cursor repositioning. |
tests/unit/test_magnifier/test_focusManager.py |
Update tests to reflect the new async key state path. |
tests/unit/test_magnifier/test_fullscreenMagnifier.py |
Update mocks/patching to target getAsyncKeyState. |
Member
|
Can you please make sure unit tests pass? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to issue number:
fixes #19691
Summary of the issue:
When using the fullscreen magnifier in Center mode, clicking on controls in certain apps (Discord, Microsoft Teams, HyperX NGENUITY Beta) had no effect. The magnifier transform could shift between the physical button press and Windows applying it to the click coordinates, causing the click to land at the wrong logical position.
Description of user facing changes:
Mouse clicks on controls in apps such as Discord, Microsoft Teams, and HyperX NGENUITY Beta should work correctly when the fullscreen magnifier is running in Center mode.
Description of developer facing changes:
Replaced winUser.getKeyState / mouseHandler.isLeftMouseButtonLocked with winUser.getAsyncKeyState in FocusManager.getCurrentFocusCoordinates and FullScreenMagnifier._keepMouseCentered. Updated tests accordingly.
Description of development approach:
tKeyState is queue-based and can return a stale button state in the wx main thread when the message queue hasn't yet processed the click event. Switching to GetAsyncKeyState ensures the magnifier detects a button press immediately, preventing the Center mode transform from shifting between the physical press and Windows applying it to the click coordinates.
Testing strategy:
Unit
Known issues with pull request:
Code Review Checklist: