firka/flutter
3
0
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[Linux] composing delta fixes (flutter/engine#42648)

Browse Source 
Fixes a few issues with the Linux text editing delta implementation that was causing some issues when composing CJK text.

`im_preedit_changed_cb` dispatches a delta with the delta range set as the current composing region causing a crash when composing because the new composing region range might not yet exist in the `oldText` that the delta is applied to. We should instead send the composing region before the change to the text as that will be the correct range that is being modified in the `oldText`.

`im_preedit_end_cb` dispatches a delta with an empty `oldText`, a not empty `deltaText`, and a delta range of `(-1,-1)` causing a crash when composing ends because the delta range will never be within the range of the text. We should instead send the current text value as the `oldText` and not send a `deltaRange` or `deltaText` since a `(-1,-1)` range essentially means nothing in the text changed.

`im_commit_cb` does not account for the case when we were previously composing when the text was committed. This causes the dispatched delta to have a delta range that always inserts at the current the selection, when it should instead replace the previous composing range. We should account for the case when we are previously composing before committing the text, and dispatch a delta with the previous composing region.

Fixes https://github.com/flutter/flutter/issues/113909
This commit is contained in:
Renzo Olivares
2023-06-08 14:14:01 -07:00
committed by GitHub
parent c659b9a590
commit 8b7818b946
2 changed files with 261 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
engine/src/flutter/shell/platform/linux/fl_text_input_plugin.cc
View File

@@ -263,6 +263,8 @@ static void im_preedit_changed_cb(FlTextInputPlugin* self) {
FlTextInputPluginPrivate* priv = static_cast<FlTextInputPluginPrivate*>(
fl_text_input_plugin_get_instance_private(self));
std::string text_before_change = priv->text_model->GetText();
flutter::TextRange composing_before_change =
priv->text_model->composing_range();
g_autofree gchar* buf = nullptr;
gint cursor_offset = 0;
gtk_im_context_get_preedit_string(priv->im_context, &buf, nullptr,
@@ -278,7 +280,7 @@ static void im_preedit_changed_cb(FlTextInputPlugin* self) {
if (priv->enable_delta_model) {
std::string text(buf);
flutter::TextEditingDelta delta = flutter::TextEditingDelta(
text_before_change, priv->text_model->composing_range(), text);
text_before_change, composing_before_change, text);
update_editing_state_with_delta(self, &delta);
} else {
update_editing_state(self);
@@ -290,7 +292,10 @@ static void im_commit_cb(FlTextInputPlugin* self, const gchar* text) {
FlTextInputPluginPrivate* priv = static_cast<FlTextInputPluginPrivate*>(
fl_text_input_plugin_get_instance_private(self));
std::string text_before_change = priv->text_model->GetText();
flutter::TextRange composing_before_change =
priv->text_model->composing_range();
flutter::TextRange selection_before_change = priv->text_model->selection();
gboolean was_composing = priv->text_model->composing();
priv->text_model->AddText(text);
if (priv->text_model->composing()) {
@@ -298,9 +303,12 @@ static void im_commit_cb(FlTextInputPlugin* self, const gchar* text) {
}
if (priv->enable_delta_model) {
flutter::TextEditingDelta delta = flutter::TextEditingDelta(
text_before_change, selection_before_change, text);
update_editing_state_with_delta(self, &delta);
flutter::TextRange replace_range =
was_composing ? composing_before_change : selection_before_change;
std::unique_ptr<flutter::TextEditingDelta> delta =
std::make_unique<flutter::TextEditingDelta>(text_before_change,
replace_range, text);
update_editing_state_with_delta(self, delta.get());
} else {
update_editing_state(self);
}
@@ -312,8 +320,8 @@ static void im_preedit_end_cb(FlTextInputPlugin* self) {
fl_text_input_plugin_get_instance_private(self));
priv->text_model->EndComposing();
if (priv->enable_delta_model) {
flutter::TextEditingDelta delta = flutter::TextEditingDelta(
"", flutter::TextRange(-1, -1), priv->text_model->GetText());
flutter::TextEditingDelta delta =
flutter::TextEditingDelta(priv->text_model->GetText());
update_editing_state_with_delta(self, &delta);
} else {
update_editing_state(self);

254
engine/src/flutter/shell/platform/linux/fl_text_input_plugin_test.cc
View File

@@ -958,6 +958,8 @@ TEST(FlTextInputPluginTest, ComposingDelta) {
messenger.ReceiveMessage("flutter/textinput", set_client);
g_signal_emit_by_name(context, "preedit-start", nullptr);
// update
EXPECT_CALL(context,
gtk_im_context_get_preedit_string(
@@ -976,7 +978,7 @@ TEST(FlTextInputPluginTest, ComposingDelta) {
.old_text = "",
.delta_text = "Flutter ",
.delta_start = 0,
.delta_end = 8,
.delta_end = 0,
.selection_base = 8,
.selection_extent = 8,
.composing_base = 0,
@@ -1004,13 +1006,13 @@ TEST(FlTextInputPluginTest, ComposingDelta) {
build_list({
build_editing_delta({
.old_text = "Flutter ",
.delta_text = "engine",
.delta_start = 8,
.delta_text = "Flutter engine",
.delta_start = 0,
.delta_end = 8,
.selection_base = 14,
.selection_extent = 14,
.composing_base = 0,
.composing_extent = 8,
.composing_base = -1,
.composing_extent = -1,
}),
}),
}}),
@@ -1024,7 +1026,7 @@ TEST(FlTextInputPluginTest, ComposingDelta) {
FlValueEq(commit)),
::testing::_, ::testing::_, ::testing::_));
g_signal_emit_by_name(context, "commit", "engine", nullptr);
g_signal_emit_by_name(context, "commit", "Flutter engine", nullptr);
// end
g_autoptr(FlValue) end = build_list({
@@ -1033,7 +1035,7 @@ TEST(FlTextInputPluginTest, ComposingDelta) {
"deltas",
build_list({
build_editing_delta({
.delta_text = "Flutter engine",
.old_text = "Flutter engine",
.selection_base = 14,
.selection_extent = 14,
}),
@@ -1051,3 +1053,241 @@ TEST(FlTextInputPluginTest, ComposingDelta) {
g_signal_emit_by_name(context, "preedit-end", nullptr);
}
TEST(FlTextInputPluginTest, NonComposingDelta) {
::testing::NiceMock<flutter::testing::MockBinaryMessenger> messenger;
::testing::NiceMock<flutter::testing::MockIMContext> context;
::testing::NiceMock<flutter::testing::MockTextInputViewDelegate> delegate;
g_autoptr(FlTextInputPlugin) plugin =
fl_text_input_plugin_new(messenger, context, delegate);
EXPECT_NE(plugin, nullptr);
// set config
g_autoptr(FlValue) args = build_input_config({
.client_id = 1,
.enable_delta_model = true,
});
g_autoptr(FlJsonMethodCodec) codec = fl_json_method_codec_new();
g_autoptr(GBytes) set_client = fl_method_codec_encode_method_call(
FL_METHOD_CODEC(codec), "TextInput.setClient", args, nullptr);
g_autoptr(FlValue) null = fl_value_new_null();
EXPECT_CALL(messenger, fl_binary_messenger_send_response(
::testing::Eq<FlBinaryMessenger*>(messenger),
::testing::A<FlBinaryMessengerResponseHandle*>(),
SuccessResponse(null), ::testing::A<GError**>()))
.WillOnce(::testing::Return(true));
messenger.ReceiveMessage("flutter/textinput", set_client);
// commit F
g_autoptr(FlValue) commit = build_list({
fl_value_new_int(1), // client_id
build_map({{
"deltas",
build_list({
build_editing_delta({
.old_text = "",
.delta_text = "F",
.delta_start = 0,
.delta_end = 0,
.selection_base = 1,
.selection_extent = 1,
.composing_base = -1,
.composing_extent = -1,
}),
}),
}}),
});
EXPECT_CALL(messenger,
fl_binary_messenger_send_on_channel(
::testing::Eq<FlBinaryMessenger*>(messenger),
::testing::StrEq("flutter/textinput"),
MethodCall("TextInputClient.updateEditingStateWithDeltas",
FlValueEq(commit)),
::testing::_, ::testing::_, ::testing::_));
g_signal_emit_by_name(context, "commit", "F", nullptr);
// commit l
g_autoptr(FlValue) commitL = build_list({
fl_value_new_int(1), // client_id
build_map({{
"deltas",
build_list({
build_editing_delta({
.old_text = "F",
.delta_text = "l",
.delta_start = 1,
.delta_end = 1,
.selection_base = 2,
.selection_extent = 2,
.composing_base = -1,
.composing_extent = -1,
}),
}),
}}),
});
EXPECT_CALL(messenger,
fl_binary_messenger_send_on_channel(
::testing::Eq<FlBinaryMessenger*>(messenger),
::testing::StrEq("flutter/textinput"),
MethodCall("TextInputClient.updateEditingStateWithDeltas",
FlValueEq(commitL)),
::testing::_, ::testing::_, ::testing::_));
g_signal_emit_by_name(context, "commit", "l", nullptr);
// commit u
g_autoptr(FlValue) commitU = build_list({
fl_value_new_int(1), // client_id
build_map({{
"deltas",
build_list({
build_editing_delta({
.old_text = "Fl",
.delta_text = "u",
.delta_start = 2,
.delta_end = 2,
.selection_base = 3,
.selection_extent = 3,
.composing_base = -1,
.composing_extent = -1,
}),
}),
}}),
});
EXPECT_CALL(messenger,
fl_binary_messenger_send_on_channel(
::testing::Eq<FlBinaryMessenger*>(messenger),
::testing::StrEq("flutter/textinput"),
MethodCall("TextInputClient.updateEditingStateWithDeltas",
FlValueEq(commitU)),
::testing::_, ::testing::_, ::testing::_));
g_signal_emit_by_name(context, "commit", "u", nullptr);
// commit t
g_autoptr(FlValue) commitTa = build_list({
fl_value_new_int(1), // client_id
build_map({{
"deltas",
build_list({
build_editing_delta({
.old_text = "Flu",
.delta_text = "t",
.delta_start = 3,
.delta_end = 3,
.selection_base = 4,
.selection_extent = 4,
.composing_base = -1,
.composing_extent = -1,
}),
}),
}}),
});
EXPECT_CALL(messenger,
fl_binary_messenger_send_on_channel(
::testing::Eq<FlBinaryMessenger*>(messenger),
::testing::StrEq("flutter/textinput"),
MethodCall("TextInputClient.updateEditingStateWithDeltas",
FlValueEq(commitTa)),
::testing::_, ::testing::_, ::testing::_));
g_signal_emit_by_name(context, "commit", "t", nullptr);
// commit t again
g_autoptr(FlValue) commitTb = build_list({
fl_value_new_int(1), // client_id
build_map({{
"deltas",
build_list({
build_editing_delta({
.old_text = "Flut",
.delta_text = "t",
.delta_start = 4,
.delta_end = 4,
.selection_base = 5,
.selection_extent = 5,
.composing_base = -1,
.composing_extent = -1,
}),
}),
}}),
});
EXPECT_CALL(messenger,
fl_binary_messenger_send_on_channel(
::testing::Eq<FlBinaryMessenger*>(messenger),
::testing::StrEq("flutter/textinput"),
MethodCall("TextInputClient.updateEditingStateWithDeltas",
FlValueEq(commitTb)),
::testing::_, ::testing::_, ::testing::_));
g_signal_emit_by_name(context, "commit", "t", nullptr);
// commit e
g_autoptr(FlValue) commitE = build_list({
fl_value_new_int(1), // client_id
build_map({{
"deltas",
build_list({
build_editing_delta({
.old_text = "Flutt",
.delta_text = "e",
.delta_start = 5,
.delta_end = 5,
.selection_base = 6,
.selection_extent = 6,
.composing_base = -1,
.composing_extent = -1,
}),
}),
}}),
});
EXPECT_CALL(messenger,
fl_binary_messenger_send_on_channel(
::testing::Eq<FlBinaryMessenger*>(messenger),
::testing::StrEq("flutter/textinput"),
MethodCall("TextInputClient.updateEditingStateWithDeltas",
FlValueEq(commitE)),
::testing::_, ::testing::_, ::testing::_));
g_signal_emit_by_name(context, "commit", "e", nullptr);
// commit r
g_autoptr(FlValue) commitR = build_list({
fl_value_new_int(1), // client_id
build_map({{
"deltas",
build_list({
build_editing_delta({
.old_text = "Flutte",
.delta_text = "r",
.delta_start = 6,
.delta_end = 6,
.selection_base = 7,
.selection_extent = 7,
.composing_base = -1,
.composing_extent = -1,
}),
}),
}}),
});
EXPECT_CALL(messenger,
fl_binary_messenger_send_on_channel(
::testing::Eq<FlBinaryMessenger*>(messenger),
::testing::StrEq("flutter/textinput"),
MethodCall("TextInputClient.updateEditingStateWithDeltas",
FlValueEq(commitR)),
::testing::_, ::testing::_, ::testing::_));
g_signal_emit_by_name(context, "commit", "r", nullptr);
}