Skip to content

Commit

Permalink
chore: cherry-pick c20afc96e36f57 from chromium
Browse files Browse the repository at this point in the history
  • Loading branch information
deepak1556 committed Jun 11, 2020
1 parent a33c10c commit 1f4d796
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 0 deletions.
1 change: 1 addition & 0 deletions patches/chromium/.patches
Expand Up @@ -100,3 +100,4 @@ disable_dcheck_that_fails_with_software_compositing.patch
fix_swap_global_proxies_before_initializing_the_windows_proxies.patch
fix_default_to_ntlm_v2_in_network_service.patch
feat_add_streaming-protocol_registry_to_multibuffer_data_source.patch
allow_ime_to_insert_zero_length_composition_string.patch
@@ -0,0 +1,142 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Siye Liu <siliu@microsoft.com>
Date: Mon, 8 Jun 2020 21:19:00 +0000
Subject: Allow IME to insert zero-length composition string.

Some IMEs and Windows OS dictation will either insert zero-length text
or delete existing composition text to commit active composition.
Therefore, we should allow IMEs to cancel composition by committing zero
length text.

Bug: 1091069
Change-Id: I99cb213dc2ba1965abfa88ccf6858b89bd7e82df
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2233234
Commit-Queue: Siye Liu <siliu@microsoft.com>
Reviewed-by: Yohei Yukawa <yukawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#776198}

diff --git a/ui/base/ime/win/tsf_text_store.cc b/ui/base/ime/win/tsf_text_store.cc
index 22f43ff72b0d6a390f1b4961aa6e23ed3117b732..0c00758a8cda57232cac64144ca8a207bc9eb7b1 100644
--- a/ui/base/ime/win/tsf_text_store.cc
+++ b/ui/base/ime/win/tsf_text_store.cc
@@ -647,9 +647,7 @@ HRESULT TSFTextStore::RequestLock(DWORD lock_flags, HRESULT* result) {
// 3. User commits current composition text.
if (((new_composition_start > last_composition_start &&
text_input_client_->HasCompositionText()) ||
- (wparam_keydown_fired_ == 0 && !has_composition_range_ &&
- !text_input_client_->HasCompositionText()) ||
- (wparam_keydown_fired_ != 0 && !has_composition_range_)) &&
+ !has_composition_range_) &&
text_input_client_) {
CommitTextAndEndCompositionIfAny(last_composition_start,
new_composition_start);
@@ -1355,8 +1353,11 @@ void TSFTextStore::CommitTextAndEndCompositionIfAny(size_t old_size,
: new_committed_string_size);
// TODO(crbug.com/978678): Unify the behavior of
// |TextInputClient::InsertText(text)| for the empty text.
- if (!new_committed_string.empty())
+ if (!new_committed_string.empty()) {
text_input_client_->InsertText(new_committed_string);
+ } else {
+ text_input_client_->ClearCompositionText();
+ }
// Notify accessibility about this committed composition
text_input_client_->SetActiveCompositionForAccessibility(
replace_text_range_, new_committed_string,
diff --git a/ui/base/ime/win/tsf_text_store_unittest.cc b/ui/base/ime/win/tsf_text_store_unittest.cc
index 081809d75a1fbbedbcbc722302d4d915eabc46d4..4e81d7c1012f86712f1bba2bfe4037ed1ef79772 100644
--- a/ui/base/ime/win/tsf_text_store_unittest.cc
+++ b/ui/base/ime/win/tsf_text_store_unittest.cc
@@ -3394,5 +3394,92 @@ TEST_F(TSFTextStoreTest, RegressionTest7) {
EXPECT_EQ(S_OK, result);
}

+// regression tests for crbug.com/1091069.
+// We should allow inserting empty compositon string to cancel composition.
+class RegressionTest8Callback : public TSFTextStoreTestCallback {
+ public:
+ explicit RegressionTest8Callback(TSFTextStore* text_store)
+ : TSFTextStoreTestCallback(text_store) {}
+
+ HRESULT LockGranted1(DWORD flags) {
+ SetTextTest(0, 0, L"bbbb", S_OK);
+ SetSelectionTest(0, 4, S_OK);
+
+ text_spans()->clear();
+ ImeTextSpan text_span_1;
+ text_span_1.start_offset = 0;
+ text_span_1.end_offset = 4;
+ text_span_1.underline_color = SK_ColorBLACK;
+ text_span_1.thickness = ImeTextSpan::Thickness::kThin;
+ text_span_1.background_color = SK_ColorTRANSPARENT;
+ text_spans()->push_back(text_span_1);
+
+ *edit_flag() = true;
+ *composition_start() = 0;
+ composition_range()->set_start(0);
+ composition_range()->set_end(4);
+ *has_composition_range() = true;
+
+ text_store_->OnKeyTraceDown(65u, 1966081u);
+ return S_OK;
+ }
+
+ void SetCompositionText1(const ui::CompositionText& composition) {
+ EXPECT_EQ(L"bbbb", composition.text);
+ EXPECT_EQ(0u, composition.selection.start());
+ EXPECT_EQ(4u, composition.selection.end());
+ ASSERT_EQ(1u, composition.ime_text_spans.size());
+ EXPECT_EQ(0u, composition.ime_text_spans[0].start_offset);
+ EXPECT_EQ(4u, composition.ime_text_spans[0].end_offset);
+ SetHasCompositionText(true);
+ }
+
+ HRESULT LockGranted2(DWORD flags) {
+ GetTextTest(0, -1, L"bbbb", 4);
+ SetTextTest(0, 4, L"", 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;
+ text_store_->OnKeyTraceUp(65u, 1966081u);
+ return S_OK;
+ }
+
+ void ClearCompositionText2() { EXPECT_EQ(false, *has_composition_range()); }
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(RegressionTest8Callback);
+};
+
+TEST_F(TSFTextStoreTest, RegressionTest8) {
+ RegressionTest8Callback callback(text_store_.get());
+ EXPECT_CALL(text_input_client_, SetCompositionText(_))
+ .WillOnce(
+ Invoke(&callback, &RegressionTest8Callback::SetCompositionText1));
+
+ EXPECT_CALL(text_input_client_, ClearCompositionText())
+ .WillOnce(
+ Invoke(&callback, &RegressionTest8Callback::ClearCompositionText2));
+
+ EXPECT_CALL(*sink_, OnLockGranted(_))
+ .WillOnce(Invoke(&callback, &RegressionTest8Callback::LockGranted1))
+ .WillOnce(Invoke(&callback, &RegressionTest8Callback::LockGranted2));
+
+ 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);
+}
+
} // namespace
} // namespace ui

0 comments on commit 1f4d796

Please sign in to comment.