Skip to content

Commit

Permalink
Revert "Fix an edge case bug in Windows TSF1 implementation."
Browse files Browse the repository at this point in the history
This reverts commit 7ab2e8d.

Reason for revert: cause regression in Omnibox with Korean autocomplete feature.

Original change's description:
> Fix an edge case bug in Windows TSF1 implementation.
>
> There is a bug in TSF1 impl where IME such as voice typing can select
> text before composition start when user says "select that" command. In
> this case, we need to select text that is before composition start.
>
> Bug: 1295578
> Change-Id: I873689569b542f8147a31d1b3837cdf543478875
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3449664
> Reviewed-by: Yohei Yukawa <yukawa@chromium.org>
> Commit-Queue: Siye Liu <siliu@microsoft.com>
> Cr-Commit-Position: refs/heads/main@{#971181}

Bug: 1295578
Change-Id: I24e12bd23b10e20beeea4297d9274ed6029998db
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3572984
Reviewed-by: Siye Liu <siliu@microsoft.com>
Reviewed-by: Prudhvikumar Bommana <pbommana@google.com>
Owners-Override: Prudhvikumar Bommana <pbommana@google.com>
Commit-Queue: Srinivas Sista <srinivassista@chromium.org>
Cr-Commit-Position: refs/branch-heads/4986@{#3}
Cr-Branched-From: 248158e-refs/heads/main@{#988910}
  • Loading branch information
siliu1 authored and Chromium LUCI CQ committed Apr 5, 2022
1 parent e8fbf59 commit 49845b0
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 113 deletions.
6 changes: 0 additions & 6 deletions ui/base/ime/win/tsf_text_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1488,12 +1488,6 @@ void TSFTextStore::CommitTextAndEndCompositionIfAny(size_t old_size,
} else {
text_input_client_->ClearCompositionText();
}

if (!selection_.is_empty() && !is_selection_interim_char_ &&
selection_.GetMax() <= string_buffer_document_.size()) {
text_input_client_->SetEditableSelectionRange(selection_);
}

// Notify accessibility about this committed composition
text_input_client_->SetActiveCompositionForAccessibility(
replace_text_range_, new_committed_string,
Expand Down
107 changes: 0 additions & 107 deletions ui/base/ime/win/tsf_text_store_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4231,113 +4231,6 @@ TEST_F(TSFTextStoreTest, RegressionTest13) {
EXPECT_EQ(S_OK, result);
}

// regression tests for crbug.com/1295578.
// Some IMEs (e.g. voice typing panel) may select text before active
// composition. We should select text after composition end.
class RegressionTest14Callback : public TSFTextStoreTestCallback {
public:
explicit RegressionTest14Callback(TSFTextStore* text_store)
: TSFTextStoreTestCallback(text_store) {}

RegressionTest14Callback(const RegressionTest14Callback&) = delete;
RegressionTest14Callback& operator=(const RegressionTest14Callback&) = delete;

HRESULT LockGranted1(DWORD flags) {
SetTextTest(0, 0, L"text ", S_OK);
SetTextTest(4, 5, L"", S_OK);
SetTextTest(4, 4, L" select that ", S_OK);
SetSelectionTest(17, 17, S_OK);

text_spans()->clear();
ImeTextSpan text_span;
text_span.start_offset = 4;
text_span.end_offset = 17;
text_span.underline_color = SK_ColorBLACK;
text_span.thickness = ImeTextSpan::Thickness::kThin;
text_span.background_color = SK_ColorTRANSPARENT;
text_spans()->push_back(text_span);
*edit_flag() = true;
*composition_start() = 4;
composition_range()->set_start(4);
composition_range()->set_end(17);
*has_composition_range() = true;

return S_OK;
}

void SetCompositionText1(const ui::CompositionText& composition) {
EXPECT_EQ(u" select that ", composition.text);
EXPECT_EQ(13u, composition.selection.start());
EXPECT_EQ(13u, composition.selection.end());
ASSERT_EQ(1u, composition.ime_text_spans.size());
EXPECT_EQ(0u, composition.ime_text_spans[0].start_offset);
EXPECT_EQ(13u, composition.ime_text_spans[0].end_offset);
SetHasCompositionText(true);
SetTextRange(0, 17);
SetTextBuffer(u"text select that ");
SetSelectionRange(17, 17);
}

HRESULT LockGranted2(DWORD flags) {
GetTextTest(0, -1, L"text select that ", 17);
SetTextTest(4, 17, L"", S_OK);
GetTextTest(0, -1, L"text", 4);
SetSelectionTest(0, 4, S_OK);

text_spans()->clear();
*edit_flag() = true;
*composition_start() = 0;
composition_range()->set_start(0);
composition_range()->set_end(0);
*has_composition_range() = false;

return S_OK;
}

bool SetEditableSelectionRange2(const gfx::Range& range) {
EXPECT_EQ(range.GetMin(), 0u);
EXPECT_EQ(range.length(), 4u);
return true;
}

HRESULT LockGranted3(DWORD flags) {
GetTextTest(0, -1, L"text", 4);
GetSelectionTest(0, 4);

return S_OK;
}
};

TEST_F(TSFTextStoreTest, RegressionTest14) {
RegressionTest14Callback callback(text_store_.get());
EXPECT_CALL(text_input_client_, InsertText(_, _)).Times(0);
EXPECT_CALL(text_input_client_, SetCompositionText(_))
.WillOnce(
Invoke(&callback, &RegressionTest14Callback::SetCompositionText1));
EXPECT_CALL(text_input_client_, SetEditableSelectionRange(_))
.WillOnce(Invoke(&callback,
&RegressionTest14Callback::SetEditableSelectionRange2));

EXPECT_CALL(*sink_, OnLockGranted(_))
.WillOnce(Invoke(&callback, &RegressionTest14Callback::LockGranted1))
.WillOnce(Invoke(&callback, &RegressionTest14Callback::LockGranted2))
.WillOnce(Invoke(&callback, &RegressionTest14Callback::LockGranted3));

ON_CALL(text_input_client_, HasCompositionText())
.WillByDefault(
Invoke(&callback, &TSFTextStoreTestCallback::HasCompositionText));

HRESULT result = kInvalidResult;
EXPECT_EQ(S_OK, text_store_->RequestLock(TS_LF_READWRITE, &result));
EXPECT_EQ(S_OK, result);
result = kInvalidResult;
EXPECT_EQ(S_OK, text_store_->RequestLock(TS_LF_READWRITE, &result));
EXPECT_EQ(S_OK, result);
result = kInvalidResult;
EXPECT_EQ(S_OK, text_store_->RequestLock(TS_LF_READWRITE, &result));
EXPECT_EQ(S_OK, result);
}

// Test multiple |SetText| call in one edit session.
class MultipleSetTextCallback : public TSFTextStoreTestCallback {
public:
Expand Down

0 comments on commit 49845b0

Please sign in to comment.