Commit 1b2590be authored by Xiyuan Xia's avatar Xiyuan Xia Committed by Commit Bot

SPM: Fix suggestions not always working

- Make InputMethodMus send changed text client data before
  notifying OnCaretBoundsChanged because InputMethodChromeOS
  processes on the data in its OnCaretBoundsChanged.
- Adjust RemoteTextInputClient::GetTextFromRange to allow empty
  range on empty text. Otherwise, when all text is deleted,
  GetTextFromRange returns false that causes IMEEngine's
  SetSurroundingText not getting called in this scenario and
  breaks suggestions later on.

Bug: 933059, 933019
Change-Id: I93802a78c6c605c5f4dcdb5ea6a806e149745f17
Reviewed-on: https://chromium-review.googlesource.com/c/1478262
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#633879}
parent 48ddd7ef
...@@ -143,7 +143,8 @@ bool RemoteTextInputClient::DeleteRange(const gfx::Range& range) { ...@@ -143,7 +143,8 @@ bool RemoteTextInputClient::DeleteRange(const gfx::Range& range) {
bool RemoteTextInputClient::GetTextFromRange(const gfx::Range& range, bool RemoteTextInputClient::GetTextFromRange(const gfx::Range& range,
base::string16* text) const { base::string16* text) const {
if (!details_->data->text.has_value() || if (!details_->data->text.has_value() ||
range.GetMin() >= details_->data->text->length()) { !details_->data->text_range.has_value() ||
!details_->data->text_range->Contains(range)) {
return false; return false;
} }
......
...@@ -168,11 +168,14 @@ void InputMethodMus::OnCaretBoundsChanged(const ui::TextInputClient* client) { ...@@ -168,11 +168,14 @@ void InputMethodMus::OnCaretBoundsChanged(const ui::TextInputClient* client) {
if (!IsTextInputClientFocused(client)) if (!IsTextInputClientFocused(client))
return; return;
// Sends text input client data (if changed) before caret change because
// InputMethodChromeOS accesses the data in its OnCaretBoundsChanged.
OnTextInputClientDataChanged(client);
if (input_method_) if (input_method_)
input_method_->OnCaretBoundsChanged(client->GetCaretBounds()); input_method_->OnCaretBoundsChanged(client->GetCaretBounds());
NotifyTextInputCaretBoundsChanged(client); NotifyTextInputCaretBoundsChanged(client);
OnTextInputClientDataChanged(client);
} }
void InputMethodMus::CancelComposition(const ui::TextInputClient* client) { void InputMethodMus::CancelComposition(const ui::TextInputClient* client) {
......
...@@ -8,6 +8,8 @@ ...@@ -8,6 +8,8 @@
#include <utility> #include <utility>
#include "base/bind.h" #include "base/bind.h"
#include "base/callback.h"
#include "base/test/bind_test_util.h"
#include "services/ws/public/mojom/ime/ime.mojom.h" #include "services/ws/public/mojom/ime/ime.mojom.h"
#include "ui/aura/test/aura_test_base.h" #include "ui/aura/test/aura_test_base.h"
#include "ui/aura/test/mus/input_method_mus_test_api.h" #include "ui/aura/test/mus/input_method_mus_test_api.h"
...@@ -21,8 +23,8 @@ namespace { ...@@ -21,8 +23,8 @@ namespace {
// Empty implementation of InputMethodDelegate. // Empty implementation of InputMethodDelegate.
class TestInputMethodDelegate : public ui::internal::InputMethodDelegate { class TestInputMethodDelegate : public ui::internal::InputMethodDelegate {
public: public:
TestInputMethodDelegate() {} TestInputMethodDelegate() = default;
~TestInputMethodDelegate() override {} ~TestInputMethodDelegate() override = default;
bool was_dispatch_key_event_post_ime_called() const { bool was_dispatch_key_event_post_ime_called() const {
return was_dispatch_key_event_post_ime_called_; return was_dispatch_key_event_post_ime_called_;
...@@ -51,13 +53,17 @@ using EventResultCallback = base::OnceCallback<void(ws::mojom::EventResult)>; ...@@ -51,13 +53,17 @@ using EventResultCallback = base::OnceCallback<void(ws::mojom::EventResult)>;
// ProcessKeyEvent(). // ProcessKeyEvent().
class TestInputMethod : public ws::mojom::InputMethod { class TestInputMethod : public ws::mojom::InputMethod {
public: public:
TestInputMethod() {} TestInputMethod() = default;
~TestInputMethod() override {} ~TestInputMethod() override = default;
ProcessKeyEventCallbacks* process_key_event_callbacks() { ProcessKeyEventCallbacks* process_key_event_callbacks() {
return &process_key_event_callbacks_; return &process_key_event_callbacks_;
} }
void set_on_carent_bounds_changed_closure(base::OnceClosure closure) {
on_caret_bounds_changed_closure_ = std::move(closure);
}
// ui::ime::InputMethod: // ui::ime::InputMethod:
void OnTextInputStateChanged( void OnTextInputStateChanged(
ws::mojom::TextInputStatePtr text_input_state) override { ws::mojom::TextInputStatePtr text_input_state) override {
...@@ -65,6 +71,8 @@ class TestInputMethod : public ws::mojom::InputMethod { ...@@ -65,6 +71,8 @@ class TestInputMethod : public ws::mojom::InputMethod {
} }
void OnCaretBoundsChanged(const gfx::Rect& caret_bounds) override { void OnCaretBoundsChanged(const gfx::Rect& caret_bounds) override {
was_on_caret_bounds_changed_called_ = true; was_on_caret_bounds_changed_called_ = true;
if (on_caret_bounds_changed_closure_)
std::move(on_caret_bounds_changed_closure_).Run();
} }
void OnTextInputClientDataChanged( void OnTextInputClientDataChanged(
ws::mojom::TextInputClientDataPtr data) override { ws::mojom::TextInputClientDataPtr data) override {
...@@ -119,8 +127,8 @@ class TestInputMethod : public ws::mojom::InputMethod { ...@@ -119,8 +127,8 @@ class TestInputMethod : public ws::mojom::InputMethod {
bool was_cancel_composition_called_ = false; bool was_cancel_composition_called_ = false;
bool was_show_virtual_keyboard_if_enabled_called_ = false; bool was_show_virtual_keyboard_if_enabled_called_ = false;
ProcessKeyEventCallbacks process_key_event_callbacks_; ProcessKeyEventCallbacks process_key_event_callbacks_;
ws::mojom::TextInputClientDataPtr text_input_client_data_; ws::mojom::TextInputClientDataPtr text_input_client_data_;
base::OnceClosure on_caret_bounds_changed_closure_;
DISALLOW_COPY_AND_ASSIGN(TestInputMethod); DISALLOW_COPY_AND_ASSIGN(TestInputMethod);
}; };
...@@ -491,4 +499,26 @@ TEST_F(InputMethodMusTest, IsTextEditCommandEnabled) { ...@@ -491,4 +499,26 @@ TEST_F(InputMethodMusTest, IsTextEditCommandEnabled) {
} }
} }
// Tests that text input client data is sent over before caret change.
TEST_F(InputMethodMusTest, TextInputClientDataAvailableBeforeCaretChange) {
TestTextInputClient focused_input_client;
TestInputMethodDelegate input_method_delegate;
InputMethodMus input_method_mus(&input_method_delegate, nullptr);
input_method_mus.SetFocusedTextInputClient(&focused_input_client);
TestInputMethod test_input_method;
InputMethodMusTestApi::SetInputMethod(&input_method_mus, &test_input_method);
EXPECT_FALSE(
test_input_method.was_on_text_input_client_data_changed_called());
test_input_method.set_on_carent_bounds_changed_closure(
base::BindLambdaForTesting([&] {
EXPECT_TRUE(
test_input_method.was_on_text_input_client_data_changed_called());
}));
InputMethodMusTestApi::CallOnCaretBoundsChanged(&input_method_mus,
&focused_input_client);
}
} // namespace aura } // namespace aura
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment