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

[Web, Keyboard] Duplicate down events are no longer ignored, but kept and preceded by up events #30488

Merged
merged 1 commit into from
Jan 4, 2022
Merged

[Web, Keyboard] Duplicate down events are no longer ignored, but kept and preceded by up events #30488

merged 1 commit into from
Jan 4, 2022

Conversation

@dkwingsmt
Copy link
Contributor

@dkwingsmt dkwingsmt commented Dec 24, 2021 •
edited
Loading

Before this PR, duplicate down events are regularized by ignoring the latter events.

For example, native KeyA down, KeyA down will result in Flutter KeyA down, (empty).

This PR changes so that the latter events are kept, and, for regularization purposes, preceded by synthesized up events. The example above will now result in Flutter KeyA down, (KeyA up) KeyA down.

This fixes flutter/flutter#95474 (more specifically, flutter/flutter#95474 (comment)). The previous strategy failed to consider the case where duplicate down events could be caused by rapid sequence of system shortcuts, such as holding Ctrl, press V then V (Web does not send the keyup events of system shortcuts; we've designed a "key guard" system to alleviate such problem). In such cases, the latter events must be dispatched as down events for the framework to correctly recognize and choose not to handle.

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.

@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label Dec 24, 2021
@dkwingsmt dkwingsmt mentioned this pull request Dec 24, 2021
Closed
gspencergoog
gspencergoog approved these changes Jan 4, 2022
View reviewed changes
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

@dkwingsmt dkwingsmt merged commit e749ba3 into flutter:main Jan 4, 2022
@dkwingsmt dkwingsmt deleted the web-key-duplicate-down-synth-up branch January 5, 2022 01:18
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 5, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 5, 2022
@engine-flutter-autoroll
e749ba3 Impl and test (flutter/engine#30488)
59441fb
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 5, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 5, 2022
@engine-flutter-autoroll
e749ba3 Impl and test (flutter/engine#30488)
e3de834
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 6, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 6, 2022
@engine-flutter-autoroll
e749ba3 Impl and test (flutter/engine#30488)
bce0c09
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 6, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 6, 2022
@engine-flutter-autoroll
e749ba3 Impl and test (flutter/engine#30488)
a7c994c
zanderso pushed a commit to flutter/flutter that referenced this pull request Jan 6, 2022
@engine-flutter-autoroll
Roll Engine from a6ce607 to 8498779 (27 revisions) (#96239)
12271f8 
* 9481a79 Roll Skia from 45f64bd52835 to 6bebf036a502 (2 revisions) (flutter/engine#30659)

* fb4a86c Roll Fuchsia Mac SDK from RyUwCnr_M... to vpa6vKu7U... (flutter/engine#30661)

* 154bd96 Roll Skia from 6bebf036a502 to 5726d457cf15 (1 revision) (flutter/engine#30665)

* ba23c6c Roll Skia from 5726d457cf15 to 61d0fbbca795 (5 revisions) (flutter/engine#30673)

* e749ba3 Impl and test (flutter/engine#30488)

* a78103c Removed "UiThread" annotation from MethodChannel#Result. (flutter/engine#30671)

* f8a398f Fix crash in BackdropFilterLayer::Diff (flutter/engine#30460)

* a6a856f Only provide frame damage to rasterizer if partial repaint is enabled (flutter/engine#30461)

* 3a667ab Roll Fuchsia Mac SDK from vpa6vKu7U... to Al-HXHXyQ... (flutter/engine#30677)

* 830abeb [web] roll CanvasKit 0.32.0; fix frame order in animated images (flutter/engine#30680)

* c726121 Add a new display_list_benchmarks test suite (flutter/engine#30678)

* f181c4d [web] flip browser image codec flag to opt-out (flutter/engine#30681)

* 436a346 Remove the ios_tools Chromium-style dependency (flutter/engine#30538)

* 3f63998 [web] bring libraries.yaml/libraries.json up to date (flutter/engine#30467)

* ab1e8f5 Roll Skia from 61d0fbbca795 to 4981c921c6d7 (1 revision) (flutter/engine#30684)

* 8e4124b Roll Skia from 4981c921c6d7 to d7771857e9e2 (2 revisions) (flutter/engine#30685)

* acb60b4 Roll Fuchsia Mac SDK from Al-HXHXyQ... to G04Sc3__F... (flutter/engine#30687)

* 0176295 [Win32, Keyboard] TextInputPlugin is no longer a KeyboardHandlerBase (flutter/engine#30456)

* bcb4b35 Roll Skia from d7771857e9e2 to ec2e8f11b97a (1 revision) (flutter/engine#30688)

* e09f8d1 Roll Dart SDK from 1697706df708 to ab5047720a9e (5 revisions) (flutter/engine#30690)

* d60c816 Roll Dart SDK from 1697706df708 to f59531cc2973 (10 revisions) (flutter/engine#30691)

* 417042c GN targets for generating release artifacts (flutter/engine#30679)

* 7f31015 Roll Skia from ec2e8f11b97a to 84d6cf9b5b76 (7 revisions) (flutter/engine#30693)

* fed9e0b Roll Skia from 84d6cf9b5b76 to 88c5af7ecd72 (3 revisions) (flutter/engine#30695)

* 36eafae [fuchsia][shader warmup] fix for fxbug.dev/90387 (flutter/engine#30482)

* 0c036a7 Revert "Only provide frame damage to rasterizer if partial repaint is enabled (#30461)" (flutter/engine#30696)

* 8498779 [fuchsia] Fix failing SDK roll. (flutter/engine#30675)
itsjustkevin pushed a commit to itsjustkevin/engine that referenced this pull request Jan 19, 2022
@dkwingsmt @itsjustkevin
Impl and test (flutter#30488)
8410475
itsjustkevin added a commit that referenced this pull request Jan 19, 2022
@itsjustkevin @dkwingsmt
@cbracken @jason-simmons
[flutter_releases] Flutter beta 2.10.0-0.2.pre Engine Cherrypicks (#3…
74b74b1 
…0918)

* Impl and test (#30488)

* Fix missing shcore.dll error on Windows 7 (#30699)

* Remove glitch when displaying platform views (#30724)

* Win32: Fix Korean text input (#30805)

Fixes an issue with Korean IMEs wherein a text input state update may be
sent to the framework that misleads the framework into assuming that IME
composing has ended.

When inputting Korean text, characters are built up keystroke by
keystroke until the point that either:
* the user presses space/enter to terminate composing and commit the
  character, or;
* the user presses a key such that the character currently being
  composed cannot be modified further, and the IME determines that the
  user has begun composing the next character.

The following is an example sequence of events for the latter case:

1. User presses ㅂ. GCS_COMPSTR event received with ㅂ. Embedder sends
   state update to framework.
2. User presses ㅏ. GCS_COMPSTR event received with 바. Embedder sends
   state update to framework.
3. User presses ㄴ. GCS_COMPSTR event received with 반. Embedder sends
   state update to framework.
4. User presses ㅏ. At this point, the current character being composed
   (반) cannot be modified in a meaningful way, and the IME determines
   that the user is typing 바 followed by 나. GCS_RESULTSTR event
   received with 바, immediately followed by GCS_COMPSTR event with 나.

In step 4, we previously sent two events to the framework, one
immediately after the other:
* GCS_RESULTSTR triggers the text input model to commit the current
  composing region to the string under edit. This causes the composing
  region to collapse to an empty range.
* GCS_COMPSTR triggers the text input model to insert the new composing
  character and set the composing region to that character.

Conceptually, this is an atomic operation. The fourth keystroke causes
the 반 character to be broken into two (바 and ㄴ) and the latter to be
modified to 나. From the user's point of view, as well as from the IME's
point of view, the user has NOT stopped composing, and the composing
region has simply moved on to the next character.

Flutter has no concept of whether the user is composing or not other
that whether a non-empty composing region exists. As such, sending a
state update after the GCS_RESULTSTR event misleads the framework into
believing that composing has ended. This triggers a serious bug:

Text fields with input formatters applied do not perform input
formatting updates while composing is active; instead they wait until
composing has ended to apply any formatting. The previous behaviour
would thus trigger input formatters to be applied each time the user
input caused a new character to be input. This has the add-on negative
effect that once formatting has been applied, it sends an update back to
the embedder so that the native OS text input state can be updated.
However, since the GCS_RESULTSTR event is _immediately_ followed by a
GCS_COMPSTR, the state has changed in the meantime, and the embedder is
left processing an update (the intermediate state sent after the
GCS_RESULTSTR) which is now out of date (i.e. missing the new state from
the GCS_COMPSTR event).

Since GCS_RESULTR events are always immediately followed by a subsequent
GCS_COMPSTR (in the case where composing continues) or a
WM_IME_ENDCOMPOSITION (in the case where composing is finished), and
because the event handlers for both of those send updated state to the
framework, this change eliminates sending the (intermediate) state in
response to GCS_COMPSTR events.

Issue: flutter/flutter#96209 (full fix)
Issue: flutter/flutter#88645 (partial fix)

* Ensure that PlatformViewIOS does not call into Shell semantics APIs during destruction (#30835)

* Ensure that PlatformViewIOS does not call into Shell semantics APIs during destruction (#30835)

Co-authored-by: Tong Mu <[email protected]>
Co-authored-by: Chris Bracken <[email protected]>
Co-authored-by: Emmanuel Garcia <[email protected]>
Co-authored-by: Jason Simmons <[email protected]>
@dkwingsmt dkwingsmt mentioned this pull request Feb 9, 2022
Closed
@yjbanov yjbanov mentioned this pull request Feb 10, 2022
Closed
@dkwingsmt dkwingsmt mentioned this pull request Mar 3, 2022
Merged
8 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@gspencergoog gspencergoog gspencergoog approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

affects: text input platform-web Code specifically for the web engine

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Flutter Web misses rapid shortcut presses

3 participants

@dkwingsmt @gspencergoog @HansMuller