Fix a bug on rotation with selection.

The symptom : when text is selected and the device is rotated,
sometimes the keyboard sets the word as being composed around
the start of the selection. Upon the next rotation this ends up
with the keyboard committing some text in place of the selection.

The cause : another bug in the framework with rotation >.>
The keyboard receives a call to startInput with a wrong cursor
position, namely one that does not represent a selection. The
keyboard sets a composition according to this wrong data. When
the keyboard is rotated again, it commits the text, which takes
the place of the selection.

The solution : actually when restarting input the keyboard
realizes that the cursor position is wrong. We cancel composition
at that time.
For robustness, this change also implements two other defensive
changes : upon call to onUpdateSelection, we actually realize
that the previous values were wrong, so we also fix it at that
time, and in addition, when rotating, we finishComposingText()
instead of commitText() which is less dangerous. Implementing
this later change also allows us to let less internal variables
from InputLogic escape to LatinIME, so it's also a good change
for design.

Bug: 14140799

Change-Id: Ib10de18e53e376ac1bbc8487e13d969828483346
This commit is contained in:
Jean Chalard 2014-06-05 17:48:10 +09:00
parent 9d4d61f9c1
commit 9fd9a68d87
3 changed files with 18 additions and 8 deletions

View file

@ -640,14 +640,10 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
@Override @Override
public void onConfigurationChanged(final Configuration conf) { public void onConfigurationChanged(final Configuration conf) {
// If orientation changed while predicting, commit the change
final SettingsValues settingsValues = mSettings.getCurrent(); final SettingsValues settingsValues = mSettings.getCurrent();
if (settingsValues.mDisplayOrientation != conf.orientation) { if (settingsValues.mDisplayOrientation != conf.orientation) {
mHandler.startOrientationChanging(); mHandler.startOrientationChanging();
mInputLogic.mConnection.beginBatchEdit(); mInputLogic.finishInput();
mInputLogic.commitTyped(mSettings.getCurrent(), LastComposedWord.NOT_A_SEPARATOR);
mInputLogic.mConnection.finishComposingText();
mInputLogic.mConnection.endBatchEdit();
} }
PersonalizationDictionarySessionRegistrar.onConfigurationChanged(this, conf, PersonalizationDictionarySessionRegistrar.onConfigurationChanged(this, conf,
mDictionaryFacilitator); mDictionaryFacilitator);

View file

@ -891,4 +891,8 @@ public final class RichInputConnection {
public boolean hasSelection() { public boolean hasSelection() {
return mExpectedSelEnd != mExpectedSelStart; return mExpectedSelEnd != mExpectedSelStart;
} }
public boolean isCursorPositionKnown() {
return INVALID_CURSOR_POSITION != mExpectedSelStart;
}
} }

View file

@ -319,8 +319,16 @@ public final class InputLogic {
|| !mWordComposer.isComposingWord(); // safe to reset || !mWordComposer.isComposingWord(); // safe to reset
final boolean hasOrHadSelection = (oldSelStart != oldSelEnd || newSelStart != newSelEnd); final boolean hasOrHadSelection = (oldSelStart != oldSelEnd || newSelStart != newSelEnd);
final int moveAmount = newSelStart - oldSelStart; final int moveAmount = newSelStart - oldSelStart;
if (selectionChangedOrSafeToReset && (hasOrHadSelection // As an added small gift from the framework, it happens upon rotation when there
|| !mWordComposer.moveCursorByAndReturnIfInsideComposingWord(moveAmount))) { // is a selection that we get a wrong cursor position delivered to startInput() that
// does not get reflected in the oldSel{Start,End} parameters to the next call to
// onUpdateSelection. In this case, we may have set a composition, and when we're here
// we realize we shouldn't have. In theory, in this case, selectionChangedOrSafeToReset
// should be true, but that is if the framework had taken that wrong cursor position
// into account, which means we have to reset the entire composing state whenever there
// is or was a selection regardless of whether it changed or not.
if (hasOrHadSelection || (selectionChangedOrSafeToReset
&& !mWordComposer.moveCursorByAndReturnIfInsideComposingWord(moveAmount))) {
// If we are composing a word and moving the cursor, we would want to set a // If we are composing a word and moving the cursor, we would want to set a
// suggestion span for recorrection to work correctly. Unfortunately, that // suggestion span for recorrection to work correctly. Unfortunately, that
// would involve the keyboard committing some new text, which would move the // would involve the keyboard committing some new text, which would move the
@ -1922,9 +1930,11 @@ public final class InputLogic {
final boolean tryResumeSuggestions, final int remainingTries, final boolean tryResumeSuggestions, final int remainingTries,
// TODO: remove these arguments // TODO: remove these arguments
final LatinIME.UIHandler handler) { final LatinIME.UIHandler handler) {
final boolean shouldFinishComposition = mConnection.hasSelection()
|| !mConnection.isCursorPositionKnown();
if (!mConnection.resetCachesUponCursorMoveAndReturnSuccess( if (!mConnection.resetCachesUponCursorMoveAndReturnSuccess(
mConnection.getExpectedSelectionStart(), mConnection.getExpectedSelectionEnd(), mConnection.getExpectedSelectionStart(), mConnection.getExpectedSelectionEnd(),
false /* shouldFinishComposition */)) { shouldFinishComposition)) {
if (0 < remainingTries) { if (0 < remainingTries) {
handler.postResetCaches(tryResumeSuggestions, remainingTries - 1); handler.postResetCaches(tryResumeSuggestions, remainingTries - 1);
return false; return false;