diff options
author | kochi@chromium.org <kochi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-19 16:37:55 +0000 |
---|---|---|
committer | kochi@chromium.org <kochi@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-19 16:37:55 +0000 |
commit | fa38ff5974ebde07719493e555d9272477c43ee3 (patch) | |
tree | 5f2f00c4130c80520d2620f17f0a64336697b47e /content | |
parent | 883890d0f96edadc23031fd9ffec141bcc73c89b (diff) | |
download | chromium_src-fa38ff5974ebde07719493e555d9272477c43ee3.zip chromium_src-fa38ff5974ebde07719493e555d9272477c43ee3.tar.gz chromium_src-fa38ff5974ebde07719493e555d9272477c43ee3.tar.bz2 |
Restore editor text upon AdapterInputConnection is recreated.
As Editable is created in BaseInputConnection which is the
parent of AdapterInputConnection and no one owns it,
once a new AdapterInputConnection is created (e.g. when user
switches keyboard), the old AdapterInputConnection object
gets lost along with the Editable object and losts internal
Editor state (especially the text content).
Still Blink holds the text content, but upon such events as
keyboard switch, Blink doesn't update the Editor and there will
be inconsistency between what is held in Blink and what Android
keyboard can see in the Editable.
Hold the previous Editable and restore the text content
upon a new AdapterInputConnection is created.
BUG=344683
TEST=ContentShellTest -f ContentCoreInputConnectionTest.*
Review URL: https://codereview.chromium.org/172273002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@257998 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'content')
6 files changed, 162 insertions, 79 deletions
diff --git a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java index a174cd3..8965192 100644 --- a/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java +++ b/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java @@ -26,6 +26,7 @@ import android.os.SystemClock; import android.provider.Browser; import android.provider.Settings; import android.text.Editable; +import android.text.Selection; import android.text.TextUtils; import android.util.Log; import android.view.ActionMode; @@ -419,6 +420,12 @@ public class ContentViewCore private SmartClipDataListener mSmartClipDataListener = null; + // This holds the state of editable text (e.g. contents of <input>, contenteditable) of + // a focused element. + // Every time the user, IME, javascript (Blink), autofill etc. modifies the content, the new + // state must be reflected to this to keep consistency. + private Editable mEditable; + /** * PID used to indicate an invalid render process. */ @@ -453,6 +460,9 @@ public class ContentViewCore getContext().getSystemService(Context.ACCESSIBILITY_SERVICE); mGestureStateListeners = new ObserverList<GestureStateListener>(); mGestureStateListenersIterator = mGestureStateListeners.rewindableIterator(); + + mEditable = Editable.Factory.getInstance().newEditable(""); + Selection.setSelection(mEditable, 0); } /** @@ -585,6 +595,11 @@ public class ContentViewCore return mInputConnection; } + @VisibleForTesting + public void setContainerViewForTest(ViewGroup view) { + mContainerView = view; + } + private ImeAdapter createImeAdapter(Context context) { return new ImeAdapter(mInputMethodManagerWrapper, new ImeAdapter.ImeAdapterDelegate() { @@ -1499,13 +1514,19 @@ public class ContentViewCore // enter fullscreen mode. outAttrs.imeOptions = EditorInfo.IME_FLAG_NO_FULLSCREEN; } - mInputConnection = - mAdapterInputConnectionFactory.get(mContainerView, mImeAdapter, outAttrs); + mInputConnection = mAdapterInputConnectionFactory.get(mContainerView, mImeAdapter, + mEditable, outAttrs); return mInputConnection; } + @VisibleForTesting + public AdapterInputConnection getAdapterInputConnectionForTest() { + return mInputConnection; + } + + @VisibleForTesting public Editable getEditableForTest() { - return mInputConnection.getEditable(); + return mEditable; } /** @@ -1524,9 +1545,7 @@ public class ContentViewCore if (newConfig.keyboard != Configuration.KEYBOARD_NOKEYS) { mImeAdapter.attach(nativeGetNativeImeAdapter(mNativeContentViewCore), - ImeAdapter.getTextInputTypeNone(), - AdapterInputConnection.INVALID_SELECTION, - AdapterInputConnection.INVALID_SELECTION); + ImeAdapter.getTextInputTypeNone()); mInputMethodManagerWrapper.restartInput(mContainerView); } mContainerViewInternals.super_onConfigurationChanged(newConfig); @@ -2351,8 +2370,7 @@ public class ContentViewCore if (mActionMode != null) mActionMode.invalidate(); - mImeAdapter.attachAndShowIfNeeded(nativeImeAdapterAndroid, textInputType, - selectionStart, selectionEnd, showImeIfNeeded); + mImeAdapter.attachAndShowIfNeeded(nativeImeAdapterAndroid, textInputType, showImeIfNeeded); if (mInputConnection != null) { mInputConnection.updateState(text, selectionStart, selectionEnd, compositionStart, diff --git a/content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java b/content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java index 95f2bbf..dcc08fa 100644 --- a/content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java +++ b/content/public/android/java/src/org/chromium/content/browser/input/AdapterInputConnection.java @@ -34,6 +34,7 @@ public class AdapterInputConnection extends BaseInputConnection { private final View mInternalView; private final ImeAdapter mImeAdapter; + private final Editable mEditable; private boolean mSingleLine; private int mNumNestedBatchEdits = 0; @@ -44,11 +45,17 @@ public class AdapterInputConnection extends BaseInputConnection { private int mLastUpdateCompositionEnd = INVALID_COMPOSITION; @VisibleForTesting - AdapterInputConnection(View view, ImeAdapter imeAdapter, EditorInfo outAttrs) { + AdapterInputConnection(View view, ImeAdapter imeAdapter, Editable editable, + EditorInfo outAttrs) { super(view, true); mInternalView = view; mImeAdapter = imeAdapter; mImeAdapter.setInputConnection(this); + mEditable = editable; + // The editable passed in might have been in use by a prior keyboard and could have had + // prior composition spans set. To avoid keyboard conflicts, remove all composing spans + // when taking ownership of an existing Editable. + removeComposingSpans(mEditable); mSingleLine = true; outAttrs.imeOptions = EditorInfo.IME_FLAG_NO_FULLSCREEN | EditorInfo.IME_FLAG_NO_EXTRACT_UI; @@ -97,10 +104,13 @@ public class AdapterInputConnection extends BaseInputConnection { | InputType.TYPE_NUMBER_VARIATION_NORMAL; outAttrs.imeOptions |= EditorInfo.IME_ACTION_NEXT; } - outAttrs.initialSelStart = imeAdapter.getInitialSelectionStart(); - outAttrs.initialSelEnd = imeAdapter.getInitialSelectionEnd(); - mLastUpdateSelectionStart = imeAdapter.getInitialSelectionStart(); - mLastUpdateSelectionEnd = imeAdapter.getInitialSelectionEnd(); + outAttrs.initialSelStart = Selection.getSelectionStart(mEditable); + outAttrs.initialSelEnd = Selection.getSelectionEnd(mEditable); + mLastUpdateSelectionStart = Selection.getSelectionStart(mEditable); + mLastUpdateSelectionEnd = Selection.getSelectionEnd(mEditable); + + Selection.setSelection(mEditable, outAttrs.initialSelStart, outAttrs.initialSelEnd); + updateSelectionIfRequired(); } /** @@ -120,6 +130,7 @@ public class AdapterInputConnection extends BaseInputConnection { * selection. * @param requireAck True when the update was not caused by IME, false otherwise. */ + @VisibleForTesting public void updateState(String text, int selectionStart, int selectionEnd, int compositionStart, int compositionEnd, boolean requireAck) { if (DEBUG) { @@ -136,18 +147,17 @@ public class AdapterInputConnection extends BaseInputConnection { compositionStart = Math.min(compositionStart, text.length()); compositionEnd = Math.min(compositionEnd, text.length()); - Editable editable = getEditable(); - String prevText = editable.toString(); + String prevText = mEditable.toString(); boolean textUnchanged = prevText.equals(text); if (!textUnchanged) { - editable.replace(0, editable.length(), text); + mEditable.replace(0, mEditable.length(), text); } - Selection.setSelection(editable, selectionStart, selectionEnd); + Selection.setSelection(mEditable, selectionStart, selectionEnd); if (compositionStart == compositionEnd) { - removeComposingSpans(editable); + removeComposingSpans(mEditable); } else { super.setComposingRegion(compositionStart, compositionEnd); } @@ -155,16 +165,23 @@ public class AdapterInputConnection extends BaseInputConnection { } /** + * @return Editable object which contains the state of current focused editable element. + */ + @Override + public Editable getEditable() { + return mEditable; + } + + /** * Sends selection update to the InputMethodManager unless we are currently in a batch edit or * if the exact same selection and composition update was sent already. */ private void updateSelectionIfRequired() { if (mNumNestedBatchEdits != 0) return; - Editable editable = getEditable(); - int selectionStart = Selection.getSelectionStart(editable); - int selectionEnd = Selection.getSelectionEnd(editable); - int compositionStart = getComposingSpanStart(editable); - int compositionEnd = getComposingSpanEnd(editable); + int selectionStart = Selection.getSelectionStart(mEditable); + int selectionEnd = Selection.getSelectionEnd(mEditable); + int compositionStart = getComposingSpanStart(mEditable); + int compositionEnd = getComposingSpanEnd(mEditable); // Avoid sending update if we sent an exact update already previously. if (mLastUpdateSelectionStart == selectionStart && mLastUpdateSelectionEnd == selectionEnd && @@ -258,11 +275,10 @@ public class AdapterInputConnection extends BaseInputConnection { public ExtractedText getExtractedText(ExtractedTextRequest request, int flags) { if (DEBUG) Log.w(TAG, "getExtractedText"); ExtractedText et = new ExtractedText(); - Editable editable = getEditable(); - et.text = editable.toString(); - et.partialEndOffset = editable.length(); - et.selectionStart = Selection.getSelectionStart(editable); - et.selectionEnd = Selection.getSelectionEnd(editable); + et.text = mEditable.toString(); + et.partialEndOffset = mEditable.length(); + et.selectionStart = Selection.getSelectionStart(mEditable); + et.selectionEnd = Selection.getSelectionEnd(mEditable); et.flags = mSingleLine ? ExtractedText.FLAG_SINGLE_LINE : 0; return et; } @@ -297,9 +313,8 @@ public class AdapterInputConnection extends BaseInputConnection { if (DEBUG) { Log.w(TAG, "deleteSurroundingText [" + beforeLength + " " + afterLength + "]"); } - Editable editable = getEditable(); - int availableBefore = Selection.getSelectionStart(editable); - int availableAfter = editable.length() - Selection.getSelectionEnd(editable); + int availableBefore = Selection.getSelectionStart(mEditable); + int availableAfter = mEditable.length() - Selection.getSelectionEnd(mEditable); beforeLength = Math.min(beforeLength, availableBefore); afterLength = Math.min(afterLength, availableAfter); super.deleteSurroundingText(beforeLength, afterLength); @@ -328,15 +343,14 @@ public class AdapterInputConnection extends BaseInputConnection { } else { int unicodeChar = event.getUnicodeChar(); if (unicodeChar != 0) { - Editable editable = getEditable(); - int selectionStart = Selection.getSelectionStart(editable); - int selectionEnd = Selection.getSelectionEnd(editable); + int selectionStart = Selection.getSelectionStart(mEditable); + int selectionEnd = Selection.getSelectionEnd(mEditable); if (selectionStart > selectionEnd) { int temp = selectionStart; selectionStart = selectionEnd; selectionEnd = temp; } - editable.replace(selectionStart, selectionEnd, + mEditable.replace(selectionStart, selectionEnd, Character.toString((char) unicodeChar)); } } @@ -364,8 +378,7 @@ public class AdapterInputConnection extends BaseInputConnection { @Override public boolean finishComposingText() { if (DEBUG) Log.w(TAG, "finishComposingText"); - Editable editable = getEditable(); - if (getComposingSpanStart(editable) == getComposingSpanEnd(editable)) { + if (getComposingSpanStart(mEditable) == getComposingSpanEnd(mEditable)) { return true; } @@ -382,7 +395,7 @@ public class AdapterInputConnection extends BaseInputConnection { @Override public boolean setSelection(int start, int end) { if (DEBUG) Log.w(TAG, "setSelection [" + start + " " + end + "]"); - int textLength = getEditable().length(); + int textLength = mEditable.length(); if (start < 0 || end < 0 || start > textLength || end > textLength) return true; super.setSelection(start, end); updateSelectionIfRequired(); @@ -405,7 +418,7 @@ public class AdapterInputConnection extends BaseInputConnection { @Override public boolean setComposingRegion(int start, int end) { if (DEBUG) Log.w(TAG, "setComposingRegion [" + start + " " + end + "]"); - int textLength = getEditable().length(); + int textLength = mEditable.length(); int a = Math.min(start, end); int b = Math.max(start, end); if (a < 0) a = 0; @@ -414,7 +427,7 @@ public class AdapterInputConnection extends BaseInputConnection { if (b > textLength) b = textLength; if (a == b) { - removeComposingSpans(getEditable()); + removeComposingSpans(mEditable); } else { super.setComposingRegion(a, b); } @@ -450,12 +463,11 @@ public class AdapterInputConnection extends BaseInputConnection { @VisibleForTesting ImeState getImeStateForTesting() { - Editable editable = getEditable(); - String text = editable.toString(); - int selectionStart = Selection.getSelectionStart(editable); - int selectionEnd = Selection.getSelectionEnd(editable); - int compositionStart = getComposingSpanStart(editable); - int compositionEnd = getComposingSpanEnd(editable); + String text = mEditable.toString(); + int selectionStart = Selection.getSelectionStart(mEditable); + int selectionEnd = Selection.getSelectionEnd(mEditable); + int compositionStart = getComposingSpanStart(mEditable); + int compositionEnd = getComposingSpanEnd(mEditable); return new ImeState(text, selectionStart, selectionEnd, compositionStart, compositionEnd); } } diff --git a/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java b/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java index 826d80e..2ce35f8 100644 --- a/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java +++ b/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java @@ -7,6 +7,7 @@ package org.chromium.content.browser.input; import android.os.Handler; import android.os.ResultReceiver; import android.os.SystemClock; +import android.text.Editable; import android.view.KeyCharacterMap; import android.view.KeyEvent; import android.view.View; @@ -63,8 +64,7 @@ public class ImeAdapter { @Override public void run() { - attach(mNativeImeAdapter, sTextInputTypeNone, AdapterInputConnection.INVALID_SELECTION, - AdapterInputConnection.INVALID_SELECTION); + attach(mNativeImeAdapter, sTextInputTypeNone); dismissInput(true); } } @@ -106,8 +106,6 @@ public class ImeAdapter { private final Handler mHandler; private DelayedDismissInput mDismissInput = null; private int mTextInputType; - private int mInitialSelectionStart; - private int mInitialSelectionEnd; @VisibleForTesting boolean mIsShowWithoutHideOutstanding = false; @@ -127,8 +125,9 @@ public class ImeAdapter { * Default factory for AdapterInputConnection classes. */ public static class AdapterInputConnectionFactory { - public AdapterInputConnection get(View view, ImeAdapter imeAdapter, EditorInfo outAttrs) { - return new AdapterInputConnection(view, imeAdapter, outAttrs); + public AdapterInputConnection get(View view, ImeAdapter imeAdapter, + Editable editable, EditorInfo outAttrs) { + return new AdapterInputConnection(view, imeAdapter, editable, outAttrs); } } @@ -162,22 +161,6 @@ public class ImeAdapter { return mTextInputType; } - /** - * Should be only used by AdapterInputConnection. - * @return The starting index of the initial text selection. - */ - int getInitialSelectionStart() { - return mInitialSelectionStart; - } - - /** - * Should be only used by AdapterInputConnection. - * @return The ending index of the initial text selection. - */ - int getInitialSelectionEnd() { - return mInitialSelectionEnd; - } - public static int getTextInputTypeNone() { return sTextInputTypeNone; } @@ -212,7 +195,7 @@ public class ImeAdapter { } public void attachAndShowIfNeeded(long nativeImeAdapter, int textInputType, - int selectionStart, int selectionEnd, boolean showIfNeeded) { + boolean showIfNeeded) { mHandler.removeCallbacks(mDismissInput); // If current input type is none and showIfNeeded is false, IME should not be shown @@ -230,7 +213,7 @@ public class ImeAdapter { return; } - attach(nativeImeAdapter, textInputType, selectionStart, selectionEnd); + attach(nativeImeAdapter, textInputType); mInputMethodManagerWrapper.restartInput(mViewEmbedder.getAttachedView()); if (showIfNeeded) { @@ -241,15 +224,12 @@ public class ImeAdapter { } } - public void attach(long nativeImeAdapter, int textInputType, int selectionStart, - int selectionEnd) { + public void attach(long nativeImeAdapter, int textInputType) { if (mNativeImeAdapterAndroid != 0) { nativeResetImeAdapter(mNativeImeAdapterAndroid); } mNativeImeAdapterAndroid = nativeImeAdapter; mTextInputType = textInputType; - mInitialSelectionStart = selectionStart; - mInitialSelectionEnd = selectionEnd; if (nativeImeAdapter != 0) { nativeAttachImeAdapter(mNativeImeAdapterAndroid); } diff --git a/content/public/android/javatests/src/org/chromium/content/browser/ContentViewCoreInputConnectionTest.java b/content/public/android/javatests/src/org/chromium/content/browser/ContentViewCoreInputConnectionTest.java new file mode 100644 index 0000000..8ef23b1 --- /dev/null +++ b/content/public/android/javatests/src/org/chromium/content/browser/ContentViewCoreInputConnectionTest.java @@ -0,0 +1,68 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +package org.chromium.content.browser; + +import android.test.suitebuilder.annotation.SmallTest; +import android.view.inputmethod.EditorInfo; +import android.view.inputmethod.InputConnection; + +import org.chromium.content.browser.input.AdapterInputConnection; +import org.chromium.content.browser.input.ImeAdapter; +import org.chromium.content.browser.input.InputMethodManagerWrapper; +import org.chromium.content.browser.test.util.TestInputMethodManagerWrapper; +import org.chromium.content_shell_apk.ContentShellTestBase; + +/** + * Tests that when InputConnection is recreated, the text is still retained. + */ +public class ContentViewCoreInputConnectionTest extends ContentShellTestBase { + private ContentViewCore mContentViewCore; + private TestImeAdapter mImeAdapter; + private TestInputMethodManagerWrapper mInputMethodManagerWrapper; + + private static class TestImeAdapter extends ImeAdapter { + public TestImeAdapter(InputMethodManagerWrapper immw) { + super(immw, null); + } + @Override + public boolean hasTextInputType() { + return true; + } + } + + public void setUp() throws Exception { + super.setUp(); + mContentViewCore = new ContentViewCore(getActivity()); + mInputMethodManagerWrapper = new TestInputMethodManagerWrapper(mContentViewCore); + mImeAdapter = new TestImeAdapter(mInputMethodManagerWrapper); + mImeAdapter.setInputMethodManagerWrapper(new TestInputMethodManagerWrapper( + mContentViewCore)); + mContentViewCore.setImeAdapterForTest(mImeAdapter); + mContentViewCore.setContainerViewForTest(getActivity().getActiveContentView()); + } + + /** + * When creating a new InputConnection (e.g. after switching software keyboard), make sure the + * text content in the Editable is not lost. + */ + @SmallTest + public void testRecreateInputConnection() throws Exception { + EditorInfo info = new EditorInfo(); + + InputConnection inputConnection = mContentViewCore.onCreateInputConnection(info); + AdapterInputConnection adapter = mContentViewCore.getAdapterInputConnectionForTest(); + adapter.updateState("Is this text restored?", 0, 0, 0, 0, true); + + String text = mContentViewCore.getEditableForTest().toString(); + assertEquals("Check if the initial text is stored.", "Is this text restored?", text); + + // Create a new InputConnection. + EditorInfo info2 = new EditorInfo(); + inputConnection = mContentViewCore.onCreateInputConnection(info2); + + String newtext = mContentViewCore.getEditableForTest().toString(); + assertEquals("Check if the string is restored.", "Is this text restored?", newtext); + } +} diff --git a/content/public/android/javatests/src/org/chromium/content/browser/input/AdapterInputConnectionTest.java b/content/public/android/javatests/src/org/chromium/content/browser/input/AdapterInputConnectionTest.java index 9dcce92..a0b5346 100644 --- a/content/public/android/javatests/src/org/chromium/content/browser/input/AdapterInputConnectionTest.java +++ b/content/public/android/javatests/src/org/chromium/content/browser/input/AdapterInputConnectionTest.java @@ -8,6 +8,7 @@ import android.content.Context; import android.os.IBinder; import android.os.ResultReceiver; import android.test.suitebuilder.annotation.MediumTest; +import android.text.Editable; import android.view.View; import android.view.inputmethod.EditorInfo; @@ -25,6 +26,7 @@ public class AdapterInputConnectionTest extends ContentShellTestBase { private AdapterInputConnection mConnection; private TestInputMethodManagerWrapper mWrapper; + private Editable mEditable; @Override public void setUp() throws Exception { @@ -35,8 +37,9 @@ public class AdapterInputConnectionTest extends ContentShellTestBase { ImeAdapterDelegate delegate = new TestImeAdapterDelegate(); ImeAdapter imeAdapter = new TestImeAdapter(mWrapper, delegate); EditorInfo info = new EditorInfo(); + mEditable = Editable.Factory.getInstance().newEditable(""); mConnection = new AdapterInputConnection( - getActivity().getActiveContentView(), imeAdapter, info); + getActivity().getActiveContentView(), imeAdapter, mEditable, info); } @MediumTest diff --git a/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java b/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java index da5bd3d..c00b83b 100644 --- a/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java +++ b/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java @@ -10,6 +10,7 @@ import android.content.ClipboardManager; import android.content.Context; import android.test.suitebuilder.annotation.MediumTest; import android.test.suitebuilder.annotation.SmallTest; +import android.text.Editable; import android.text.TextUtils; import android.view.KeyEvent; import android.view.View; @@ -451,16 +452,17 @@ public class ImeTest extends ContentShellTestBase { ImeAdapter.AdapterInputConnectionFactory { @Override public AdapterInputConnection get(View view, ImeAdapter imeAdapter, - EditorInfo outAttrs) { - return new TestAdapterInputConnection(view, imeAdapter, outAttrs); + Editable editable, EditorInfo outAttrs) { + return new TestAdapterInputConnection(view, imeAdapter, editable, outAttrs); } } private static class TestAdapterInputConnection extends AdapterInputConnection { private final ArrayList<TestImeState> mImeUpdateQueue = new ArrayList<TestImeState>(); - public TestAdapterInputConnection(View view, ImeAdapter imeAdapter, EditorInfo outAttrs) { - super(view, imeAdapter, outAttrs); + public TestAdapterInputConnection(View view, ImeAdapter imeAdapter, + Editable editable, EditorInfo outAttrs) { + super(view, imeAdapter, editable, outAttrs); } @Override |