Skip to content

Commit

Permalink
editor: add a timeout for text insertion
Browse files Browse the repository at this point in the history
This CL adds a timeout timer of 1s which starts when a text insertion
request is sent.

To reduce user confusion, the text is only inserted if the input field
is re-focused within 1s from the moment when the user hits
Insert/Replace.

Bug: b:317143117
Change-Id: I79a77eeeec0aeddd250cc86158ea0b3acd1a5a23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5138576
Reviewed-by: Curtis McMullan <curtismcmullan@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: Chuong Ho <hdchuong@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1243915}
  • Loading branch information
Chuong Ho authored and Chromium LUCI CQ committed Jan 8, 2024
1 parent 71aee9c commit 7debc2f
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 4 deletions.
2 changes: 1 addition & 1 deletion chrome/browser/ash/input_method/editor_mediator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ void EditorMediator::CacheContext() {
}
}

void EditorMediator::OnTextInserted() {
void EditorMediator::OnTextInsertionRequested() {
// After queuing the text to be inserted, closing the mako web ui should
// return the focus back to the original input.
mako_bubble_coordinator_.CloseUI();
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ash/input_method/editor_mediator.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class EditorMediator : public EditorEventSink,
void OnDisplayTabletStateChanged(display::TabletState state) override;

// EditorTextActuator::Delegate overrides
void OnTextInserted() override;
void OnTextInsertionRequested() override;
void ProcessConsentAction(ConsentAction consent_action) override;
void ShowUI() override;
void CloseUI() override;
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ash/input_method/editor_text_actuator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ void EditorTextActuator::InsertText(const std::string& text) {
// We queue the text to be inserted here rather then insert it directly into
// the input.
inserter_.InsertTextOnNextFocus(text);
delegate_->OnTextInserted();
delegate_->OnTextInsertionRequested();
}

void EditorTextActuator::ApproveConsent() {
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ash/input_method/editor_text_actuator.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class EditorTextActuator : public orca::mojom::TextActuator {
class Delegate {
public:
virtual ~Delegate() = default;
virtual void OnTextInserted() = 0;
virtual void OnTextInsertionRequested() = 0;
virtual void ProcessConsentAction(ConsentAction consent_action) = 0;
virtual void ShowUI() = 0;
virtual void CloseUI() = 0;
Expand Down
14 changes: 14 additions & 0 deletions chrome/browser/ash/input_method/editor_text_inserter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ namespace ash {
namespace input_method {
namespace {

constexpr base::TimeDelta kInsertionTimeout = base::Seconds(1);

void InsertText(std::string_view text) {
TextInputTarget* input = IMEBridge::Get()->GetInputContextHandler();
if (!input) {
Expand All @@ -30,6 +32,13 @@ EditorTextInserter::~EditorTextInserter() = default;

void EditorTextInserter::InsertTextOnNextFocus(std::string_view text) {
pending_text_insert_ = PendingTextInsert{std::string(text)};

// Starts the timer and if the text field is not re-focused after the timer
// expires, do not insert the text.
text_insertion_timer_.Start(
FROM_HERE, kInsertionTimeout,
base::BindOnce(&EditorTextInserter::CancelTextInsertion,
weak_ptr_factory_.GetWeakPtr()));
}

void EditorTextInserter::OnFocus(int context_id) {
Expand All @@ -40,12 +49,17 @@ void EditorTextInserter::OnFocus(int context_id) {
if (pending_text_insert_) {
InsertText(pending_text_insert_->text);
pending_text_insert_ = absl::nullopt;
text_insertion_timer_.Reset();
}
}

void EditorTextInserter::OnBlur() {
focused_client_ = absl::nullopt;
}

void EditorTextInserter::CancelTextInsertion() {
pending_text_insert_ = absl::nullopt;
}

} // namespace input_method
} // namespace ash
9 changes: 9 additions & 0 deletions chrome/browser/ash/input_method/editor_text_inserter.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@

#include <string>

#include "base/memory/weak_ptr.h"
#include "base/time/time.h"
#include "base/timer/timer.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

namespace ash {
Expand Down Expand Up @@ -35,12 +38,18 @@ class EditorTextInserter {
std::string text;
};

void CancelTextInsertion();

// Holds any pending text insertions. It is assumed that only one text
// insertion will be requested at any given time.
absl::optional<PendingTextInsert> pending_text_insert_;

// Holds the context of a focused text client.
absl::optional<TextClientContext> focused_client_;

base::OneShotTimer text_insertion_timer_;

base::WeakPtrFactory<EditorTextInserter> weak_ptr_factory_{this};
};

} // namespace input_method
Expand Down

0 comments on commit 7debc2f

Please sign in to comment.