Skip to content

Commit

Permalink
Rename {Get,Set}SelectionRange to {Get,Set}EditableSelectionRange.
Browse files Browse the repository at this point in the history
Some implementation of TextInputClient interface, for example RWHVA, can
have text outside of an editable text field.
For those views, {Get, Set}SelectionRange is confusing names because
it's not clear whether they return selections outside of the editable
text field or not.
This CL renames them to {Get, Set}EditableSelectionRange to make it
clear that they returns only selections in an editable text field.

Bug: 901439, 908762
Test: Build.
Change-Id: I99e6140a635a61b755ebfa35efff2d306cec8d32
Reviewed-on: https://chromium-review.googlesource.com/c/1373932
Commit-Queue: Yuichiro Hanada <yhanada@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Mitsuru Oshima (gardener - slow) <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616605}
  • Loading branch information
Yuichiro Hanada authored and Commit Bot committed Dec 14, 2018
1 parent e542d68 commit 65eaaf4
Show file tree
Hide file tree
Showing 30 changed files with 84 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ mojom::TextInputStatePtr InputConnectionImpl::GetTextInputState(
gfx::Range text_range, selection_range;
base::string16 text;
client->GetTextRange(&text_range);
client->GetSelectionRange(&selection_range);
client->GetEditableSelectionRange(&selection_range);
client->GetTextFromRange(text_range, &text);

return mojom::TextInputStatePtr(
Expand Down Expand Up @@ -147,7 +147,7 @@ void InputConnectionImpl::FinishComposingText() {

ui::TextInputClient* client = GetTextInputClient();
gfx::Range selection_range, composition_range;
client->GetSelectionRange(&selection_range);
client->GetEditableSelectionRange(&selection_range);
client->GetCompositionTextRange(&composition_range);

std::string error;
Expand Down Expand Up @@ -183,7 +183,7 @@ void InputConnectionImpl::SetComposingText(

ui::TextInputClient* client = GetTextInputClient();
gfx::Range selection_range;
client->GetSelectionRange(&selection_range);
client->GetEditableSelectionRange(&selection_range);
if (text.empty() &&
selection_range.start() == static_cast<uint32_t>(selection_start) &&
selection_range.end() == static_cast<uint32_t>(selection_end)) {
Expand Down Expand Up @@ -214,15 +214,15 @@ void InputConnectionImpl::SetSelection(const gfx::Range& new_selection_range) {
ui::TextInputClient* client = GetTextInputClient();

gfx::Range selection_range;
client->GetSelectionRange(&selection_range);
client->GetEditableSelectionRange(&selection_range);
if (new_selection_range == selection_range) {
// This SetSelection call is no-op.
// Return the current state immediately.
UpdateTextInputState(true);
}

StartStateUpdateTimer();
client->SetSelectionRange(new_selection_range);
client->SetEditableSelectionRange(new_selection_range);
}

void InputConnectionImpl::StartStateUpdateTimer() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ void TextInputTestHelper::OnCaretBoundsChanged(
if (!GetTextInputClient()->GetTextRange(&text_range) ||
!GetTextInputClient()->GetTextFromRange(text_range,
&surrounding_text_) ||
!GetTextInputClient()->GetSelectionRange(&selection_range_))
!GetTextInputClient()->GetEditableSelectionRange(&selection_range_))
return;
}
if (waiting_type_ == WAIT_ON_CARET_BOUNDS_CHANGED)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,13 @@ bool RemoteTextInputClient::GetCompositionTextRange(gfx::Range* range) const {
return false;
}

bool RemoteTextInputClient::GetSelectionRange(gfx::Range* range) const {
bool RemoteTextInputClient::GetEditableSelectionRange(gfx::Range* range) const {
// TODO(moshayedi): crbug.com/631527.
NOTIMPLEMENTED_LOG_ONCE();
return false;
}

bool RemoteTextInputClient::SetSelectionRange(const gfx::Range& range) {
bool RemoteTextInputClient::SetEditableSelectionRange(const gfx::Range& range) {
// TODO(moshayedi): crbug.com/631527.
NOTIMPLEMENTED_LOG_ONCE();
return false;
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/ui/views/ime_driver/remote_text_input_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ class RemoteTextInputClient : public ui::TextInputClient,
FocusReason GetFocusReason() const override;
bool GetTextRange(gfx::Range* range) const override;
bool GetCompositionTextRange(gfx::Range* range) const override;
bool GetSelectionRange(gfx::Range* range) const override;
bool SetSelectionRange(const gfx::Range& range) override;
bool GetEditableSelectionRange(gfx::Range* range) const override;
bool SetEditableSelectionRange(const gfx::Range& range) override;
bool DeleteRange(const gfx::Range& range) override;
bool GetTextFromRange(const gfx::Range& range,
base::string16* text) const override;
Expand Down
4 changes: 2 additions & 2 deletions components/arc/ime/arc_ime_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ bool ArcImeService::GetTextRange(gfx::Range* range) const {
return true;
}

bool ArcImeService::GetSelectionRange(gfx::Range* range) const {
bool ArcImeService::GetEditableSelectionRange(gfx::Range* range) const {
if (!selection_range_.IsValid())
return false;
*range = selection_range_;
Expand Down Expand Up @@ -502,7 +502,7 @@ bool ArcImeService::GetCompositionTextRange(gfx::Range* range) const {
return false;
}

bool ArcImeService::SetSelectionRange(const gfx::Range& range) {
bool ArcImeService::SetEditableSelectionRange(const gfx::Range& range) {
return false;
}

Expand Down
4 changes: 2 additions & 2 deletions components/arc/ime/arc_ime_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ class ArcImeService : public KeyedService,
ui::TextInputType GetTextInputType() const override;
gfx::Rect GetCaretBounds() const override;
bool GetTextRange(gfx::Range* range) const override;
bool GetSelectionRange(gfx::Range* range) const override;
bool GetEditableSelectionRange(gfx::Range* range) const override;
bool GetTextFromRange(const gfx::Range& range,
base::string16* text) const override;

Expand All @@ -129,7 +129,7 @@ class ArcImeService : public KeyedService,
bool HasCompositionText() const override;
FocusReason GetFocusReason() const override;
bool GetCompositionTextRange(gfx::Range* range) const override;
bool SetSelectionRange(const gfx::Range& range) override;
bool SetEditableSelectionRange(const gfx::Range& range) override;
bool DeleteRange(const gfx::Range& range) override;
void OnInputMethodChanged() override {}
bool ChangeTextDirectionAndLayoutAlignment(
Expand Down
2 changes: 1 addition & 1 deletion components/arc/ime/arc_ime_service_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ TEST_F(ArcImeServiceTest, GetTextFromRange) {
instance_->GetTextFromRange(text_range, &temp_str);
EXPECT_EQ(text_in_range, temp_str);

instance_->GetSelectionRange(&temp);
instance_->GetEditableSelectionRange(&temp);
EXPECT_EQ(selection_range, temp);
}

Expand Down
4 changes: 2 additions & 2 deletions components/exo/text_input.cc
Original file line number Diff line number Diff line change
Expand Up @@ -172,12 +172,12 @@ bool TextInput::GetCompositionTextRange(gfx::Range* range) const {
return false;
}

bool TextInput::GetSelectionRange(gfx::Range* range) const {
bool TextInput::GetEditableSelectionRange(gfx::Range* range) const {
NOTIMPLEMENTED_LOG_ONCE();
return false;
}

bool TextInput::SetSelectionRange(const gfx::Range& range) {
bool TextInput::SetEditableSelectionRange(const gfx::Range& range) {
NOTIMPLEMENTED_LOG_ONCE();
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions components/exo/text_input.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ class TextInput : public ui::TextInputClient,
ui::TextInputClient::FocusReason GetFocusReason() const override;
bool GetTextRange(gfx::Range* range) const override;
bool GetCompositionTextRange(gfx::Range* range) const override;
bool GetSelectionRange(gfx::Range* range) const override;
bool SetSelectionRange(const gfx::Range& range) override;
bool GetEditableSelectionRange(gfx::Range* range) const override;
bool SetEditableSelectionRange(const gfx::Range& range) override;
bool DeleteRange(const gfx::Range& range) override;
bool GetTextFromRange(const gfx::Range& range,
base::string16* text) const override;
Expand Down
7 changes: 5 additions & 2 deletions content/browser/renderer_host/render_widget_host_view_aura.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1409,7 +1409,9 @@ bool RenderWidgetHostViewAura::GetCompositionTextRange(
return false;
}

bool RenderWidgetHostViewAura::GetSelectionRange(gfx::Range* range) const {
bool RenderWidgetHostViewAura::GetEditableSelectionRange(
gfx::Range* range) const {
// TODO(yhanada, crbug.com/908762): Return only selections in a text field.
if (!text_input_manager_ || !GetFocusedWidget())
return false;

Expand All @@ -1423,7 +1425,8 @@ bool RenderWidgetHostViewAura::GetSelectionRange(gfx::Range* range) const {
return true;
}

bool RenderWidgetHostViewAura::SetSelectionRange(const gfx::Range& range) {
bool RenderWidgetHostViewAura::SetEditableSelectionRange(
const gfx::Range& range) {
// TODO(suzhe): implement this method when fixing http://crbug.com/55130.
NOTIMPLEMENTED_LOG_ONCE();
return false;
Expand Down
4 changes: 2 additions & 2 deletions content/browser/renderer_host/render_widget_host_view_aura.h
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,8 @@ class CONTENT_EXPORT RenderWidgetHostViewAura
ui::TextInputClient::FocusReason GetFocusReason() const override;
bool GetTextRange(gfx::Range* range) const override;
bool GetCompositionTextRange(gfx::Range* range) const override;
bool GetSelectionRange(gfx::Range* range) const override;
bool SetSelectionRange(const gfx::Range& range) override;
bool GetEditableSelectionRange(gfx::Range* range) const override;
bool SetEditableSelectionRange(const gfx::Range& range) override;
bool DeleteRange(const gfx::Range& range) override;
bool GetTextFromRange(const gfx::Range& range,
base::string16* text) const override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6453,7 +6453,7 @@ TEST_F(InputMethodStateAuraTest, GetTextRange) {
}

// This test is for selection range.
TEST_F(InputMethodStateAuraTest, GetSelectionRange) {
TEST_F(InputMethodStateAuraTest, GetEditableSelectionRange) {
base::string16 text;
gfx::Range expected_range(0U, 1U);

Expand All @@ -6464,7 +6464,8 @@ TEST_F(InputMethodStateAuraTest, GetSelectionRange) {
gfx::Range range_from_client;

// This method always returns true.
EXPECT_TRUE(text_input_client()->GetSelectionRange(&range_from_client));
EXPECT_TRUE(
text_input_client()->GetEditableSelectionRange(&range_from_client));
EXPECT_EQ(expected_range, range_from_client);

// Changing range to make sure that the next view has a different text
Expand Down
4 changes: 2 additions & 2 deletions ui/base/ime/dummy_text_input_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,11 @@ bool DummyTextInputClient::GetCompositionTextRange(gfx::Range* range) const {
return false;
}

bool DummyTextInputClient::GetSelectionRange(gfx::Range* range) const {
bool DummyTextInputClient::GetEditableSelectionRange(gfx::Range* range) const {
return false;
}

bool DummyTextInputClient::SetSelectionRange(const gfx::Range& range) {
bool DummyTextInputClient::SetEditableSelectionRange(const gfx::Range& range) {
selection_history_.push_back(range);
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions ui/base/ime/dummy_text_input_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ class DummyTextInputClient : public TextInputClient {
ui::TextInputClient::FocusReason GetFocusReason() const override;
bool GetTextRange(gfx::Range* range) const override;
bool GetCompositionTextRange(gfx::Range* range) const override;
bool GetSelectionRange(gfx::Range* range) const override;
bool SetSelectionRange(const gfx::Range& range) override;
bool GetEditableSelectionRange(gfx::Range* range) const override;
bool SetEditableSelectionRange(const gfx::Range& range) override;
bool DeleteRange(const gfx::Range& range) override;
bool GetTextFromRange(const gfx::Range& range,
base::string16* text) const override;
Expand Down
2 changes: 1 addition & 1 deletion ui/base/ime/input_method_auralinux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ void InputMethodAuraLinux::OnCaretBoundsChanged(const TextInputClient* client) {
base::string16 text;
if (client->GetTextRange(&text_range) &&
client->GetTextFromRange(text_range, &text) &&
client->GetSelectionRange(&selection_range)) {
client->GetEditableSelectionRange(&selection_range)) {
context_->SetSurroundingText(text, selection_range);
}

Expand Down
2 changes: 1 addition & 1 deletion ui/base/ime/input_method_auralinux_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ class TextInputClientForTesting : public DummyTextInputClient {
*range = text_range;
return true;
}
bool GetSelectionRange(gfx::Range* range) const override {
bool GetEditableSelectionRange(gfx::Range* range) const override {
*range = selection_range;
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion ui/base/ime/input_method_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ SurroundingTextInfo InputMethodBase::GetSurroundingTextInfo() {
TextInputClient* client = GetTextInputClient();
if (!client->GetTextRange(&text_range) ||
!client->GetTextFromRange(text_range, &info.surrounding_text) ||
!client->GetSelectionRange(&info.selection_range)) {
!client->GetEditableSelectionRange(&info.selection_range)) {
return SurroundingTextInfo();
}
return info;
Expand Down
2 changes: 1 addition & 1 deletion ui/base/ime/input_method_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ void InputMethodChromeOS::OnCaretBoundsChanged(const TextInputClient* client) {
base::string16 surrounding_text;
if (!client->GetTextRange(&text_range) ||
!client->GetTextFromRange(text_range, &surrounding_text) ||
!client->GetSelectionRange(&selection_range)) {
!client->GetEditableSelectionRange(&selection_range)) {
previous_surrounding_text_.clear();
previous_selection_range_ = gfx::Range::InvalidRange();
return;
Expand Down
2 changes: 1 addition & 1 deletion ui/base/ime/input_method_chromeos_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ class InputMethodChromeOSTest : public internal::InputMethodDelegate,
*range = text_range_;
return true;
}
bool GetSelectionRange(gfx::Range* range) const override {
bool GetEditableSelectionRange(gfx::Range* range) const override {
*range = selection_range_;
return true;
}
Expand Down
5 changes: 3 additions & 2 deletions ui/base/ime/input_method_win_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,8 @@ LRESULT InputMethodWinBase::OnDocumentFeed(RECONVERTSTRING* reconv) {
result = client->GetCompositionTextRange(&target_range);

if (!result || target_range.is_empty()) {
if (!client->GetSelectionRange(&target_range) || !target_range.IsValid()) {
if (!client->GetEditableSelectionRange(&target_range) ||
!target_range.IsValid()) {
return 0;
}
}
Expand Down Expand Up @@ -411,7 +412,7 @@ LRESULT InputMethodWinBase::OnReconvertString(RECONVERTSTRING* reconv) {
return 0;

gfx::Range selection_range;
if (!client->GetSelectionRange(&selection_range) ||
if (!client->GetEditableSelectionRange(&selection_range) ||
selection_range.is_empty()) {
return 0;
}
Expand Down
22 changes: 12 additions & 10 deletions ui/base/ime/text_input_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,23 +127,25 @@ class UI_BASE_IME_EXPORT TextInputClient {
// Returns false if the information cannot be retrieved right now.
virtual bool GetCompositionTextRange(gfx::Range* range) const = 0;

// Retrieves the UTF-16 based character range of current selection.
// Returns false if the information cannot be retrieved right now.
virtual bool GetSelectionRange(gfx::Range* range) const = 0;
// Retrieves the UTF-16 based character range of current selection in the text
// input. Returns false if the information cannot be retrieved right now.
// Returns false if the selected text is outside of the text input (== the
// text input is not focused)
virtual bool GetEditableSelectionRange(gfx::Range* range) const = 0;

// Selects the given UTF-16 based character range. Current composition text
// will be confirmed before selecting the range.
// Returns false if the operation is not supported.
virtual bool SetSelectionRange(const gfx::Range& range) = 0;
virtual bool SetEditableSelectionRange(const gfx::Range& range) = 0;

// Deletes contents in the given UTF-16 based character range. Current
// composition text will be confirmed before deleting the range.
// The input caret will be moved to the place where the range gets deleted.
// ExtendSelectionAndDelete should be used instead as far as you are deleting
// characters around current caret. This function with the range based on
// GetSelectionRange has a race condition due to asynchronous IPCs between
// browser and renderer.
// Returns false if the operation is not supported.
// GetEditableSelectionRange has a race condition due to asynchronous IPCs
// between browser and renderer. Returns false if the operation is not
// supported.
virtual bool DeleteRange(const gfx::Range& range) = 0;

// Retrieves the text content in a given UTF-16 based character range.
Expand All @@ -168,9 +170,9 @@ class UI_BASE_IME_EXPORT TextInputClient {

// Deletes the current selection plus the specified number of characters
// before and after the selection or caret. This function should be used
// instead of calling DeleteRange with GetSelectionRange, because
// GetSelectionRange may not be the latest value due to asynchronous of IPC
// between browser and renderer.
// instead of calling DeleteRange with GetEditableSelectionRange, because
// GetEditableSelectionRange may not be the latest value due to asynchronous
// of IPC between browser and renderer.
virtual void ExtendSelectionAndDelete(size_t before, size_t after) = 0;

// Ensure the caret is not in |rect|. |rect| is in screen coordinates in
Expand Down
4 changes: 2 additions & 2 deletions ui/base/ime/win/tsf_text_store_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ class MockTextInputClient : public TextInputClient {
MOCK_METHOD0(ShouldDoLearning, bool());
MOCK_CONST_METHOD1(GetTextRange, bool(gfx::Range*));
MOCK_CONST_METHOD1(GetCompositionTextRange, bool(gfx::Range*));
MOCK_CONST_METHOD1(GetSelectionRange, bool(gfx::Range*));
MOCK_METHOD1(SetSelectionRange, bool(const gfx::Range&));
MOCK_CONST_METHOD1(GetEditableSelectionRange, bool(gfx::Range*));
MOCK_METHOD1(SetEditableSelectionRange, bool(const gfx::Range&));
MOCK_METHOD1(DeleteRange, bool(const gfx::Range&));
MOCK_CONST_METHOD2(GetTextFromRange,
bool(const gfx::Range&, base::string16*));
Expand Down
4 changes: 2 additions & 2 deletions ui/views/cocoa/bridged_native_widget_unittest.mm
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ bool AcceleratorPressed(const ui::Accelerator& accelerator) override {

void BridgedNativeWidgetTest::SetSelectionRange(NSRange range) {
ui::TextInputClient* client = [ns_view_ textInputClient];
client->SetSelectionRange(gfx::Range(range));
client->SetEditableSelectionRange(gfx::Range(range));

[dummy_text_view_ setSelectedRange:range];
}
Expand All @@ -607,7 +607,7 @@ bool AcceleratorPressed(const ui::Accelerator& accelerator) override {

// Although a gfx::Range is directed, the underlying model will not choose an
// affinity until the cursor is moved.
client->SetSelectionRange(range);
client->SetEditableSelectionRange(range);

// Set the range without an affinity. The first @selector sent to the text
// field determines the affinity. Note that Range::ToNSRange() may discard
Expand Down
4 changes: 2 additions & 2 deletions ui/views/controls/prefix_selector.cc
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,12 @@ bool PrefixSelector::GetCompositionTextRange(gfx::Range* range) const {
return false;
}

bool PrefixSelector::GetSelectionRange(gfx::Range* range) const {
bool PrefixSelector::GetEditableSelectionRange(gfx::Range* range) const {
*range = gfx::Range();
return false;
}

bool PrefixSelector::SetSelectionRange(const gfx::Range& range) {
bool PrefixSelector::SetEditableSelectionRange(const gfx::Range& range) {
return false;
}

Expand Down
4 changes: 2 additions & 2 deletions ui/views/controls/prefix_selector.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ class VIEWS_EXPORT PrefixSelector : public ui::TextInputClient {
FocusReason GetFocusReason() const override;
bool GetTextRange(gfx::Range* range) const override;
bool GetCompositionTextRange(gfx::Range* range) const override;
bool GetSelectionRange(gfx::Range* range) const override;
bool SetSelectionRange(const gfx::Range& range) override;
bool GetEditableSelectionRange(gfx::Range* range) const override;
bool SetEditableSelectionRange(const gfx::Range& range) override;
bool DeleteRange(const gfx::Range& range) override;
bool GetTextFromRange(const gfx::Range& range,
base::string16* text) const override;
Expand Down
Loading

0 comments on commit 65eaaf4

Please sign in to comment.