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

Win32: Fix Korean text input #30805

Merged
merged 1 commit into from
Jan 12, 2022
Merged

Conversation

cbracken
Copy link
Member

@cbracken cbracken commented Jan 12, 2022

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.

This bug was revealed by flutter/flutter#90211
which applies an input formatter to single-line text fields in order to
suppress newlines.

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

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.

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
Issue: flutter/flutter#88645
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

The comment seems very clear to me, and I'm no IME expert.

@cbracken cbracken merged commit f9eabec into flutter:main Jan 12, 2022
@cbracken cbracken deleted the korean-input-fix branch January 12, 2022 18:01
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 12, 2022
JsouLiang pushed a commit to JsouLiang/engine that referenced this pull request Jan 14, 2022
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)
itsjustkevin pushed a commit to itsjustkevin/engine that referenced this pull request Jan 19, 2022
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)
itsjustkevin added a commit that referenced this pull request Jan 19, 2022
…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 <dkwingsmt@users.noreply.github.com>
Co-authored-by: Chris Bracken <chris@bracken.jp>
Co-authored-by: Emmanuel Garcia <egarciad@google.com>
Co-authored-by: Jason Simmons <jason-simmons@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants