-
Notifications
You must be signed in to change notification settings - Fork 6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Android] Fix a crash related to zeroed scanCode #35924
[Android] Fix a crash related to zeroed scanCode #35924
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nits about comments 👍
Thanks for fixing this!
I see no problem with passing through eventPhysicalKey but anyone with more experience in the Android engine feel free to chime in.
@@ -122,7 +122,7 @@ void updatePressingState(@NonNull Long physicalKey, @Nullable Long logicalKey) { | |||
// the current event in consideration. | |||
// | |||
// Events that should be synthesized before the main event are synthesized | |||
// immediately, while events that should be syntehsized after the main event are appended to | |||
// immediately, while events that should be synthesized after the main event are appended to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching that! 😊
|
||
// Test: DOWN event when the current state is 0 and scanCode is 0. | ||
final KeyEvent keyEvent = new FakeKeyEvent(ACTION_DOWN, 0, KEYCODE_SHIFT_LEFT, 0, '\0', 0); | ||
// Compute physicalKey in the same way than KeyboardManager.getPhysicalKey |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"same way than" => "same was as"
And also period at the end.
@Test | ||
public void synchronizeModifiersForZeroedScanCode() { | ||
// Test if ShiftLeft can be correctly synchronized during down events of | ||
// ShiftLeft that has 0 for its metaState and 0 for its scanCode. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly:
"has" => "have"
"its" => "their"
d98e78b
to
24337ea
Compare
nowStates[keyIdx] = pressingRecords.containsKey(goal.keys[keyIdx].physicalKey); | ||
if (goal.keys[keyIdx].logicalKey == eventLogicalKey) { | ||
nowStates[keyIdx] = pressingRecords.containsKey(key.physicalKey); | ||
if (key.logicalKey == eventLogicalKey) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to confirm, are these two lines of change make any difference, or purely cosmetic? (I'm ok with either)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Purely cosmetic. The 'key' variable was already defined (before this PR) and It could be used on this two lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…ting text in `TextField` (#36288) * [Android] Synchronize incorrect metaState using post-synchronization (#34827) (cherry picked from commit a56d98e) * [Android] Fix a crash related to zeroed scanCode (#35924) (cherry picked from commit fe5b84a) Co-authored-by: Tong Mu <[email protected]> Co-authored-by: Bruno Leroux <[email protected]>
Description
#33686, introduced the Android implementation of the backend of
HardwareKeyboard
.#34827 fixed a crash related to incorrect metaState.
This PR fixes another crash related to incorrect metaState and a zeroed scanCode.
Implementation
#34827 synthesizes a key up event when there is a down event with incorrect metaState.
If the scanCode is zeroed the computed physical key codes are not the same (for the down event and the up event) and leads to an incorrect state.
This PR updates how the key up event is synthesized by using the same physical key code (the one in the down event).
Related Issue
Fixes flutter/flutter#110640
Tests
Adds 1 test.