am 0a51c2b6: Merge "Track selection end in RichInputConnection"

* commit '0a51c2b657e41b794e118137de79d074f45ae4b5':
  Track selection end in RichInputConnection
main
Kurt Partridge 2013-11-15 16:59:50 -08:00 committed by Android Git Automerger
commit 90df70ed4d
2 changed files with 78 additions and 43 deletions

View File

@ -905,7 +905,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
// refresh later. // refresh later.
final boolean canReachInputConnection; final boolean canReachInputConnection;
if (!mConnection.resetCachesUponCursorMoveAndReturnSuccess(editorInfo.initialSelStart, if (!mConnection.resetCachesUponCursorMoveAndReturnSuccess(editorInfo.initialSelStart,
false /* shouldFinishComposition */)) { editorInfo.initialSelEnd, false /* shouldFinishComposition */)) {
// We try resetting the caches up to 5 times before giving up. // We try resetting the caches up to 5 times before giving up.
mHandler.postResetCaches(isDifferentTextField, 5 /* remainingTries */); mHandler.postResetCaches(isDifferentTextField, 5 /* remainingTries */);
// mLastSelection{Start,End} are reset later in this method, don't need to do it here // mLastSelection{Start,End} are reset later in this method, don't need to do it here
@ -1107,7 +1107,8 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
// TODO: revisit this when LatinIME supports hardware keyboards. // TODO: revisit this when LatinIME supports hardware keyboards.
// NOTE: the test harness subclasses LatinIME and overrides isInputViewShown(). // NOTE: the test harness subclasses LatinIME and overrides isInputViewShown().
// TODO: find a better way to simulate actual execution. // TODO: find a better way to simulate actual execution.
if (isInputViewShown() && !mConnection.isBelatedExpectedUpdate(oldSelStart, newSelStart)) { if (isInputViewShown() && !mConnection.isBelatedExpectedUpdate(oldSelStart, newSelStart,
oldSelEnd, newSelEnd)) {
// TODO: the following is probably better done in resetEntireInputState(). // TODO: the following is probably better done in resetEntireInputState().
// it should only happen when the cursor moved, and the very purpose of the // it should only happen when the cursor moved, and the very purpose of the
// test below is to narrow down whether this happened or not. Likewise with // test below is to narrow down whether this happened or not. Likewise with
@ -1133,13 +1134,13 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
// Another option would be to send suggestions each time we set the composing // Another option would be to send suggestions each time we set the composing
// text, but that is probably too expensive to do, so we decided to leave things // text, but that is probably too expensive to do, so we decided to leave things
// as is. // as is.
resetEntireInputState(newSelStart); resetEntireInputState(newSelStart, newSelEnd);
} else { } else {
// resetEntireInputState calls resetCachesUponCursorMove, but with the second // resetEntireInputState calls resetCachesUponCursorMove, but forcing the
// argument as true. But in all cases where we don't reset the entire input state, // composition to end. But in all cases where we don't reset the entire input
// we still want to tell the rich input connection about the new cursor position so // state, we still want to tell the rich input connection about the new cursor
// that it can update its caches. // position so that it can update its caches.
mConnection.resetCachesUponCursorMoveAndReturnSuccess(newSelStart, mConnection.resetCachesUponCursorMoveAndReturnSuccess(newSelStart, newSelEnd,
false /* shouldFinishComposition */); false /* shouldFinishComposition */);
} }
@ -1362,7 +1363,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
// This will reset the whole input state to the starting state. It will clear // This will reset the whole input state to the starting state. It will clear
// the composing word, reset the last composed word, tell the inputconnection about it. // the composing word, reset the last composed word, tell the inputconnection about it.
private void resetEntireInputState(final int newCursorPosition) { private void resetEntireInputState(final int newSelStart, final int newSelEnd) {
final boolean shouldFinishComposition = mWordComposer.isComposingWord(); final boolean shouldFinishComposition = mWordComposer.isComposingWord();
resetComposingState(true /* alsoResetLastComposedWord */); resetComposingState(true /* alsoResetLastComposedWord */);
final SettingsValues settingsValues = mSettings.getCurrent(); final SettingsValues settingsValues = mSettings.getCurrent();
@ -1371,7 +1372,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
} else { } else {
setSuggestedWords(settingsValues.mSuggestPuncList, false); setSuggestedWords(settingsValues.mSuggestPuncList, false);
} }
mConnection.resetCachesUponCursorMoveAndReturnSuccess(newCursorPosition, mConnection.resetCachesUponCursorMoveAndReturnSuccess(newSelStart, newSelEnd,
shouldFinishComposition); shouldFinishComposition);
} }
@ -1714,7 +1715,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) { if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) {
// If we are in the middle of a recorrection, we need to commit the recorrection // If we are in the middle of a recorrection, we need to commit the recorrection
// first so that we can insert the character at the current cursor position. // first so that we can insert the character at the current cursor position.
resetEntireInputState(mLastSelectionStart); resetEntireInputState(mLastSelectionStart, mLastSelectionEnd);
} else { } else {
commitTyped(LastComposedWord.NOT_A_SEPARATOR); commitTyped(LastComposedWord.NOT_A_SEPARATOR);
} }
@ -1782,7 +1783,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) { if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) {
// If we are in the middle of a recorrection, we need to commit the recorrection // If we are in the middle of a recorrection, we need to commit the recorrection
// first so that we can insert the batch input at the current cursor position. // first so that we can insert the batch input at the current cursor position.
resetEntireInputState(mLastSelectionStart); resetEntireInputState(mLastSelectionStart, mLastSelectionEnd);
} else if (wordComposerSize <= 1) { } else if (wordComposerSize <= 1) {
// We auto-correct the previous (typed, not gestured) string iff it's one character // We auto-correct the previous (typed, not gestured) string iff it's one character
// long. The reason for this is, even in the middle of gesture typing, you'll still // long. The reason for this is, even in the middle of gesture typing, you'll still
@ -2070,7 +2071,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) { if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) {
// If we are in the middle of a recorrection, we need to commit the recorrection // If we are in the middle of a recorrection, we need to commit the recorrection
// first so that we can remove the character at the current cursor position. // first so that we can remove the character at the current cursor position.
resetEntireInputState(mLastSelectionStart); resetEntireInputState(mLastSelectionStart, mLastSelectionEnd);
// When we exit this if-clause, mWordComposer.isComposingWord() will return false. // When we exit this if-clause, mWordComposer.isComposingWord() will return false.
} }
if (mWordComposer.isComposingWord()) { if (mWordComposer.isComposingWord()) {
@ -2238,7 +2239,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) { if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) {
// If we are in the middle of a recorrection, we need to commit the recorrection // If we are in the middle of a recorrection, we need to commit the recorrection
// first so that we can insert the character at the current cursor position. // first so that we can insert the character at the current cursor position.
resetEntireInputState(mLastSelectionStart); resetEntireInputState(mLastSelectionStart, mLastSelectionEnd);
isComposingWord = false; isComposingWord = false;
} }
// We want to find out whether to start composing a new word with this character. If so, // We want to find out whether to start composing a new word with this character. If so,
@ -2350,7 +2351,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) { if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) {
// If we are in the middle of a recorrection, we need to commit the recorrection // If we are in the middle of a recorrection, we need to commit the recorrection
// first so that we can insert the separator at the current cursor position. // first so that we can insert the separator at the current cursor position.
resetEntireInputState(mLastSelectionStart); resetEntireInputState(mLastSelectionStart, mLastSelectionEnd);
} }
if (mWordComposer.isComposingWord()) { // May have changed since we stored wasComposing if (mWordComposer.isComposingWord()) { // May have changed since we stored wasComposing
if (currentSettings.mCorrectionEnabled) { if (currentSettings.mCorrectionEnabled) {
@ -2982,7 +2983,8 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
* @param remainingTries How many times we may try again before giving up. * @param remainingTries How many times we may try again before giving up.
*/ */
private void retryResetCaches(final boolean tryResumeSuggestions, final int remainingTries) { private void retryResetCaches(final boolean tryResumeSuggestions, final int remainingTries) {
if (!mConnection.resetCachesUponCursorMoveAndReturnSuccess(mLastSelectionStart, false)) { if (!mConnection.resetCachesUponCursorMoveAndReturnSuccess(mLastSelectionStart,
mLastSelectionEnd, false)) {
if (0 < remainingTries) { if (0 < remainingTries) {
mHandler.postResetCaches(tryResumeSuggestions, remainingTries - 1); mHandler.postResetCaches(tryResumeSuggestions, remainingTries - 1);
return; return;

View File

@ -57,14 +57,19 @@ public final class RichInputConnection {
private static final int INVALID_CURSOR_POSITION = -1; private static final int INVALID_CURSOR_POSITION = -1;
/** /**
* This variable contains an expected value for the cursor position. This is where the * This variable contains an expected value for the selection start position. This is where the
* cursor may end up after all the keyboard-triggered updates have passed. We keep this to * cursor or selection start may end up after all the keyboard-triggered updates have passed. We
* compare it to the actual cursor position to guess whether the move was caused by a * keep this to compare it to the actual selection start to guess whether the move was caused by
* keyboard command or not. * a keyboard command or not.
* It's not really the cursor position: the cursor may not be there yet, and it's also expected * It's not really the selection start position: the selection start may not be there yet, and
* there be cases where it never actually comes to be there. * in some cases, it may never arrive there.
*/ */
private int mExpectedSelStart = INVALID_CURSOR_POSITION; // in chars, not code points private int mExpectedSelStart = INVALID_CURSOR_POSITION; // in chars, not code points
/**
* The expected selection end. Only differs from mExpectedSelStart if a non-empty selection is
* expected. The same caveats as mExpectedSelStart apply.
*/
private int mExpectedSelEnd = INVALID_CURSOR_POSITION; // in chars, not code points
/** /**
* This contains the committed text immediately preceding the cursor and the composing * This contains the committed text immediately preceding the cursor and the composing
* text if any. It is refreshed when the cursor moves by calling upon the TextView. * text if any. It is refreshed when the cursor moves by calling upon the TextView.
@ -150,15 +155,17 @@ public final class RichInputConnection {
* data, so we empty the cache and note that we don't know the new cursor position, and we * data, so we empty the cache and note that we don't know the new cursor position, and we
* return false so that the caller knows about this and can retry later. * return false so that the caller knows about this and can retry later.
* *
* @param newSelStart The new position of the selection start, as received from the system. * @param newSelStart the new position of the selection start, as received from the system.
* @param shouldFinishComposition Whether we should finish the composition in progress. * @param newSelEnd the new position of the selection end, as received from the system.
* @param shouldFinishComposition whether we should finish the composition in progress.
* @return true if we were able to connect to the editor successfully, false otherwise. When * @return true if we were able to connect to the editor successfully, false otherwise. When
* this method returns false, the caches could not be correctly refreshed so they were only * this method returns false, the caches could not be correctly refreshed so they were only
* reset: the caller should try again later to return to normal operation. * reset: the caller should try again later to return to normal operation.
*/ */
public boolean resetCachesUponCursorMoveAndReturnSuccess(final int newSelStart, public boolean resetCachesUponCursorMoveAndReturnSuccess(final int newSelStart,
final boolean shouldFinishComposition) { final int newSelEnd, final boolean shouldFinishComposition) {
mExpectedSelStart = newSelStart; mExpectedSelStart = newSelStart;
mExpectedSelEnd = newSelEnd;
mComposingText.setLength(0); mComposingText.setLength(0);
final boolean didReloadTextSuccessfully = reloadTextCache(); final boolean didReloadTextSuccessfully = reloadTextCache();
if (!didReloadTextSuccessfully) { if (!didReloadTextSuccessfully) {
@ -169,11 +176,13 @@ public final class RichInputConnection {
if (lengthOfTextBeforeCursor > newSelStart if (lengthOfTextBeforeCursor > newSelStart
|| (lengthOfTextBeforeCursor < Constants.EDITOR_CONTENTS_CACHE_SIZE || (lengthOfTextBeforeCursor < Constants.EDITOR_CONTENTS_CACHE_SIZE
&& newSelStart < Constants.EDITOR_CONTENTS_CACHE_SIZE)) { && newSelStart < Constants.EDITOR_CONTENTS_CACHE_SIZE)) {
// newSelStart may be lying -- when rotating the device (probably a framework bug). If // newSelStart and newSelEnd may be lying -- when rotating the device (probably a
// we have less chars than we asked for, then we know how many chars we have, and if we // framework bug). If we have less chars than we asked for, then we know how many chars
// got more than newSelStart says, then we know it was lying. In both cases the length // we have, and if we got more than newSelStart says, then we know it was lying. In both
// is more reliable. // cases the length is more reliable. Note that we only have to check newSelStart (not
// newSelEnd) since if newSelEnd is wrong, the newSelStart will be wrong as well.
mExpectedSelStart = lengthOfTextBeforeCursor; mExpectedSelStart = lengthOfTextBeforeCursor;
mExpectedSelEnd = lengthOfTextBeforeCursor;
} }
if (null != mIC && shouldFinishComposition) { if (null != mIC && shouldFinishComposition) {
mIC.finishComposingText(); mIC.finishComposingText();
@ -200,6 +209,7 @@ public final class RichInputConnection {
// For some reason the app thinks we are not connected to it. This looks like a // For some reason the app thinks we are not connected to it. This looks like a
// framework bug... Fall back to ground state and return false. // framework bug... Fall back to ground state and return false.
mExpectedSelStart = INVALID_CURSOR_POSITION; mExpectedSelStart = INVALID_CURSOR_POSITION;
mExpectedSelEnd = INVALID_CURSOR_POSITION;
Log.e(TAG, "Unable to connect to the editor to retrieve text."); Log.e(TAG, "Unable to connect to the editor to retrieve text.");
return false; return false;
} }
@ -351,8 +361,12 @@ public final class RichInputConnection {
} }
if (mExpectedSelStart > beforeLength) { if (mExpectedSelStart > beforeLength) {
mExpectedSelStart -= beforeLength; mExpectedSelStart -= beforeLength;
mExpectedSelEnd -= beforeLength;
} else { } else {
// There are fewer characters before the cursor in the buffer than we are being asked to
// delete. Only delete what is there.
mExpectedSelStart = 0; mExpectedSelStart = 0;
mExpectedSelEnd -= mExpectedSelStart;
} }
if (null != mIC) { if (null != mIC) {
mIC.deleteSurroundingText(beforeLength, afterLength); mIC.deleteSurroundingText(beforeLength, afterLength);
@ -387,6 +401,7 @@ public final class RichInputConnection {
case KeyEvent.KEYCODE_ENTER: case KeyEvent.KEYCODE_ENTER:
mCommittedTextBeforeComposingText.append("\n"); mCommittedTextBeforeComposingText.append("\n");
mExpectedSelStart += 1; mExpectedSelStart += 1;
mExpectedSelEnd = mExpectedSelStart;
break; break;
case KeyEvent.KEYCODE_DEL: case KeyEvent.KEYCODE_DEL:
if (0 == mComposingText.length()) { if (0 == mComposingText.length()) {
@ -398,18 +413,24 @@ public final class RichInputConnection {
} else { } else {
mComposingText.delete(mComposingText.length() - 1, mComposingText.length()); mComposingText.delete(mComposingText.length() - 1, mComposingText.length());
} }
if (mExpectedSelStart > 0) mExpectedSelStart -= 1; if (mExpectedSelStart > 0 && mExpectedSelStart == mExpectedSelEnd) {
// TODO: Handle surrogate pairs.
mExpectedSelStart -= 1;
}
mExpectedSelEnd = mExpectedSelStart;
break; break;
case KeyEvent.KEYCODE_UNKNOWN: case KeyEvent.KEYCODE_UNKNOWN:
if (null != keyEvent.getCharacters()) { if (null != keyEvent.getCharacters()) {
mCommittedTextBeforeComposingText.append(keyEvent.getCharacters()); mCommittedTextBeforeComposingText.append(keyEvent.getCharacters());
mExpectedSelStart += keyEvent.getCharacters().length(); mExpectedSelStart += keyEvent.getCharacters().length();
mExpectedSelEnd = mExpectedSelStart;
} }
break; break;
default: default:
final String text = new String(new int[] { keyEvent.getUnicodeChar() }, 0, 1); final String text = new String(new int[] { keyEvent.getUnicodeChar() }, 0, 1);
mCommittedTextBeforeComposingText.append(text); mCommittedTextBeforeComposingText.append(text);
mExpectedSelStart += text.length(); mExpectedSelStart += text.length();
mExpectedSelEnd = mExpectedSelStart;
break; break;
} }
} }
@ -444,9 +465,11 @@ public final class RichInputConnection {
if (DEBUG_BATCH_NESTING) checkBatchEdit(); if (DEBUG_BATCH_NESTING) checkBatchEdit();
if (DEBUG_PREVIOUS_TEXT) checkConsistencyForDebug(); if (DEBUG_PREVIOUS_TEXT) checkConsistencyForDebug();
mExpectedSelStart += text.length() - mComposingText.length(); mExpectedSelStart += text.length() - mComposingText.length();
mExpectedSelEnd = mExpectedSelStart;
mComposingText.setLength(0); mComposingText.setLength(0);
mComposingText.append(text); mComposingText.append(text);
// TODO: support values of i != 1. At this time, this is never called with i != 1. // TODO: support values of newCursorPosition != 1. At this time, this is never called with
// newCursorPosition != 1.
if (null != mIC) { if (null != mIC) {
mIC.setComposingText(text, newCursorPosition); mIC.setComposingText(text, newCursorPosition);
if (ProductionFlag.USES_DEVELOPMENT_ONLY_DIAGNOSTICS) { if (ProductionFlag.USES_DEVELOPMENT_ONLY_DIAGNOSTICS) {
@ -782,20 +805,30 @@ public final class RichInputConnection {
* this update and not the ones in-between. This is almost impossible to achieve even trying * this update and not the ones in-between. This is almost impossible to achieve even trying
* very very hard. * very very hard.
* *
* @param oldSelStart The value of the old cursor position in the update. * @param oldSelStart The value of the old selection in the update.
* @param newSelStart The value of the new cursor position in the update. * @param newSelStart The value of the new selection in the update.
* @param oldSelEnd The value of the old selection end in the update.
* @param newSelEnd The value of the new selection end in the update.
* @return whether this is a belated expected update or not. * @return whether this is a belated expected update or not.
*/ */
public boolean isBelatedExpectedUpdate(final int oldSelStart, final int newSelStart) { public boolean isBelatedExpectedUpdate(final int oldSelStart, final int newSelStart,
// If this is an update that arrives at our expected position, it's a belated update. final int oldSelEnd, final int newSelEnd) {
if (newSelStart == mExpectedSelStart) return true; // This update is "belated" if we are expecting it. That is, mExpectedSelStart and
// If this is an update that moves the cursor from our expected position, it must be // mExpectedSelEnd match the new values that the TextView is updating TO.
// an explicit move. if (mExpectedSelStart == newSelStart && mExpectedSelEnd == newSelEnd) return true;
if (oldSelStart == mExpectedSelStart) return false; // This update is not belated if mExpectedSelStart and mExpeectedSelend match the old
// The following returns true if newSelStart is between oldSelStart and // values, and one of newSelStart or newSelEnd is updated to a different value. In this
// mCurrentCursorPosition. We assume that if the updated position is between the old // case, there is likely something other than the IME that has moved the selection endpoint
// position and the expected position, then it must be a belated update. // to the new value.
return (newSelStart - oldSelStart) * (mExpectedSelStart - newSelStart) >= 0; if (mExpectedSelStart == oldSelStart && mExpectedSelEnd == oldSelEnd
&& (oldSelStart != newSelStart || oldSelEnd != newSelEnd)) return false;
// If nether of the above two cases holds, then the system may be having trouble keeping up
// with updates. If 1) the selection is a cursor, 2) newSelStart is between oldSelStart
// and mExpectedSelStart, and 3) newSelEnd is between oldSelEnd and mExpectedSelEnd, then
// assume a belated update.
return (newSelStart == newSelEnd)
&& (newSelStart - oldSelStart) * (mExpectedSelStart - newSelStart) >= 0
&& (newSelEnd - oldSelEnd) * (mExpectedSelEnd - newSelEnd) >= 0;
} }
/** /**