[CP-beta][Android] Gboard Text Shift Stuck Fix#185749
Conversation
Changing Android embedder keyboard logic to check if the key event originates from a software keyboard using Android's native KeyCharacterMap.VIRTUAL_KEYBOARD device identifier (deviceId == -1): boolean isVirtualKeyboard = event.getDeviceId() == android.view.KeyCharacterMap.VIRTUAL_KEYBOARD; When META_SHIFT_ON is active, the KeyEmbedderResponder will now trust the current state for virtual keyboards and skip synthesizing missing physical modifier keys. Why this works: 1. Fixes Gboard Shift: When Gboard sends ShiftLeft UP but keeps META_SHIFT_ON active (to signify Shift lock), Flutter will no longer synthesize a phantom ShiftRight DOWN. The framework will correctly register that NO physical shift keys are held down, so tapping the screen will no longer trigger a shift-click text selection. 2. Fixes Backspace/Enter: If you type another virtual key (like Backspace) while META_SHIFT_ON is active, Flutter will no longer synthesize a phantom ShiftLeft DOWN. 3. Preserves Hardware Keyboard Fallbacks: ChromeOS and external physical keyboards (which do not use VIRTUAL_KEYBOARD as their device ID) will continue to have missing modifier keys correctly synthesized by Flutter if their hardware drops the DOWN events. Fixes: flutter#184744 ## Pre-launch Checklist - [X] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [X] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [X] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [X] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [X] I signed the [CLA]. - [X] I listed at least one issue that this PR fixes in the description above. - [X] I updated/added relevant documentation (doc comments with `///`). - [X] I added new tests to check the change I am making, or this PR is [test-exempt]. - [X] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [X] All existing and new tests are passing.
|
@mboetger please fill out the PR description above, afterwards the release team will review this request. |
|
This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter. Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed. |
There was a problem hiding this comment.
Code Review
This pull request updates the Android key embedder to handle virtual keyboards by trusting their meta state and skipping the synthesis of physical modifier keys, which prevents modifiers from becoming stuck. It also adds a corresponding test case and a new constructor to FakeKeyEvent to support testing with specific device IDs. Review feedback suggests improving the scoping of the virtual keyboard check for better readability and adding required Javadoc documentation to the new public constructor in FakeKeyEvent.
| // Virtual keyboards (like Gboard) often keep meta bits active without sending | ||
| // corresponding physical key events. For these, we trust the meta state | ||
| // and skip synthesizing physical modifier keys to avoid "stuck" modifiers. | ||
| boolean isVirtualKeyboard = | ||
| event.getDeviceId() == android.view.KeyCharacterMap.VIRTUAL_KEYBOARD; | ||
|
|
||
| // Fill the rest of the pre-event states to match the true state. | ||
| if (truePressed) { |
There was a problem hiding this comment.
The isVirtualKeyboard check and its associated comment are only relevant when truePressed is true. Moving this logic inside the if (truePressed) block improves code scoping and readability by keeping the variable closer to its usage.
| // Virtual keyboards (like Gboard) often keep meta bits active without sending | |
| // corresponding physical key events. For these, we trust the meta state | |
| // and skip synthesizing physical modifier keys to avoid "stuck" modifiers. | |
| boolean isVirtualKeyboard = | |
| event.getDeviceId() == android.view.KeyCharacterMap.VIRTUAL_KEYBOARD; | |
| // Fill the rest of the pre-event states to match the true state. | |
| if (truePressed) { | |
| // Fill the rest of the pre-event states to match the true state. | |
| if (truePressed) { | |
| // Virtual keyboards (like Gboard) often keep meta bits active without sending | |
| // corresponding physical key events. For these, we trust the meta state | |
| // and skip synthesizing physical modifier keys to avoid "stuck" modifiers. | |
| boolean isVirtualKeyboard = | |
| event.getDeviceId() == android.view.KeyCharacterMap.VIRTUAL_KEYBOARD; | |
References
- Optimize for readability: Code is read more often than it is written. (link)
| public FakeKeyEvent( | ||
| int action, | ||
| int scancode, | ||
| int code, | ||
| int repeat, | ||
| char character, | ||
| int metaState, | ||
| int deviceId, | ||
| int source) { |
There was a problem hiding this comment.
All public members should have documentation. Please add a Javadoc comment for this new constructor.
/**
* Creates a new {@link FakeKeyEvent} with the specified parameters.
*/
public FakeKeyEvent(
int action,
int scancode,
int code,
int repeat,
char character,
int metaState,
int deviceId,
int source) {References
- All public members should have documentation. (link)
camsim99
left a comment
There was a problem hiding this comment.
LGTM I think the impact of the issue makes the risk worth it here
b710888
into
flutter:flutter-3.44-candidate.0
This pull request is created by automatic cherry pick workflow
Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request.
Issue Link:
What is the link to the issue this cherry-pick is addressing?
#184744
Impact Description:
What is the impact:
Does it impact development (ex. flutter doctor crashes when Android Studio is installed),
or the shipping of production apps (the app crashes on launch):
This information is for domain experts and release engineers to understand the consequences of saying yes or no to the cherry pick.
Changelog Description:
Explain this cherry pick:
Workaround:
Is there a workaround for this issue?
Risk:
What is the risk level of this cherry-pick?
Test Coverage:
Are you confident that your fix is well-tested by automated tests?
Validation Steps:
What are the steps to validate that this fix works?
Expected: Again the cursor moves where you tap.
Previously: A text selection opens up. One end of the selection moves where you tap; the other end remains where the cursor was. The text input remains in this state no matter how many times you tap, double-tap, etc., trying to get back to normal.