firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

feature: make the text input plugin use the correct view on the Windows platform (#163847)

Browse Source 
## What's new?
- Updates the `TextInput.setClient` method to expect a view ID, which is
already being sent up by clients. This makes it so that the IME info
shows on the text input correctly across views 🎉. Also - text input
on windows works properly across views (although it was working before
too)
- Using the view ID in `TextInputPlugin::HandleMethodCall` to resolve
the view
- Update tests to no longer assume that we are using the implicit view
id
- Add two tests to ensure that the view id is set

## What's fixed?
- Partially fixes: https://github.com/flutter/flutter/issues/142845

## 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], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [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/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
This commit is contained in:
Matthew Kosarek 2025-03-04 10:22:20 -05:00 committed by GitHub
parent 249e954600
commit 9246b02af1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 202 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
engine/src/flutter/shell/platform/windows/keyboard_unittests.cc
View File

@ -2203,7 +2203,7 @@ TEST_F(KeyboardTest, TextInputSubmit) {
tester.InjectPlatformMessage(
"flutter/textinput", "TextInput.setClient",
R"|([108, {"inputAction": "TextInputAction.none"}])|");
R"|([108, {"inputAction": "TextInputAction.none", "viewId": 0}])|");
// Press Enter
tester.InjectKeyboardChanges(std::vector<KeyboardChange>{

5
engine/src/flutter/shell/platform/windows/testing/engine_modifier.h
View File

@ -43,6 +43,11 @@ class EngineModifier {
engine_->views_[kImplicitViewId] = view;
}
/// Associate a view with a view id.
void SetViewById(FlutterWindowsView* view, FlutterViewId viewId) {
engine_->views_[viewId] = view;
}
/// Reset the start_time field that is used to align vsync events.
void SetStartTime(uint64_t start_time_nanos) {
engine_->start_time_ = std::chrono::nanoseconds(start_time_nanos);

40
engine/src/flutter/shell/platform/windows/text_input_plugin.cc
View File

@ -38,6 +38,7 @@ static constexpr char kDeltaEndKey[] = "deltaEnd";
static constexpr char kDeltasKey[] = "deltas";
static constexpr char kEnableDeltaModel[] = "enableDeltaModel";
static constexpr char kTextInputAction[] = "inputAction";
static constexpr char kViewId[] = "viewId";
static constexpr char kTextInputType[] = "inputType";
static constexpr char kTextInputTypeName[] = "name";
static constexpr char kComposingBaseKey[] = "composingBase";
@ -216,12 +217,12 @@ void TextInputPlugin::HandleMethodCall(
if (method.compare(kShowMethod) == 0 || method.compare(kHideMethod) == 0) {
// These methods are no-ops.
} else if (method.compare(kClearClientMethod) == 0) {
// TODO(loicsharma): Remove implicit view assumption.
// https://github.com/flutter/flutter/issues/142845
FlutterWindowsView* view = engine_->view(kImplicitViewId);
FlutterWindowsView* view = engine_->view(view_id_);
if (view == nullptr) {
result->Error(kInternalConsistencyError,
"Text input is not available in Windows headless mode");
std::stringstream ss;
ss << "Text input is not available because view with view_id=" << view_id_
<< " cannot be found";
result->Error(kInternalConsistencyError, ss.str());
return;
}
if (active_model_ != nullptr && active_model_->composing()) {
@ -255,6 +256,15 @@ void TextInputPlugin::HandleMethodCall(
enable_delta_model_json->value.IsBool()) {
enable_delta_model = enable_delta_model_json->value.GetBool();
}
auto view_id_json = client_config.FindMember(kViewId);
if (view_id_json != client_config.MemberEnd() &&
view_id_json->value.IsInt()) {
view_id_ = view_id_json->value.GetInt();
} else {
result->Error(kBadArgumentError,
"Could not set client, view ID is null.");
return;
}
input_action_ = "";
auto input_action_json = client_config.FindMember(kTextInputAction);
if (input_action_json != client_config.MemberEnd() &&
@ -328,12 +338,12 @@ void TextInputPlugin::HandleMethodCall(
TextRange(composing_base, composing_extent), cursor_offset);
}
} else if (method.compare(kSetMarkedTextRect) == 0) {
// TODO(loicsharma): Remove implicit view assumption.
// https://github.com/flutter/flutter/issues/142845
FlutterWindowsView* view = engine_->view(kImplicitViewId);
FlutterWindowsView* view = engine_->view(view_id_);
if (view == nullptr) {
result->Error(kInternalConsistencyError,
"Text input is not available in Windows headless mode");
std::stringstream ss;
ss << "Text input is not available because view with view_id=" << view_id_
<< " cannot be found";
result->Error(kInternalConsistencyError, ss.str());
return;
}
if (!method_call.arguments() || method_call.arguments()->IsNull()) {
@ -359,12 +369,12 @@ void TextInputPlugin::HandleMethodCall(
Rect transformed_rect = GetCursorRect();
view->OnCursorRectUpdated(transformed_rect);
} else if (method.compare(kSetEditableSizeAndTransform) == 0) {
// TODO(loicsharma): Remove implicit view assumption.
// https://github.com/flutter/flutter/issues/142845
FlutterWindowsView* view = engine_->view(kImplicitViewId);
FlutterWindowsView* view = engine_->view(view_id_);
if (view == nullptr) {
result->Error(kInternalConsistencyError,
"Text input is not available in Windows headless mode");
std::stringstream ss;
ss << "Text input is not available because view with view_id=" << view_id_
<< " cannot be found";
result->Error(kInternalConsistencyError, ss.str());
return;
}
if (!method_call.arguments() || method_call.arguments()->IsNull()) {

7
engine/src/flutter/shell/platform/windows/text_input_plugin.h
View File

@ -16,6 +16,7 @@
#include "flutter/shell/platform/common/json_method_codec.h"
#include "flutter/shell/platform/common/text_editing_delta.h"
#include "flutter/shell/platform/common/text_input_model.h"
#include "flutter/shell/platform/embedder/embedder.h"
#include "flutter/shell/platform/windows/keyboard_handler_base.h"
namespace flutter {
@ -70,6 +71,9 @@ class TextInputPlugin {
virtual void ComposeChangeHook(const std::u16string& text, int cursor_pos);
private:
// Allows modifying the TextInputPlugin in tests.
friend class TextInputPluginModifier;
// Sends the current state of the given model to the Flutter engine.
void SendStateUpdate(const TextInputModel& model);
@ -98,6 +102,9 @@ class TextInputPlugin {
// The active client id.
int client_id_;
// The active view id.
FlutterViewId view_id_ = 0;
// The active model. nullptr if not set.
std::unique_ptr<TextInputModel> active_model_;

171
engine/src/flutter/shell/platform/windows/text_input_plugin_unittest.cc
View File

@ -20,6 +20,22 @@
#include "gtest/gtest.h"
namespace flutter {
class TextInputPluginModifier {
public:
explicit TextInputPluginModifier(TextInputPlugin* text_input_plugin)
: text_input_plugin(text_input_plugin) {}
void SetViewId(FlutterViewId view_id) {
text_input_plugin->view_id_ = view_id;
}
private:
TextInputPlugin* text_input_plugin;
FML_DISALLOW_COPY_AND_ASSIGN(TextInputPluginModifier);
};
namespace testing {
namespace {
@ -33,6 +49,7 @@ static constexpr int kDefaultClientId = 42;
// Should be identical to constants in text_input_plugin.cc.
static constexpr char kChannelName[] = "flutter/textinput";
static constexpr char kEnableDeltaModel[] = "enableDeltaModel";
static constexpr char kViewId[] = "viewId";
static constexpr char kSetClientMethod[] = "TextInput.setClient";
static constexpr char kAffinityDownstream[] = "TextAffinity.downstream";
static constexpr char kTextKey[] = "text";
@ -63,6 +80,7 @@ static std::unique_ptr<rapidjson::Document> EncodedClientConfig(
rapidjson::Value config(rapidjson::kObjectType);
config.AddMember("inputAction", input_action, allocator);
config.AddMember(kEnableDeltaModel, false, allocator);
config.AddMember(kViewId, 456, allocator);
rapidjson::Value type_info(rapidjson::kObjectType);
type_info.AddMember("name", type_name, allocator);
config.AddMember("inputType", type_info, allocator);
@ -149,7 +167,20 @@ class TextInputPluginTest : public WindowsTest {
std::move(window));
EngineModifier modifier{engine_.get()};
modifier.SetImplicitView(view_.get());
modifier.SetViewById(view_.get(), 456);
}
std::unique_ptr<MockFlutterWindowsView> AddViewWithId(int view_id) {
EXPECT_NE(engine_, nullptr);
auto window = std::make_unique<MockWindowBindingHandler>();
EXPECT_CALL(*window, SetView).Times(1);
EXPECT_CALL(*window, GetWindowHandle).WillRepeatedly(Return(nullptr));
auto view = std::make_unique<MockFlutterWindowsView>(engine_.get(),
std::move(window));
EngineModifier modifier{engine_.get()};
modifier.SetViewById(view_.get(), view_id);
return view;
}
private:
@ -194,6 +225,8 @@ TEST_F(TextInputPluginTest, ClearClientResetsComposing) {
BinaryReply reply_handler = [](const uint8_t* reply, size_t reply_size) {};
TextInputPlugin handler(&messenger, engine());
TextInputPluginModifier modifier(&handler);
modifier.SetViewId(456);
EXPECT_CALL(*view(), OnResetImeComposing());
@ -224,9 +257,10 @@ TEST_F(TextInputPluginTest, ClearClientRequiresView) {
messenger.SimulateEngineMessage(kChannelName, message->data(),
message->size(), reply_handler);
EXPECT_EQ(reply,
"[\"Internal Consistency Error\",\"Text input is not available in "
"Windows headless mode\",null]");
EXPECT_EQ(
reply,
"[\"Internal Consistency Error\",\"Text input is not available because "
"view with view_id=0 cannot be found\",null]");
}
// Verify that the embedder sends state update messages to the framework during
@ -253,6 +287,7 @@ TEST_F(TextInputPluginTest, VerifyComposingSendStateUpdate) {
config.AddMember("inputAction", "done", allocator);
config.AddMember("inputType", "text", allocator);
config.AddMember(kEnableDeltaModel, false, allocator);
config.AddMember(kViewId, 456, allocator);
arguments->PushBack(config, allocator);
auto message =
codec.EncodeMethodCall({"TextInput.setClient", std::move(arguments)});
@ -392,6 +427,71 @@ TEST_F(TextInputPluginTest, VerifyInputActionSendDoesNotInsertNewLine) {
messages.front().begin()));
}
TEST_F(TextInputPluginTest, SetClientRequiresViewId) {
UseEngineWithView();
TestBinaryMessenger messenger([](const std::string& channel,
const uint8_t* message, size_t message_size,
BinaryReply reply) {});
TextInputPlugin handler(&messenger, engine());
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)));
std::string reply;
BinaryReply reply_handler = [&reply](const uint8_t* reply_bytes,
size_t reply_size) {
reply = std::string(reinterpret_cast<const char*>(reply_bytes), reply_size);
};
EXPECT_TRUE(messenger.SimulateEngineMessage(kChannelName, encoded->data(),
encoded->size(), reply_handler));
EXPECT_EQ(
reply,
"[\"Bad Arguments\",\"Could not set client, view ID is null.\",null]");
}
TEST_F(TextInputPluginTest, SetClientRequiresViewIdToBeInteger) {
UseEngineWithView();
TestBinaryMessenger messenger([](const std::string& channel,
const uint8_t* message, size_t message_size,
BinaryReply reply) {});
TextInputPlugin handler(&messenger, engine());
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);
client_config.AddMember(kViewId, "Not an integer", allocator); // view_id
args->PushBack(client_config, allocator);
auto encoded = JsonMethodCodec::GetInstance().EncodeMethodCall(
MethodCall<rapidjson::Document>(kSetClientMethod, std::move(args)));
std::string reply;
BinaryReply reply_handler = [&reply](const uint8_t* reply_bytes,
size_t reply_size) {
reply = std::string(reinterpret_cast<const char*>(reply_bytes), reply_size);
};
EXPECT_TRUE(messenger.SimulateEngineMessage(kChannelName, encoded->data(),
encoded->size(), reply_handler));
EXPECT_EQ(
reply,
"[\"Bad Arguments\",\"Could not set client, view ID is null.\",null]");
}
TEST_F(TextInputPluginTest, TextEditingWorksWithDeltaModel) {
UseEngineWithView();
@ -413,6 +513,7 @@ TEST_F(TextInputPluginTest, TextEditingWorksWithDeltaModel) {
rapidjson::Value client_config(rapidjson::kObjectType);
client_config.AddMember(kEnableDeltaModel, true, allocator);
client_config.AddMember(kViewId, 456, allocator);
args->PushBack(client_config, allocator);
auto encoded = JsonMethodCodec::GetInstance().EncodeMethodCall(
@ -479,6 +580,7 @@ TEST_F(TextInputPluginTest, CompositionCursorPos) {
auto& allocator = args->GetAllocator();
args->PushBack(123, allocator); // client_id
rapidjson::Value client_config(rapidjson::kObjectType);
client_config.AddMember(kViewId, 456, allocator);
args->PushBack(client_config, allocator);
auto encoded = JsonMethodCodec::GetInstance().EncodeMethodCall(
MethodCall<rapidjson::Document>(kSetClientMethod, std::move(args)));
@ -535,6 +637,8 @@ TEST_F(TextInputPluginTest, TransformCursorRect) {
BinaryReply reply_handler = [](const uint8_t* reply, size_t reply_size) {};
TextInputPlugin handler(&messenger, engine());
TextInputPluginModifier modifier(&handler);
modifier.SetViewId(456);
auto& codec = JsonMethodCodec::GetInstance();
@ -611,9 +715,62 @@ TEST_F(TextInputPluginTest, SetMarkedTextRectRequiresView) {
messenger.SimulateEngineMessage(kChannelName, message->data(),
message->size(), reply_handler);
EXPECT_EQ(reply,
"[\"Internal Consistency Error\",\"Text input is not available in "
"Windows headless mode\",null]");
EXPECT_EQ(
reply,
"[\"Internal Consistency Error\",\"Text input is not available because "
"view with view_id=0 cannot be found\",null]");
}
TEST_F(TextInputPluginTest, SetAndUseMultipleClients) {
UseEngineWithView(); // Creates the default view
AddViewWithId(789); // Creates the next view
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; });
TextInputPlugin handler(&messenger, engine());
auto const set_client_and_send_message = [&](int client_id, int view_id) {
auto args = std::make_unique<rapidjson::Document>(rapidjson::kArrayType);
auto& allocator = args->GetAllocator();
args->PushBack(client_id, allocator); // client_id
rapidjson::Value client_config(rapidjson::kObjectType);
client_config.AddMember(kViewId, view_id, allocator); // view_id
args->PushBack(client_config, allocator);
auto encoded = JsonMethodCodec::GetInstance().EncodeMethodCall(
MethodCall<rapidjson::Document>(kSetClientMethod, std::move(args)));
std::string reply;
BinaryReply reply_handler = [&reply](const uint8_t* reply_bytes,
size_t reply_size) {
reply =
std::string(reinterpret_cast<const char*>(reply_bytes), reply_size);
};
EXPECT_TRUE(messenger.SimulateEngineMessage(
kChannelName, encoded->data(), encoded->size(), reply_handler));
sent_message = false;
handler.ComposeBeginHook();
EXPECT_TRUE(sent_message);
sent_message = false;
handler.ComposeChangeHook(u"4", 1);
EXPECT_TRUE(sent_message);
sent_message = false;
handler.ComposeCommitHook();
EXPECT_FALSE(sent_message);
sent_message = false;
handler.ComposeEndHook();
EXPECT_TRUE(sent_message);
};
set_client_and_send_message(123, 456); // Set and send for the first view
set_client_and_send_message(123, 789); // Set and send for the next view
}
} // namespace testing