Use `start` instead of `extent` for Windows IME cursor position (#45667)
When composing with the IME in a text edit, we should add the `start` of
the composition range to the in-composition `cursor_pos` rather than its
`extent`. When using `extent`, the cursor position would always be
outside of the composition range, resulting in the linked bug. Add a
test to check cursor position.
https://github.com/flutter/flutter/issues/123749
## Pre-launch Checklist
- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or the PR is [test-exempt]. See [testing the engine] for
instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [ ] I signed the [CLA].
- [x] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel
on [Discord].
<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
diff --git a/shell/platform/windows/text_input_plugin.cc b/shell/platform/windows/text_input_plugin.cc
index 340092e..08c0007 100644
--- a/shell/platform/windows/text_input_plugin.cc
+++ b/shell/platform/windows/text_input_plugin.cc
@@ -196,7 +196,7 @@
std::string text_before_change = active_model_->GetText();
TextRange composing_before_change = active_model_->composing_range();
active_model_->AddText(text);
- cursor_pos += active_model_->composing_range().extent();
+ cursor_pos += active_model_->composing_range().start();
active_model_->UpdateComposingText(text);
active_model_->SetSelection(TextRange(cursor_pos, cursor_pos));
std::string text_after_change = active_model_->GetText();
diff --git a/shell/platform/windows/text_input_plugin_unittest.cc b/shell/platform/windows/text_input_plugin_unittest.cc
index 7d47050..a99e014 100644
--- a/shell/platform/windows/text_input_plugin_unittest.cc
+++ b/shell/platform/windows/text_input_plugin_unittest.cc
@@ -36,6 +36,8 @@
static constexpr char kSelectionIsDirectionalKey[] = "selectionIsDirectional";
static constexpr char kComposingBaseKey[] = "composingBase";
static constexpr char kComposingExtentKey[] = "composingExtent";
+static constexpr char kUpdateEditingStateMethod[] =
+ "TextInputClient.updateEditingState";
static std::unique_ptr<std::vector<uint8_t>> CreateResponse(bool handled) {
auto response_doc =
@@ -243,7 +245,7 @@
// Editing state should have been updated.
auto encoded_arguments = EncodedEditingState("\n", TextRange(1));
auto update_state_message = codec.EncodeMethodCall(
- {"TextInputClient.updateEditingState", std::move(encoded_arguments)});
+ {kUpdateEditingStateMethod, std::move(encoded_arguments)});
EXPECT_TRUE(std::equal(update_state_message->begin(),
update_state_message->end(),
@@ -366,6 +368,65 @@
// Passes if it did not crash
}
+// Regression test for https://github.com/flutter/flutter/issues/123749
+TEST(TextInputPluginTest, CompositionCursorPos) {
+ int selection_base = -1;
+ TestBinaryMessenger messenger([&](const std::string& channel,
+ const uint8_t* message, size_t size,
+ BinaryReply reply) {
+ auto method = JsonMethodCodec::GetInstance().DecodeMethodCall(
+ std::vector<uint8_t>(message, message + size));
+ if (method->method_name() == kUpdateEditingStateMethod) {
+ const auto& args = *method->arguments();
+ const auto& editing_state = args[1];
+ auto base = editing_state.FindMember(kSelectionBaseKey);
+ auto extent = editing_state.FindMember(kSelectionExtentKey);
+ ASSERT_NE(base, editing_state.MemberEnd());
+ ASSERT_TRUE(base->value.IsInt());
+ ASSERT_NE(extent, editing_state.MemberEnd());
+ ASSERT_TRUE(extent->value.IsInt());
+ selection_base = base->value.GetInt();
+ EXPECT_EQ(extent->value.GetInt(), selection_base);
+ }
+ });
+ MockTextInputPluginDelegate delegate;
+
+ TextInputPlugin plugin(&messenger, &delegate);
+
+ auto args = std::make_unique<rapidjson::Document>(rapidjson::kArrayType);
+ auto& allocator = args->GetAllocator();
+ args->PushBack(123, allocator); // client_id
+ rapidjson::Value client_config(rapidjson::kObjectType);
+ args->PushBack(client_config, allocator);
+ auto encoded = JsonMethodCodec::GetInstance().EncodeMethodCall(
+ MethodCall<rapidjson::Document>(kSetClientMethod, std::move(args)));
+ EXPECT_TRUE(messenger.SimulateEngineMessage(
+ kChannelName, encoded->data(), encoded->size(),
+ [](const uint8_t* reply, size_t reply_size) {}));
+
+ plugin.ComposeBeginHook();
+ EXPECT_EQ(selection_base, 0);
+ plugin.ComposeChangeHook(u"abc", 3);
+ EXPECT_EQ(selection_base, 3);
+
+ plugin.ComposeCommitHook();
+ plugin.ComposeEndHook();
+ EXPECT_EQ(selection_base, 3);
+
+ plugin.ComposeBeginHook();
+ plugin.ComposeChangeHook(u"1", 1);
+ EXPECT_EQ(selection_base, 4);
+
+ plugin.ComposeChangeHook(u"12", 2);
+ EXPECT_EQ(selection_base, 5);
+
+ plugin.ComposeChangeHook(u"12", 1);
+ EXPECT_EQ(selection_base, 4);
+
+ plugin.ComposeChangeHook(u"12", 2);
+ EXPECT_EQ(selection_base, 5);
+}
+
TEST(TextInputPluginTest, TransformCursorRect) {
// A position of `EditableText`.
double view_x = 100;
