Commit b030e3d6 authored by Findit's avatar Findit

Revert "ime: Handle InsertTextCursorBehavior in InputMethodChromeOS."

This reverts commit 29ff7965.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 840082 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzI5ZmY3OTY1MmQ0YTRmMDIxNzc5ZjBlNGIyNDdkNTNiMjgxMDQzZGYM

Sample Failed Build: https://ci.chromium.org/b/8858949956473740752

Sample Failed Step: ui_base_unittests

Original change's description:
> ime: Handle InsertTextCursorBehavior in InputMethodChromeOS.
> 
> TextInputClients now support moving the cursor after committing some
> text. This patch adds support for input engines to call the new API.
> 
> There is a slight complication with multiple CommitText calls during
> key event handling. Previously, when handling a key event, any calls
> to CommitText will be coalesced together and executed once the key
> event is handled. However, this coalescing logic doesn't handle
> cursor movement.
> 
> Add logic to handle cursor movement when coalescing commits by observing that
> any sequence of CommitTexts with cursor movement can be implemented with two
> separate CommitTexts. Note that this is a no-op change if the IME never uses
> the new cursor movement parameter.
> 
> Bug: 1155331
> Change-Id: I6c89ec1cde5624eca7bf0d5c803085f7d7803c71
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2576067
> Reviewed-by: Keith Lee <keithlee@chromium.org>
> Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
> Commit-Queue: Darren Shen <shend@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#840082}


Change-Id: If06b84f85826520e8a65ebf908eba18ff7bd2428
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1155331
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2607068
Cr-Commit-Position: refs/heads/master@{#840114}
parent 5eef4e30
...@@ -138,9 +138,7 @@ void DictationChromeos::DictationOff() { ...@@ -138,9 +138,7 @@ void DictationChromeos::DictationOff() {
ui::IMEInputContextHandlerInterface* input_context = GetInputContext(); ui::IMEInputContextHandlerInterface* input_context = GetInputContext();
if (input_context) if (input_context)
input_context->CommitText( input_context->CommitText(base::UTF16ToUTF8(composition_->text));
base::UTF16ToUTF8(composition_->text),
ui::TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
composition_->text = base::string16(); composition_->text = base::string16();
} else { } else {
......
...@@ -189,8 +189,7 @@ void AutocorrectManager::UndoAutocorrect() { ...@@ -189,8 +189,7 @@ void AutocorrectManager::UndoAutocorrect() {
input_context->CommitText( input_context->CommitText(
(base::UTF16ToUTF8( (base::UTF16ToUTF8(
surrounding_text.surrounding_text.substr(0, range.start())) + surrounding_text.surrounding_text.substr(0, range.start())) +
original_text_), original_text_));
ui::TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
LogAssistiveAutocorrectAction(AutocorrectActions::kReverted); LogAssistiveAutocorrectAction(AutocorrectActions::kReverted);
RecordAssistiveCoverage(AssistiveType::kAutocorrectReverted); RecordAssistiveCoverage(AssistiveType::kAutocorrectReverted);
RecordAssistiveSuccess(AssistiveType::kAutocorrectReverted); RecordAssistiveSuccess(AssistiveType::kAutocorrectReverted);
......
...@@ -502,9 +502,7 @@ void InputMethodEngine::CommitTextToInputContext(int context_id, ...@@ -502,9 +502,7 @@ void InputMethodEngine::CommitTextToInputContext(int context_id,
return; return;
const bool had_composition_text = input_context->HasCompositionText(); const bool had_composition_text = input_context->HasCompositionText();
input_context->CommitText( input_context->CommitText(text);
text,
ui::TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
if (had_composition_text) { if (had_composition_text) {
// Records histograms for committed characters with composition text. // Records histograms for committed characters with composition text.
......
...@@ -412,9 +412,7 @@ void NativeInputMethodEngine::ImeObserver::OnInputMethodOptionsChanged( ...@@ -412,9 +412,7 @@ void NativeInputMethodEngine::ImeObserver::OnInputMethodOptionsChanged(
} }
void NativeInputMethodEngine::ImeObserver::CommitText(const std::string& text) { void NativeInputMethodEngine::ImeObserver::CommitText(const std::string& text) {
GetInputContext()->CommitText( GetInputContext()->CommitText(NormalizeString(text));
NormalizeString(text),
ui::TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
} }
void NativeInputMethodEngine::ImeObserver::SetComposition( void NativeInputMethodEngine::ImeObserver::SetComposition(
...@@ -493,10 +491,7 @@ void NativeInputMethodEngine::ImeObserver::OnRuleBasedKeyEventResponse( ...@@ -493,10 +491,7 @@ void NativeInputMethodEngine::ImeObserver::OnRuleBasedKeyEventResponse(
for (const auto& op : response->operations) { for (const auto& op : response->operations) {
switch (op->method) { switch (op->method) {
case ime::mojom::OperationMethodForRulebased::COMMIT_TEXT: case ime::mojom::OperationMethodForRulebased::COMMIT_TEXT:
GetInputContext()->CommitText( GetInputContext()->CommitText(NormalizeString(op->arguments));
NormalizeString(op->arguments),
ui::TextInputClient::InsertTextCursorBehavior::
kMoveCursorAfterText);
break; break;
case ime::mojom::OperationMethodForRulebased::SET_COMPOSITION: case ime::mojom::OperationMethodForRulebased::SET_COMPOSITION:
ui::CompositionText composition; ui::CompositionText composition;
......
...@@ -11,7 +11,6 @@ ...@@ -11,7 +11,6 @@
#include "base/component_export.h" #include "base/component_export.h"
#include "ui/base/ime/composition_text.h" #include "ui/base/ime/composition_text.h"
#include "ui/base/ime/input_method.h" #include "ui/base/ime/input_method.h"
#include "ui/base/ime/text_input_client.h"
#include "ui/events/event.h" #include "ui/events/event.h"
namespace ui { namespace ui {
...@@ -24,9 +23,7 @@ struct SurroundingTextInfo { ...@@ -24,9 +23,7 @@ struct SurroundingTextInfo {
class COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS) IMEInputContextHandlerInterface { class COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS) IMEInputContextHandlerInterface {
public: public:
// Called when the engine commit a text. // Called when the engine commit a text.
virtual void CommitText( virtual void CommitText(const std::string& text) = 0;
const std::string& text,
TextInputClient::InsertTextCursorBehavior cursor_behavior) = 0;
// Called when the engine changes the composition range. // Called when the engine changes the composition range.
// Returns true if the operation was successful. // Returns true if the operation was successful.
......
...@@ -442,7 +442,6 @@ void InputMethodChromeOS::ResetContext(bool reset_engine) { ...@@ -442,7 +442,6 @@ void InputMethodChromeOS::ResetContext(bool reset_engine) {
pending_composition_ = base::nullopt; pending_composition_ = base::nullopt;
result_text_.clear(); result_text_.clear();
result_text_cursor_ = 0;
composing_text_ = false; composing_text_ = false;
composition_changed_ = false; composition_changed_ = false;
...@@ -579,22 +578,9 @@ void InputMethodChromeOS::ProcessInputMethodResult(ui::KeyEvent* event, ...@@ -579,22 +578,9 @@ void InputMethodChromeOS::ProcessInputMethodResult(ui::KeyEvent* event,
client->InsertChar(ch_event); client->InsertChar(ch_event);
} }
} else { } else {
// Split |result_text_| into two separate commits, one for the substring
// before |result_text_cursor_| and one for the substring after.
const base::string16 before_cursor =
result_text_.substr(0, result_text_cursor_);
if (!before_cursor.empty()) {
client->InsertText( client->InsertText(
before_cursor, result_text_,
TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText); TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
}
const base::string16 after_cursor =
result_text_.substr(result_text_cursor_);
if (!after_cursor.empty()) {
client->InsertText(
after_cursor,
TextInputClient::InsertTextCursorBehavior::kMoveCursorBeforeText);
}
composing_text_ = false; composing_text_ = false;
} }
} }
...@@ -626,24 +612,20 @@ void InputMethodChromeOS::ProcessInputMethodResult(ui::KeyEvent* event, ...@@ -626,24 +612,20 @@ void InputMethodChromeOS::ProcessInputMethodResult(ui::KeyEvent* event,
// We should not clear composition text here, as it may belong to the next // We should not clear composition text here, as it may belong to the next
// composition session. // composition session.
result_text_.clear(); result_text_.clear();
result_text_cursor_ = 0;
composition_changed_ = false; composition_changed_ = false;
} }
bool InputMethodChromeOS::NeedInsertChar() const { bool InputMethodChromeOS::NeedInsertChar() const {
return GetTextInputClient() && return GetTextInputClient() &&
(IsTextInputTypeNone() || (IsTextInputTypeNone() ||
(!composing_text_ && result_text_.length() == 1 && (!composing_text_ && result_text_.length() == 1));
result_text_cursor_ == 1));
} }
bool InputMethodChromeOS::HasInputMethodResult() const { bool InputMethodChromeOS::HasInputMethodResult() const {
return result_text_.length() || composition_changed_; return result_text_.length() || composition_changed_;
} }
void InputMethodChromeOS::CommitText( void InputMethodChromeOS::CommitText(const std::string& text) {
const std::string& text,
TextInputClient::InsertTextCursorBehavior cursor_behavior) {
if (text.empty()) if (text.empty())
return; return;
...@@ -664,11 +646,7 @@ void InputMethodChromeOS::CommitText( ...@@ -664,11 +646,7 @@ void InputMethodChromeOS::CommitText(
// Append the text to the buffer, because commit signal might be fired // Append the text to the buffer, because commit signal might be fired
// multiple times when processing a key event. // multiple times when processing a key event.
result_text_.insert(result_text_cursor_, utf16_text); result_text_.append(utf16_text);
if (cursor_behavior ==
TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText) {
result_text_cursor_ += utf16_text.length();
}
// If we are not handling key event, do not bother sending text result if the // If we are not handling key event, do not bother sending text result if the
// focused text input client does not support text input. // focused text input client does not support text input.
...@@ -679,7 +657,6 @@ void InputMethodChromeOS::CommitText( ...@@ -679,7 +657,6 @@ void InputMethodChromeOS::CommitText(
TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText); TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
SendFakeProcessKeyEvent(false); SendFakeProcessKeyEvent(false);
result_text_.clear(); result_text_.clear();
result_text_cursor_ = 0;
} }
} }
...@@ -790,8 +767,7 @@ bool InputMethodChromeOS::ExecuteCharacterComposer(const ui::KeyEvent& event) { ...@@ -790,8 +767,7 @@ bool InputMethodChromeOS::ExecuteCharacterComposer(const ui::KeyEvent& event) {
std::string commit_text = std::string commit_text =
base::UTF16ToUTF8(character_composer_.composed_character()); base::UTF16ToUTF8(character_composer_.composed_character());
if (!commit_text.empty()) { if (!commit_text.empty()) {
CommitText(commit_text, CommitText(commit_text);
TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
} }
return true; return true;
} }
......
...@@ -53,9 +53,7 @@ class COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS) InputMethodChromeOS ...@@ -53,9 +53,7 @@ class COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS) InputMethodChromeOS
TextInputClient* focused) override; TextInputClient* focused) override;
// ui::IMEInputContextHandlerInterface overrides: // ui::IMEInputContextHandlerInterface overrides:
void CommitText( void CommitText(const std::string& text) override;
const std::string& text,
TextInputClient::InsertTextCursorBehavior cursor_behavior) override;
bool SetCompositionRange( bool SetCompositionRange(
uint32_t before, uint32_t before,
uint32_t after, uint32_t after,
...@@ -160,10 +158,6 @@ class COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS) InputMethodChromeOS ...@@ -160,10 +158,6 @@ class COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS) InputMethodChromeOS
// processing result of the pending key event. // processing result of the pending key event.
base::string16 result_text_; base::string16 result_text_;
// Where the cursor should be place after inserting |result_text_|.
// 0 <= |result_text_cursor_| <= |result_text_.length()|.
size_t result_text_cursor_ = 0;
base::string16 previous_surrounding_text_; base::string16 previous_surrounding_text_;
gfx::Range previous_selection_range_; gfx::Range previous_selection_range_;
......
...@@ -89,10 +89,8 @@ class TestableInputMethodChromeOS : public InputMethodChromeOS { ...@@ -89,10 +89,8 @@ class TestableInputMethodChromeOS : public InputMethodChromeOS {
} }
return details; return details;
} }
void CommitText( void CommitText(const std::string& text) override {
const std::string& text, InputMethodChromeOS::CommitText(text);
TextInputClient::InsertTextCursorBehavior cursor_behavior) override {
InputMethodChromeOS::CommitText(text, cursor_behavior);
text_committed_ = text; text_committed_ = text;
} }
...@@ -449,8 +447,7 @@ TEST_F(InputMethodChromeOSTest, ...@@ -449,8 +447,7 @@ TEST_F(InputMethodChromeOSTest,
OnWillChangeFocusedClientClearAutocorrectRange) { OnWillChangeFocusedClientClearAutocorrectRange) {
input_type_ = TEXT_INPUT_TYPE_TEXT; input_type_ = TEXT_INPUT_TYPE_TEXT;
ime_->SetFocusedTextInputClient(this); ime_->SetFocusedTextInputClient(this);
ime_->CommitText( ime_->CommitText("hello");
"hello", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
ime_->SetAutocorrectRange(gfx::Range(0, 5)); ime_->SetAutocorrectRange(gfx::Range(0, 5));
EXPECT_EQ(gfx::Range(0, 5), this->GetAutocorrectRange()); EXPECT_EQ(gfx::Range(0, 5), this->GetAutocorrectRange());
...@@ -1021,9 +1018,7 @@ TEST_F(InputMethodChromeOSKeyEventTest, KeyEventDelayResponseTest) { ...@@ -1021,9 +1018,7 @@ TEST_F(InputMethodChromeOSKeyEventTest, KeyEventDelayResponseTest) {
EXPECT_EQ(kFlags, key_event->flags()); EXPECT_EQ(kFlags, key_event->flags());
EXPECT_EQ(0, ime_->process_key_event_post_ime_call_count()); EXPECT_EQ(0, ime_->process_key_event_post_ime_call_count());
static_cast<IMEInputContextHandlerInterface*>(ime_.get()) static_cast<IMEInputContextHandlerInterface*>(ime_.get())->CommitText("A");
->CommitText(
"A", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
EXPECT_EQ(0, inserted_char_); EXPECT_EQ(0, inserted_char_);
...@@ -1187,8 +1182,7 @@ TEST_F(InputMethodChromeOSKeyEventTest, ...@@ -1187,8 +1182,7 @@ TEST_F(InputMethodChromeOSKeyEventTest,
ui::KeyEvent event(ui::ET_KEY_PRESSED, ui::VKEY_A, ui::EF_NONE); ui::KeyEvent event(ui::ET_KEY_PRESSED, ui::VKEY_A, ui::EF_NONE);
ime_->DispatchKeyEvent(&event); ime_->DispatchKeyEvent(&event);
ime_->CommitText( ime_->CommitText("a");
"a", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
ime_->SetAutocorrectRange(gfx::Range(0, 1)); ime_->SetAutocorrectRange(gfx::Range(0, 1));
std::move(mock_ime_engine_handler_->last_passed_callback()) std::move(mock_ime_engine_handler_->last_passed_callback())
.Run(/*handled=*/true); .Run(/*handled=*/true);
...@@ -1197,50 +1191,4 @@ TEST_F(InputMethodChromeOSKeyEventTest, ...@@ -1197,50 +1191,4 @@ TEST_F(InputMethodChromeOSKeyEventTest,
EXPECT_EQ(gfx::Range(0, 1), GetAutocorrectRange()); EXPECT_EQ(gfx::Range(0, 1), GetAutocorrectRange());
} }
TEST_F(InputMethodChromeOSKeyEventTest,
MultipleCommitTextsWhileHandlingKeyEventCoalescesIntoOne) {
InputMethodChromeOS ime(this);
FakeTextInputClient fake_text_input_client(TEXT_INPUT_TYPE_TEXT);
ime.SetFocusedTextInputClient(&fake_text_input_client);
ui::KeyEvent event(ui::ET_KEY_PRESSED, ui::VKEY_A, ui::EF_NONE);
ime.DispatchKeyEvent(&event);
ime.CommitText(
"a", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
ime.CommitText(
"b", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
ime.CommitText(
"cde", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
std::move(mock_ime_engine_handler_->last_passed_callback())
.Run(/*handled=*/true);
EXPECT_EQ(fake_text_input_client.text(), base::ASCIIToUTF16("abcde"));
EXPECT_EQ(fake_text_input_client.selection(), gfx::Range(5, 5));
}
TEST_F(InputMethodChromeOSKeyEventTest,
MultipleCommitTextsWhileHandlingKeyEventCoalescesByCaretBehavior) {
InputMethodChromeOS ime(this);
FakeTextInputClient fake_text_input_client(TEXT_INPUT_TYPE_TEXT);
ime.SetFocusedTextInputClient(&fake_text_input_client);
ui::KeyEvent event(ui::ET_KEY_PRESSED, ui::VKEY_A, ui::EF_NONE);
ime.DispatchKeyEvent(&event);
ime.CommitText(
"a", TextInputClient::InsertTextCursorBehavior::kMoveCursorBeforeText);
ime.CommitText(
"b", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
ime.CommitText(
"c", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
ime.CommitText(
"d", TextInputClient::InsertTextCursorBehavior::kMoveCursorBeforeText);
ime.CommitText(
"e", TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
std::move(mock_ime_engine_handler_->last_passed_callback())
.Run(/*handled=*/true);
EXPECT_EQ(fake_text_input_client.text(), base::ASCIIToUTF16("bceda"));
EXPECT_EQ(fake_text_input_client.selection(), gfx::Range(3, 3));
}
} // namespace ui } // namespace ui
...@@ -21,9 +21,7 @@ MockIMEInputContextHandler::MockIMEInputContextHandler() ...@@ -21,9 +21,7 @@ MockIMEInputContextHandler::MockIMEInputContextHandler()
MockIMEInputContextHandler::~MockIMEInputContextHandler() = default; MockIMEInputContextHandler::~MockIMEInputContextHandler() = default;
void MockIMEInputContextHandler::CommitText( void MockIMEInputContextHandler::CommitText(const std::string& text) {
const std::string& text,
TextInputClient::InsertTextCursorBehavior cursor_behavior) {
++commit_text_call_count_; ++commit_text_call_count_;
last_commit_text_ = text; last_commit_text_ = text;
} }
...@@ -117,8 +115,7 @@ void MockIMEInputContextHandler::ConfirmCompositionText(bool reset_engine, ...@@ -117,8 +115,7 @@ void MockIMEInputContextHandler::ConfirmCompositionText(bool reset_engine,
return; return;
CommitText( CommitText(
base::UTF16ToUTF8(last_update_composition_arg_.composition_text.text), base::UTF16ToUTF8(last_update_composition_arg_.composition_text.text));
TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText);
last_update_composition_arg_.composition_text.text = base::string16(); last_update_composition_arg_.composition_text.text = base::string16();
} }
......
...@@ -33,9 +33,7 @@ class COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS) MockIMEInputContextHandler ...@@ -33,9 +33,7 @@ class COMPONENT_EXPORT(UI_BASE_IME_CHROMEOS) MockIMEInputContextHandler
MockIMEInputContextHandler(); MockIMEInputContextHandler();
virtual ~MockIMEInputContextHandler(); virtual ~MockIMEInputContextHandler();
void CommitText( void CommitText(const std::string& text) override;
const std::string& text,
TextInputClient::InsertTextCursorBehavior cursor_behavior) override;
void UpdateCompositionText(const CompositionText& text, void UpdateCompositionText(const CompositionText& text,
uint32_t cursor_pos, uint32_t cursor_pos,
bool visible) override; bool visible) override;
......
...@@ -30,15 +30,7 @@ void FakeTextInputClient::ClearCompositionText() {} ...@@ -30,15 +30,7 @@ void FakeTextInputClient::ClearCompositionText() {}
void FakeTextInputClient::InsertText( void FakeTextInputClient::InsertText(
const base::string16& text, const base::string16& text,
TextInputClient::InsertTextCursorBehavior cursor_behavior) { TextInputClient::InsertTextCursorBehavior cursor_behavior) {}
text_.insert(selection_.start(), text);
if (cursor_behavior ==
TextInputClient::InsertTextCursorBehavior::kMoveCursorAfterText) {
selection_ = gfx::Range(selection_.start() + text.length(),
selection_.end() + text.length());
}
}
void FakeTextInputClient::InsertChar(const KeyEvent& event) {} void FakeTextInputClient::InsertChar(const KeyEvent& event) {}
......
...@@ -25,8 +25,6 @@ class FakeTextInputClient : public TextInputClient { ...@@ -25,8 +25,6 @@ class FakeTextInputClient : public TextInputClient {
~FakeTextInputClient() override; ~FakeTextInputClient() override;
void set_text_input_type(TextInputType text_input_type); void set_text_input_type(TextInputType text_input_type);
const base::string16& text() const { return text_; }
const gfx::Range& selection() const { return selection_; }
// TextInputClient: // TextInputClient:
void SetCompositionText(const CompositionText& composition) override; void SetCompositionText(const CompositionText& composition) override;
...@@ -84,8 +82,6 @@ class FakeTextInputClient : public TextInputClient { ...@@ -84,8 +82,6 @@ class FakeTextInputClient : public TextInputClient {
private: private:
TextInputType text_input_type_; TextInputType text_input_type_;
base::string16 text_;
gfx::Range selection_;
}; };
} // namespace ui } // namespace ui
......
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