Skip to content
This repository has been archived by the owner on Feb 25, 2025. It is now read-only.

[Window, Keyboard] Fix repeat events of modifier keys #35046

Merged
merged 5 commits into from
Aug 1, 2022

Conversation

dkwingsmt
Copy link
Contributor

@dkwingsmt dkwingsmt commented Aug 1, 2022

Fixes flutter/flutter#108646 .

The synchronization logic revised in #33746 always synchronizes the pre-event state of a modifier key to be "released". This is problematic since it disallows repeat events. With this PR, pressing synchronization no longer handles down events of the event key except in one scenario where the event key's toggling state has just been synchronized. However, this exception is not trivial and requires communication between the two synchronization function. The approach adopted by this PR is a little ugly, but it works. Maybe one day we will hate it and find a better way.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
    writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@dkwingsmt dkwingsmt marked this pull request as ready for review August 1, 2022 12:36
@dkwingsmt dkwingsmt requested a review from gspencergoog August 1, 2022 12:38
Copy link
Contributor

@gspencergoog gspencergoog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32384589-a60f0e74-c078-11e7-9bc1-e5b5287aea9d

@@ -424,8 +429,18 @@ void KeyboardKeyEmbedderHandler::SynchronizeCritialPressedStates(
if (is_event_down) {
// For down events, this key is the event key if they have the same
// virtual key, because virtual key represents "functionality."
//
// In that case, normally Flutter should synthesize nothing since the
// resulting event can adapt to the current state by flexing between a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flexing? Do you just mean that it swaps its state?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... I phrased it into "the resulting event can adapt to the current state by dispatching either a down or a repeat event". Hopefully this is clearer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that works.

@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 1, 2022
@auto-submit auto-submit bot merged commit b257966 into flutter:main Aug 1, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 1, 2022
@dkwingsmt dkwingsmt deleted the fix-win-key-modifier-repeat branch August 2, 2022 00:22
betrevisan pushed a commit to betrevisan/engine that referenced this pull request Aug 5, 2022
emilyabest pushed a commit to emilyabest/engine that referenced this pull request Aug 12, 2022
@dkwingsmt dkwingsmt restored the fix-win-key-modifier-repeat branch August 30, 2022 00:43
Doflatango added a commit to Doflatango/flutter-webview-windows that referenced this pull request Oct 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pressing shiftkey to insert ( or ) in textfield crashes flutter application
2 participants