Skip to content
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

[Web] Fix an assertion error due to synthesized keyboard events #49087

Conversation

bleroux
Copy link
Contributor

@bleroux bleroux commented Dec 15, 2023

Description

On Web, browsers can emit key events with a logical key sets to Process during composition.
It is usually not a problem, but in some edge cases (for instance when the browser window lost focus and some keys events were missed), the Flutter web engine might synthesize an up event with a logical key value different that the one used for the down event and this will lead to an assertion message on the framework side.

Related Issue

Fixes flutter/flutter#126247.

Tests

Adds 1 test.

@github-actions github-actions bot added the platform-web Code specifically for the web engine label Dec 15, 2023
@@ -472,7 +472,7 @@ class KeyboardConverter {
timeStamp: timeStamp,
type: ui.KeyEventType.up,
physical: physicalKey,
logical: logicalKey(),
logical: _pressingRecords[physicalKey]!,
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for your contribution @bleroux!

@dkwingsmt is this an acceptable approach to handling the hardware_keyboard physical/logical key mismatch error on web? From my understanding, the code here is written to map logical keys on web similar to how other platforms do it - however, I think only the browser emits extra "Process" key events for special characters like IME composition, emojis, etc

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think so. Actually I wonder why I didn't do it in the first place. Since the up event should always have the same logical key as the down event, this looks very idiomatic.

@dkwingsmt
Copy link
Contributor

Thank you for the fix! This is definitely an edge case I didn't think about before.

Copy link
Contributor

@htoor3 htoor3 left a comment

Choose a reason for hiding this comment

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

thank you again @bleroux

@bleroux bleroux added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 23, 2024
@auto-submit auto-submit bot merged commit 11a16d8 into flutter:main Jan 23, 2024
27 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 23, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jan 23, 2024
…142071)

flutter/engine@57d6b51...11a16d8

2024-01-23 leroux_bruno@yahoo.fr [Web] Fix an assertion error due to synthesized keyboard events (flutter/engine#49087)
2024-01-23 737941+loic-sharma@users.noreply.github.com [Windows] Introduce egl::Context (flutter/engine#49954)
2024-01-23 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from uQK4l0QeH_PlZO6cF... to kYC2-fFgjbb36mukB... (flutter/engine#49976)
2024-01-23 skia-flutter-autoroll@skia.org Roll Skia from 58973cca0edd to 59a70a1efc39 (17 revisions) (flutter/engine#49975)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from uQK4l0QeH_Pl to kYC2-fFgjbb3

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC jacksongardner@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@bleroux bleroux deleted the web_synthesize_keyboard_event_using_saved_logical_key branch January 24, 2024 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TextField] Flutter web - Textfield hardware_keyboard error
3 participants