From b9ce84214212f5b5072cd9fda458526887c3c739 Mon Sep 17 00:00:00 2001 From: Ken Wakasa Date: Mon, 14 Oct 2013 01:09:40 +0900 Subject: [PATCH] mExpectingUpdateSelection was out of sync when nothing to delete. This bug was leading to corrupted rendering of surrogate pairs in the following scenario. 1. Type some emojis 2. Move the cursor at the beginning of the text field 3. Hit backspace even though there's nothing to delete 4. Move the cursor after some emoji 5. Hit backspace The root cause of this issue was the out-of-sync mExpectingUpdateSelection if handleBackspace() gets called when the cursor reaches at the beginning of the TextView. In such case, mExpectingUpdateSelection shouldn't be set true because there's nothing to delete, so there will be no onUpdateSelection() calls associated with it. Due to this bug, the cache in RichInputConnection could get stale at step 4 described above. Then the following handleBackspace() that should delete a surrogate pair was not working correctly because of the stale cache. bug: 11181913 Change-Id: I9c6a948331726a821bd3ccec9c1d02dec2c4703a --- .../android/inputmethod/latin/LatinIME.java | 34 +++++++++++++------ 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/java/src/com/android/inputmethod/latin/LatinIME.java b/java/src/com/android/inputmethod/latin/LatinIME.java index 8654f3b63..10f643c49 100644 --- a/java/src/com/android/inputmethod/latin/LatinIME.java +++ b/java/src/com/android/inputmethod/latin/LatinIME.java @@ -1622,8 +1622,6 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen case Constants.CODE_DELETE: mSpaceState = SPACE_STATE_NONE; handleBackspace(spaceState); - mDeleteCount++; - mExpectingUpdateSelection = true; LatinImeLogger.logOnDelete(x, y); break; case Constants.CODE_SHIFT: @@ -2051,6 +2049,10 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen } private void handleBackspace(final int spaceState) { + // We revert these in this method if the deletion doesn't happen. + mDeleteCount++; + mExpectingUpdateSelection = true; + // In many cases, we may have to put the keyboard in auto-shift state again. However // we want to wait a few milliseconds before doing it to avoid the keyboard flashing // during key repeat. @@ -2140,8 +2142,16 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen // This should never happen. Log.e(TAG, "Backspace when we don't know the selection position"); } - final int lengthToDelete = Character.isSupplementaryCodePoint( - mConnection.getCodePointBeforeCursor()) ? 2 : 1; + final int codePointBeforeCursor = mConnection.getCodePointBeforeCursor(); + if (codePointBeforeCursor == Constants.NOT_A_CODE) { + // Nothing to delete before the cursor. We have to revert the deletion states + // that were updated at the beginning of this method. + mDeleteCount--; + mExpectingUpdateSelection = false; + return; + } + final int lengthToDelete = + Character.isSupplementaryCodePoint(codePointBeforeCursor) ? 2 : 1; if (mAppWorkAroundsUtils.isBeforeJellyBean() || currentSettings.mInputAttributes.isTypeNull()) { // There are two possible reasons to send a key event: either the field has @@ -2160,12 +2170,16 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen true /* shouldUncommitLogUnit */); } if (mDeleteCount > DELETE_ACCELERATE_AT) { - final int lengthToDeleteAgain = Character.isSupplementaryCodePoint( - mConnection.getCodePointBeforeCursor()) ? 2 : 1; - mConnection.deleteSurroundingText(lengthToDeleteAgain, 0); - if (ProductionFlag.USES_DEVELOPMENT_ONLY_DIAGNOSTICS) { - ResearchLogger.latinIME_handleBackspace(lengthToDeleteAgain, - true /* shouldUncommitLogUnit */); + final int codePointBeforeCursorToDeleteAgain = + mConnection.getCodePointBeforeCursor(); + if (codePointBeforeCursorToDeleteAgain != Constants.NOT_A_CODE) { + final int lengthToDeleteAgain = Character.isSupplementaryCodePoint( + codePointBeforeCursorToDeleteAgain) ? 2 : 1; + mConnection.deleteSurroundingText(lengthToDeleteAgain, 0); + if (ProductionFlag.USES_DEVELOPMENT_ONLY_DIAGNOSTICS) { + ResearchLogger.latinIME_handleBackspace(lengthToDeleteAgain, + true /* shouldUncommitLogUnit */); + } } } }