From 64b7be76054e86a1e5843e32e7ea73a79081d94a Mon Sep 17 00:00:00 2001 From: Chris Bracken Date: Tue, 11 Jan 2022 16:27:23 -0800 Subject: [PATCH] Win32: Fix Korean text input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/flutter/flutter/issues/96209 Issue: https://github.com/flutter/flutter/issues/88645 --- shell/platform/windows/text_input_plugin.cc | 24 +++++++- .../windows/text_input_plugin_unittest.cc | 56 +++++++++++++++++++ 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/shell/platform/windows/text_input_plugin.cc b/shell/platform/windows/text_input_plugin.cc index 15e497e512de0..543b0c002337d 100644 --- a/shell/platform/windows/text_input_plugin.cc +++ b/shell/platform/windows/text_input_plugin.cc @@ -111,7 +111,29 @@ void TextInputPlugin::ComposeCommitHook() { return; } active_model_->CommitComposing(); - SendStateUpdate(*active_model_); + + // We do not trigger SendStateUpdate here. + // + // Until a WM_IME_ENDCOMPOSING event, the user is still composing from the OS + // point of view. Commit events are always immediately followed by another + // composing event or an end composing event. However, in the brief window + // between the commit event and the following event, the composing region is + // collapsed. Notifying the framework of this intermediate state will trigger + // any framework code designed to execute at the end of composing, such as + // input formatters, which may try to update the text and send a message back + // to the engine with changes. + // + // This is a particular problem with Korean IMEs, which build up one + // character at a time in their composing region until a keypress that makes + // no sense for the in-progress character. At that point, the result + // character is committed and a compose event is immedidately received with + // the new composing region. + // + // In the case where this event is immediately followed by a composing event, + // the state will be sent in ComposeChangeHook. + // + // In the case where this event is immediately followed by an end composing + // event, the state will be sent in ComposeEndHook. } void TextInputPlugin::ComposeEndHook() { diff --git a/shell/platform/windows/text_input_plugin_unittest.cc b/shell/platform/windows/text_input_plugin_unittest.cc index 2da9c77aae3d5..30af07d96e42d 100644 --- a/shell/platform/windows/text_input_plugin_unittest.cc +++ b/shell/platform/windows/text_input_plugin_unittest.cc @@ -81,5 +81,61 @@ TEST(TextInputPluginTest, ClearClientResetsComposing) { EXPECT_TRUE(delegate.ime_was_reset()); } +// Verify that the embedder sends state update messages to the framework during +// IME composing. +TEST(TextInputPluginTest, VerifyComposingSendStateUpdate) { + bool sent_message = false; + TestBinaryMessenger messenger( + [&sent_message](const std::string& channel, const uint8_t* message, + size_t message_size, + BinaryReply reply) { sent_message = true; }); + BinaryReply reply_handler = [](const uint8_t* reply, size_t reply_size) {}; + + EmptyTextInputPluginDelegate delegate; + TextInputPlugin handler(&messenger, &delegate); + + auto& codec = JsonMethodCodec::GetInstance(); + + // Call TextInput.setClient to initialize the TextInputModel. + auto arguments = std::make_unique(rapidjson::kArrayType); + auto& allocator = arguments->GetAllocator(); + arguments->PushBack(42, allocator); + rapidjson::Value config(rapidjson::kObjectType); + config.AddMember("inputAction", "done", allocator); + config.AddMember("inputType", "text", allocator); + arguments->PushBack(config, allocator); + auto message = + codec.EncodeMethodCall({"TextInput.setClient", std::move(arguments)}); + messenger.SimulateEngineMessage("flutter/textinput", message->data(), + message->size(), reply_handler); + + // ComposeBeginHook should send state update. + sent_message = false; + handler.ComposeBeginHook(); + EXPECT_TRUE(sent_message); + + // ComposeChangeHook should send state update. + sent_message = false; + handler.ComposeChangeHook(u"4", 1); + EXPECT_TRUE(sent_message); + + // ComposeCommitHook should NOT send state update. + // + // Commit messages are always immediately followed by a change message or an + // end message, both of which will send an update. Sending intermediate state + // with a collapsed composing region will trigger the framework to assume + // composing has ended, which is not the case until a WM_IME_ENDCOMPOSING + // event is received in the main event loop, which will trigger a call to + // ComposeEndHook. + sent_message = false; + handler.ComposeCommitHook(); + EXPECT_FALSE(sent_message); + + // ComposeEndHook should send state update. + sent_message = false; + handler.ComposeEndHook(); + EXPECT_TRUE(sent_message); +} + } // namespace testing } // namespace flutter