From 7b9f09c1e32c9d64069d463e2c6332a5e42d3d61 Mon Sep 17 00:00:00 2001 From: Changwan Ryu Date: Fri, 8 Sep 2017 00:03:13 +0000 Subject: [PATCH] Fix for Japanese keyboards Japanese keyboards do not finish composition when we restore the deleted text. Then some keyboards call setComposingText([text], 1) again which moves the cursor before the restored text, and some keyboards do not respond at all and therefore later composition change moves the cursor before the restored text. It seems that all the keyboard apps I tested calmly respond to the finishComposingText() call, probably because it gets the call in various situations such as focus loss. BUG=758443 (cherry picked from commit 08ed69a327a50f7712b348ba9bf6df466f4fce5d) Change-Id: I752d9d86053a923d7ffc4ef248d088ca991aeae1 Reviewed-on: https://chromium-review.googlesource.com/636107 Commit-Queue: Changwan Ryu Reviewed-by: Ted Choc Reviewed-by: Alexandre Elias Cr-Original-Commit-Position: refs/heads/master@{#500122} Reviewed-on: https://chromium-review.googlesource.com/656515 Reviewed-by: Changwan Ryu Cr-Commit-Position: refs/branch-heads/3202@{#74} Cr-Branched-From: fa6a5d87adff761bc16afc5498c3f5944c1daa68-refs/heads/master@{#499098} --- .../SpannableAutocompleteEditTextModel.java | 5 +++++ .../browser/omnibox/AutocompleteEditTextTest.java | 14 +++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/chrome/android/java/src/org/chromium/chrome/browser/omnibox/SpannableAutocompleteEditTextModel.java b/chrome/android/java/src/org/chromium/chrome/browser/omnibox/SpannableAutocompleteEditTextModel.java index d66f102f8b33..404309619e0b 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/omnibox/SpannableAutocompleteEditTextModel.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/omnibox/SpannableAutocompleteEditTextModel.java @@ -515,6 +515,11 @@ private void restoreBackspacedText(String diff) { Editable editable = mDelegate.getEditableText(); editable.append(diff); decrementBatchEditCount(); + if (mBatchEditNestCount == 0) { // only at the outermost batch edit + // crbug.com/758443: Japanese keyboard does not finish composition when we restore + // the deleted text. + super.finishComposingText(); + } } private boolean setAutocompleteSpan() { diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextTest.java index 747de3243ef9..57aa813f54e8 100644 --- a/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextTest.java +++ b/chrome/android/junit/src/org/chromium/chrome/browser/omnibox/AutocompleteEditTextTest.java @@ -17,6 +17,7 @@ import android.util.AttributeSet; import android.view.KeyEvent; import android.view.accessibility.AccessibilityEvent; +import android.view.inputmethod.BaseInputConnection; import android.view.inputmethod.EditorInfo; import android.view.inputmethod.InputConnection; import android.widget.LinearLayout; @@ -744,7 +745,7 @@ private void internalTestDelete_CommitText() { if (isUsingSpannableModel()) { // Pretend that we have deleted 'o' first. mInOrder.verify(mVerifier).onUpdateSelection(4, 4); - // We restore 'o', and clears autocomplete text instead. + // We restore 'o', and clear autocomplete text instead. mInOrder.verify(mVerifier).onUpdateSelection(5, 5); assertTrue(mAutocomplete.isCursorVisible()); // Autocomplete removed. @@ -778,12 +779,18 @@ private void internalTestDelete_CommitText() { assertTexts("hello", ""); } + private boolean isComposing() { + return BaseInputConnection.getComposingSpanStart(mAutocomplete.getText()) + != BaseInputConnection.getComposingSpanEnd(mAutocomplete.getText()); + } + @Test @Features(@Features.Register( value = ChromeFeatureList.SPANNABLE_INLINE_AUTOCOMPLETE, enabled = true)) public void testDelete_SetComposingTextWithSpannableModel() { // User types "hello". assertTrue(mInputConnection.setComposingText("hello", 1)); + assertTrue(isComposing()); mInOrder.verify(mVerifier).onUpdateSelection(5, 5); verifyOnPopulateAccessibilityEvent( AccessibilityEvent.TYPE_VIEW_TEXT_CHANGED, "hello", "", -1, 0, -1, 0, 5); @@ -806,9 +813,10 @@ public void testDelete_SetComposingTextWithSpannableModel() { assertTrue(mInputConnection.setComposingText("hell", 1)); // Pretend that we have deleted 'o'. mInOrder.verify(mVerifier).onUpdateSelection(4, 4); - // We restore 'o', and clear autocomplete text instead. + // We restore 'o', finish composition, and clear autocomplete text instead. mInOrder.verify(mVerifier).onUpdateSelection(5, 5); assertTrue(mAutocomplete.isCursorVisible()); + assertFalse(isComposing()); // Remove autocomplete. verifyOnPopulateAccessibilityEvent( AccessibilityEvent.TYPE_VIEW_TEXT_CHANGED, "hello", "hello world", -1, 5, -1, 6, 0); @@ -1167,4 +1175,4 @@ private void internalTestFocusInAndSelectAll() { } mInOrder.verifyNoMoreInteractions(); } -} \ No newline at end of file +}