From 843b8c44f737262d3e9cabbdbe904dd8d1c5b42d Mon Sep 17 00:00:00 2001 From: Helium314 Date: Tue, 11 Jun 2024 22:02:32 +0200 Subject: [PATCH] replace old fix from 9713c4a25a4a25784a018a991ffbf6ddcdd8927a with improved consistency check and avoid somewhat unexpected crash --- .../keyboard/latin/RichInputConnection.java | 107 ++++++++---------- .../keyboard/latin/inputlogic/InputLogic.java | 7 +- 2 files changed, 48 insertions(+), 66 deletions(-) diff --git a/app/src/main/java/helium314/keyboard/latin/RichInputConnection.java b/app/src/main/java/helium314/keyboard/latin/RichInputConnection.java index 3c83028a..2cd1e0b4 100644 --- a/app/src/main/java/helium314/keyboard/latin/RichInputConnection.java +++ b/app/src/main/java/helium314/keyboard/latin/RichInputConnection.java @@ -449,12 +449,13 @@ public final class RichInputConnection implements PrivateCommandPerformer { final long startTime = SystemClock.uptimeMillis(); final CharSequence result = mIC.getTextBeforeCursor(n, flags); detectLaggyConnection(operation, timeout, startTime); - if (result != null && result.length() > 1 && (mComposingText.length() > 0 || mCommittedTextBeforeComposingText.length() > 0)) { - final char actualLastChar = result.charAt(result.length() - 1); - final char cachedLastChar = mComposingText.length() > 0 - ? mComposingText.charAt(mComposingText.length() - 1) - : mCommittedTextBeforeComposingText.charAt(mCommittedTextBeforeComposingText.length() - 1); - if (actualLastChar != cachedLastChar) { + // inconsistent state can occur for (at least) two reasons + // 1. the app actively changes text field content, e.g. joplin when deleting "list markers like 2. + // 2. the app has outdated contents in the text field, e.g. notepad (com.farmerbb.notepad) returns the + // just deleted char right after deletion, instead of the correct one + // todo: understand where this inconsistent state comes from, is it really the other app's fault, or is it HeliBoard? + if (result != null) { + if (!checkTextBeforeCursorConsistency(result)) { Log.w(TAG, "cached text out of sync, reloading"); ExtractedTextRequest r = new ExtractedTextRequest(); final ExtractedText et = mIC.getExtractedText(r, 0); @@ -467,6 +468,41 @@ public final class RichInputConnection implements PrivateCommandPerformer { return result; } + // checks whether the end of cached text before cursor is the same as end of the given CharSequence + // this is done to find inconsistencies that arise in some text fields when characters are deleted, but also in some other cases + // only checks the end for performance reasons + // may need to check more than just the last character, because the text may end in e.g. rrrrr and even there a single car offset should be found + private boolean checkTextBeforeCursorConsistency(final CharSequence textField) { + final int lastIndex = textField.length() - 1; + if (lastIndex == -1) return true; + final char lastChar = textField.charAt(lastIndex); + final int composingLength = mComposingText.length(); + for (int i = 0; i <= lastIndex; i++) { + // get last minus i character and compare + final char currentTextFieldChar = textField.charAt(lastIndex - i); + final char currentCachedChar; + if (i < composingLength) { + // take char from composing text + currentCachedChar = mComposingText.charAt(composingLength - 1 - i); + } else { + // take last char from mCommittedTextBeforeComposingText, consider composing length + final int index = mCommittedTextBeforeComposingText.length() - 1 - (i - composingLength); + if (index < mCommittedTextBeforeComposingText.length() && index >= 0) + currentCachedChar = mCommittedTextBeforeComposingText.charAt(index); + else return lastIndex > 100; // still let it pass if the same character is repeated many times, but cached text too short + } + + if (currentTextFieldChar != currentCachedChar) + // different character -> inconsistent + return false; + + if (lastChar != currentTextFieldChar) + // not the same, and no inconsistency found so far -> unlikely there is one that won't be found later -> return early + return true; + } + return true; // no inconsistency found after going through everything + } + @Nullable public CharSequence getTextAfterCursor(final int n, final int flags) { return getTextAfterCursorAndDetectLaggyConnection( OPERATION_GET_TEXT_AFTER_CURSOR, @@ -595,6 +631,7 @@ public final class RichInputConnection implements PrivateCommandPerformer { public void setComposingRegion(final int start, final int end) { if (DEBUG_BATCH_NESTING) checkBatchEdit(); if (DEBUG_PREVIOUS_TEXT) checkConsistencyForDebug(); + final int moveBy = mExpectedSelStart - start; // determine now, as mExpectedSelStart may change in getTextBeforeCursor final CharSequence textBeforeCursor = getTextBeforeCursor(Constants.EDITOR_CONTENTS_CACHE_SIZE + (end - start), 0); mCommittedTextBeforeComposingText.setLength(0); @@ -603,8 +640,7 @@ public final class RichInputConnection implements PrivateCommandPerformer { // position in mExpectedSelStart and mExpectedSelEnd. In this case we want the start // of the text, so we should use mExpectedSelStart. In other words, the composing // text starts (mExpectedSelStart - start) characters before the end of textBeforeCursor - final int indexOfStartOfComposingText = - Math.max(textBeforeCursor.length() - (mExpectedSelStart - start), 0); + final int indexOfStartOfComposingText = Math.max(textBeforeCursor.length() - moveBy, 0); mComposingText.append(textBeforeCursor.subSequence(indexOfStartOfComposingText, textBeforeCursor.length())); mCommittedTextBeforeComposingText.append( @@ -666,7 +702,7 @@ public final class RichInputConnection implements PrivateCommandPerformer { public void selectWord(final SpacingAndPunctuations spacingAndPunctuations, final String script) { if (!isConnected()) return; if (mExpectedSelStart != mExpectedSelEnd) return; // already something selected - final TextRange range = getWordRangeAtCursor(spacingAndPunctuations, script, false); + final TextRange range = getWordRangeAtCursor(spacingAndPunctuations, script); if (range == null) return; mIC.setSelection(mExpectedSelStart - range.getNumberOfCharsInWordBeforeCursor(), mExpectedSelStart + range.getNumberOfCharsInWordAfterCursor()); } @@ -766,7 +802,7 @@ public final class RichInputConnection implements PrivateCommandPerformer { * @return a range containing the text surrounding the cursor */ @Nullable public TextRange getWordRangeAtCursor(final SpacingAndPunctuations spacingAndPunctuations, - final String script, final boolean justDeleted) { + final String script) { mIC = mParent.getCurrentInputConnection(); if (!isConnected()) { return null; @@ -785,26 +821,6 @@ public final class RichInputConnection implements PrivateCommandPerformer { return null; } - // we need text before, and text after is either empty or a separator or similar - if (justDeleted && before.length() > 0 && - (after.length() == 0 - || !isPartOfCompositionForScript(Character.codePointAt(after, 0), spacingAndPunctuations, script) - ) - ) { - // issue: - // type 2 words and space, press delete twice -> remaining word and space before are selected - // now on next key press, the space before the word is removed - // or complete a word by choosing a suggestion, then press backspace -> same thing - // what is sometimes happening (depending on app, or maybe input field attributes): - // we just pressed delete, and getTextBeforeCursor gets the correct text, - // but getTextBeforeCursorAndDetectLaggyConnection returns the old word, before the deletion (not sure why) - // -> we try to detect this difference, and then try to fix it - // interestingly, getTextBeforeCursor seems to only get the correct text because it uses - // mCommittedTextBeforeComposingText, where the text is cached - // what could be actually going on? we probably need to fetch the text, because we want updated styles (if any) - before = fixIncorrectLength(before); - } - // Going backward, find the first breaking point (separator) int startIndexInBefore = before.length(); int endIndexInAfter = -1; @@ -890,37 +906,6 @@ public final class RichInputConnection implements PrivateCommandPerformer { hasUrlSpans); } - // mostly fixes an issue where the space before the word is selected after deleting a codepoint, - // because the text length is not yet updated in the field (i.e. trying to select "word length" - // before cursor, but the last letter has just been deleted and thus the space before is also selected) - private CharSequence fixIncorrectLength(final CharSequence before) { - // don't use codepoints, just do the simple thing... - int initialCheckLength = Math.min(3, before.length()); - // this should have been checked before calling this method, but better be safe - if (initialCheckLength == 0) return before; - final CharSequence lastCharsInBefore = before.subSequence(before.length() - initialCheckLength, before.length()); - final CharSequence lastCharsBeforeCursor = getTextBeforeCursor(initialCheckLength, 0); - // if the last 3 chars are equal, we can be relatively sure to not have this bug (can still be e.g. rrrr, which is not detected) - // (we could also check everything though, it's just a little slower) - if (TextUtils.equals(lastCharsInBefore, lastCharsBeforeCursor)) return before; - - // delete will hopefully have deleted a codepoint, not only a char - // we want to compare whether the text before the cursor is the same as "before" without - // the last codepoint. if yes, return "before" without the last codepoint - final int lastBeforeCodePoint = Character.codePointBefore(before, before.length()); - int lastBeforeLength = Character.charCount(lastBeforeCodePoint); - final CharSequence codePointBeforeCursor = getTextBeforeCursor(lastBeforeLength, 0); - if (codePointBeforeCursor == null || codePointBeforeCursor.length() == 0) return before; - - // now check whether they are the same if the last codepoint of before is removed - final CharSequence beforeWithoutLast = before.subSequence(0, before.length() - lastBeforeLength); - final CharSequence beforeCursor = getTextBeforeCursor(beforeWithoutLast.length(), 0); - if (beforeCursor == null || beforeCursor.length() != beforeWithoutLast.length()) return before; - if (TextUtils.equals(beforeCursor, beforeWithoutLast)) - return beforeWithoutLast; - return before; - } - public boolean isCursorTouchingWord(final SpacingAndPunctuations spacingAndPunctuations, boolean checkTextAfter) { if (checkTextAfter && isCursorFollowedByWordCharacter(spacingAndPunctuations)) { diff --git a/app/src/main/java/helium314/keyboard/latin/inputlogic/InputLogic.java b/app/src/main/java/helium314/keyboard/latin/inputlogic/InputLogic.java index e844e12d..c813223f 100644 --- a/app/src/main/java/helium314/keyboard/latin/inputlogic/InputLogic.java +++ b/app/src/main/java/helium314/keyboard/latin/inputlogic/InputLogic.java @@ -1353,9 +1353,7 @@ public final class InputLogic { if (!mConnection.hasSelection() && settingsValues.isSuggestionsEnabledPerUserSettings() && settingsValues.mSpacingAndPunctuations.mCurrentLanguageHasSpaces) { - final TextRange range = mConnection.getWordRangeAtCursor( - settingsValues.mSpacingAndPunctuations, - currentKeyboardScript, false); + final TextRange range = mConnection.getWordRangeAtCursor(settingsValues.mSpacingAndPunctuations, currentKeyboardScript); if (range != null) { return range.mWord.toString(); } @@ -1709,8 +1707,7 @@ public final class InputLogic { mConnection.finishComposingText(); return; } - final TextRange range = - mConnection.getWordRangeAtCursor(settingsValues.mSpacingAndPunctuations, currentKeyboardScript, true); + final TextRange range = mConnection.getWordRangeAtCursor(settingsValues.mSpacingAndPunctuations, currentKeyboardScript); if (null == range) return; // Happens if we don't have an input connection at all if (range.length() <= 0) { // Race condition, or touching a word in a non-supported script.