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

Commit b9ce92c

Browse files
yaakovschectmanharryterkelsen
authored andcommitted
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. flutter/flutter#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
1 parent 1db3c03 commit b9ce92c

File tree

2 files changed

+63
-2
lines changed

2 files changed

+63
-2
lines changed

shell/platform/windows/text_input_plugin.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ void TextInputPlugin::ComposeChangeHook(const std::u16string& text,
196196
std::string text_before_change = active_model_->GetText();
197197
TextRange composing_before_change = active_model_->composing_range();
198198
active_model_->AddText(text);
199-
cursor_pos += active_model_->composing_range().extent();
199+
cursor_pos += active_model_->composing_range().start();
200200
active_model_->UpdateComposingText(text);
201201
active_model_->SetSelection(TextRange(cursor_pos, cursor_pos));
202202
std::string text_after_change = active_model_->GetText();

shell/platform/windows/text_input_plugin_unittest.cc

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ static constexpr char kSelectionAffinityKey[] = "selectionAffinity";
3636
static constexpr char kSelectionIsDirectionalKey[] = "selectionIsDirectional";
3737
static constexpr char kComposingBaseKey[] = "composingBase";
3838
static constexpr char kComposingExtentKey[] = "composingExtent";
39+
static constexpr char kUpdateEditingStateMethod[] =
40+
"TextInputClient.updateEditingState";
3941

4042
static std::unique_ptr<std::vector<uint8_t>> CreateResponse(bool handled) {
4143
auto response_doc =
@@ -243,7 +245,7 @@ TEST(TextInputPluginTest, VerifyInputActionNewlineInsertNewLine) {
243245
// Editing state should have been updated.
244246
auto encoded_arguments = EncodedEditingState("\n", TextRange(1));
245247
auto update_state_message = codec.EncodeMethodCall(
246-
{"TextInputClient.updateEditingState", std::move(encoded_arguments)});
248+
{kUpdateEditingStateMethod, std::move(encoded_arguments)});
247249

248250
EXPECT_TRUE(std::equal(update_state_message->begin(),
249251
update_state_message->end(),
@@ -366,6 +368,65 @@ TEST(TextInputPluginTest, TextEditingWorksWithDeltaModel) {
366368
// Passes if it did not crash
367369
}
368370

371+
// Regression test for https://github.com/flutter/flutter/issues/123749
372+
TEST(TextInputPluginTest, CompositionCursorPos) {
373+
int selection_base = -1;
374+
TestBinaryMessenger messenger([&](const std::string& channel,
375+
const uint8_t* message, size_t size,
376+
BinaryReply reply) {
377+
auto method = JsonMethodCodec::GetInstance().DecodeMethodCall(
378+
std::vector<uint8_t>(message, message + size));
379+
if (method->method_name() == kUpdateEditingStateMethod) {
380+
const auto& args = *method->arguments();
381+
const auto& editing_state = args[1];
382+
auto base = editing_state.FindMember(kSelectionBaseKey);
383+
auto extent = editing_state.FindMember(kSelectionExtentKey);
384+
ASSERT_NE(base, editing_state.MemberEnd());
385+
ASSERT_TRUE(base->value.IsInt());
386+
ASSERT_NE(extent, editing_state.MemberEnd());
387+
ASSERT_TRUE(extent->value.IsInt());
388+
selection_base = base->value.GetInt();
389+
EXPECT_EQ(extent->value.GetInt(), selection_base);
390+
}
391+
});
392+
MockTextInputPluginDelegate delegate;
393+
394+
TextInputPlugin plugin(&messenger, &delegate);
395+
396+
auto args = std::make_unique<rapidjson::Document>(rapidjson::kArrayType);
397+
auto& allocator = args->GetAllocator();
398+
args->PushBack(123, allocator); // client_id
399+
rapidjson::Value client_config(rapidjson::kObjectType);
400+
args->PushBack(client_config, allocator);
401+
auto encoded = JsonMethodCodec::GetInstance().EncodeMethodCall(
402+
MethodCall<rapidjson::Document>(kSetClientMethod, std::move(args)));
403+
EXPECT_TRUE(messenger.SimulateEngineMessage(
404+
kChannelName, encoded->data(), encoded->size(),
405+
[](const uint8_t* reply, size_t reply_size) {}));
406+
407+
plugin.ComposeBeginHook();
408+
EXPECT_EQ(selection_base, 0);
409+
plugin.ComposeChangeHook(u"abc", 3);
410+
EXPECT_EQ(selection_base, 3);
411+
412+
plugin.ComposeCommitHook();
413+
plugin.ComposeEndHook();
414+
EXPECT_EQ(selection_base, 3);
415+
416+
plugin.ComposeBeginHook();
417+
plugin.ComposeChangeHook(u"1", 1);
418+
EXPECT_EQ(selection_base, 4);
419+
420+
plugin.ComposeChangeHook(u"12", 2);
421+
EXPECT_EQ(selection_base, 5);
422+
423+
plugin.ComposeChangeHook(u"12", 1);
424+
EXPECT_EQ(selection_base, 4);
425+
426+
plugin.ComposeChangeHook(u"12", 2);
427+
EXPECT_EQ(selection_base, 5);
428+
}
429+
369430
TEST(TextInputPluginTest, TransformCursorRect) {
370431
// A position of `EditableText`.
371432
double view_x = 100;

0 commit comments

Comments
 (0)