diff --git a/android/KMEA/app/src/main/java/com/tavultesoft/kmea/KMManager.java b/android/KMEA/app/src/main/java/com/tavultesoft/kmea/KMManager.java index c1a3da4d8c..355c8a04f0 100644 --- a/android/KMEA/app/src/main/java/com/tavultesoft/kmea/KMManager.java +++ b/android/KMEA/app/src/main/java/com/tavultesoft/kmea/KMManager.java @@ -60,6 +60,7 @@ import com.tavultesoft.kmea.packages.JSONUtils; import com.tavultesoft.kmea.packages.LexicalModelPackageProcessor; import com.tavultesoft.kmea.packages.PackageProcessor; import com.tavultesoft.kmea.KMScanCodeMap; +import com.tavultesoft.kmea.util.CharSequenceUtil; import com.tavultesoft.kmea.util.FileUtils; import org.json.JSONArray; @@ -2034,17 +2035,8 @@ public final class KMManager { } // Perform left-deletions - for (int i = 0; i < dn; i++) { - CharSequence chars = ic.getTextBeforeCursor(1, 0); - if (chars != null && chars.length() > 0) { - char c = chars.charAt(0); - SystemKeyboardShouldIgnoreSelectionChange = true; - if (Character.isLowSurrogate(c)) { - ic.deleteSurroundingText(2, 0); - } else { - ic.deleteSurroundingText(1, 0); - } - } + if (dn > 0) { + performLeftDeletions(ic, dn); } // Perform right-deletions @@ -2063,7 +2055,9 @@ public final class KMManager { if (s.length() > 0) { SystemKeyboardShouldIgnoreSelectionChange = true; - ic.commitText(s, s.length()); + + // Commit the string s. Use newCursorPosition 1 so cursor will end up after the string. + ic.commitText(s, 1); } ic.endBatchEdit(); @@ -2075,5 +2069,75 @@ public final class KMManager { IMService.getCurrentInputConnection().sendKeyEvent(new KeyEvent(KeyEvent.ACTION_DOWN, keyEventCode)); IMService.getCurrentInputConnection().sendKeyEvent(new KeyEvent(KeyEvent.ACTION_UP, keyEventCode)); } + + /* + // TODO: Chromium has a bug where deleteSurroundingText deletes an entire grapheme cluster + // instead of one code-point. See Chromium issue #1024738 + // https://bugs.chromium.org/p/chromium/issues/detail?id=1024738 + // + // We'll retrieve up to (dn*2+16) characters before the cursor to collect enough characters + // for surrogate pairs + a long grapheme cluster. + // This buffer will be used to put back characters as-needed + */ + private static void performLeftDeletions(InputConnection ic, int dn) { + int originalBufferLength = dn*2 + 16; // characters + CharSequence charsBackup = getCharacterSequence(ic, originalBufferLength); + + int lastIndex = charsBackup.length()-1; + + // Exit if there's no context to delete + if (lastIndex < 0) { + return; + } + + int numPairs = CharSequenceUtil.countSurrogatePairs(charsBackup, dn); + + // Chop dn+numPairs code points from the end of charsBackup + // subSequence indices are start(inclusive) to end(exclusive) + CharSequence expectedChars = charsBackup.subSequence(0, charsBackup.length() - (dn + numPairs)); + ic.deleteSurroundingText(dn + numPairs, 0); + CharSequence newContext = getCharacterSequence(ic, originalBufferLength - dn); + + CharSequence charsToRestore = CharSequenceUtil.restoreChars(expectedChars, newContext); + if (charsToRestore.length() > 0) { + // Restore expectedChars that Chromium deleted. + // Use newCusorPosition 1 so cursor will be after the inserted string + ic.commitText(charsToRestore, 1); + } + } + + /** + * Get a character sequence from the InputConnection. + * Sometimes the WebView can split a surrogate pair at either end, + * so chop that and update the cursor + * @param ic - the InputConnection + * @param length - number of characters to get + * @return CharSequence + */ + private static CharSequence getCharacterSequence(InputConnection ic, int length) { + if (ic == null || length <= 0) { + return ""; + } + + CharSequence sequence = ic.getTextBeforeCursor(length, 0); + if (sequence.length() <= 0) { + return ""; + } + + // Move the cursor back if there's a split surrogate pair + if (Character.isHighSurrogate(sequence.charAt(sequence.length()-1))) { + String origChars = sequence.toString(); + ic.commitText("", -1); + sequence = ic.getTextBeforeCursor(length, 0); + } + + if (Character.isLowSurrogate(sequence.charAt(0))) { + // Adjust if the first char is also a split surrogate pair + // subSequence indices are start(inclusive) to end(exclusive) + sequence = sequence.subSequence(1, sequence.length()); + } + + return sequence; + } } -} \ No newline at end of file +} diff --git a/android/KMEA/app/src/main/java/com/tavultesoft/kmea/util/CharSequenceUtil.java b/android/KMEA/app/src/main/java/com/tavultesoft/kmea/util/CharSequenceUtil.java new file mode 100644 index 0000000000..717ffbdc97 --- /dev/null +++ b/android/KMEA/app/src/main/java/com/tavultesoft/kmea/util/CharSequenceUtil.java @@ -0,0 +1,94 @@ +package com.tavultesoft.kmea.util; + +import android.util.Log; + +import java.lang.CharSequence; + +public final class CharSequenceUtil { + private final static String TAG = "CharSeqUtil"; + + /** + * Count the number of surrogate pairs starting from the end of a character sequence + * until we reach dn codepoints or searched the entire sequence. + * Doing this the hard way because we can't foreach charSequence in reverse. + * @param sequence - the character sequence to analyze + * @param dn - the number of code points to count up to + * @return int + */ + public static int countSurrogatePairs(CharSequence sequence, int dn) { + if ((sequence == null) || (sequence.length() == 0) || (dn <= 0)) { + return 0; + } + + int numPairs = 0, counter = 0; + int lastIndex = sequence.length()-1; + try { + do { + if (Character.isLowSurrogate(sequence.charAt(lastIndex - counter))) { + numPairs++; + } + counter++; + } while ((lastIndex - counter >= 0) && (counter < dn) && (numPairs < dn)); + return numPairs; + } catch (Exception e) { + Log.e(TAG, "Error in countSurrogatePairs: " + e); + return numPairs; + } + } + + /** + * Determine a character sequence that needs to be re-inserted. If currentContext is a + * subSequence of expectedChars, returns the character sequence that needs to be restored. + * @param expectedChars - expected character sequence + * @param currentContext - current character sequence + * @return CharSequence - Character sequence that will need to be appended to currentContext. + * Empty string if no characters need to be restored. + */ + public static CharSequence restoreChars(CharSequence expectedChars, CharSequence currentContext) { + CharSequence charsToRestore = ""; + + try { + // Now see if we need to re-insert characters + if (expectedChars.length() != currentContext.length()) { + String expectedCharsString = expectedChars.toString(); + String currentContextString = currentContext.toString(); + int index = expectedCharsString.indexOf(currentContextString); + if (index > -1) { + // subSequence indices are start(inclusive) to end(exclusive) + charsToRestore = expectedChars.subSequence(index + currentContextString.length(), expectedChars.length()); + } + } + return charsToRestore; + } catch (Exception e) { + Log.e(TAG, "Error in restoreChars: " + e); + return charsToRestore; + } + } + + /** + * Adjust cursor position after insert the new text. + * @param charsBefore the character sequence of the inputconnection (2*s.length) + * @param s the inserted text + * @return int - number of characters to adjust the cursor + */ + public static int adjustCursorPosition(CharSequence charsBefore, String s) { + if (charsBefore == null || s == null) { + return 0; + } + + int numPairs = countSurrogatePairs(charsBefore, s.length()); + int _expected_start_index = charsBefore.length() - s.length(); + int _move = 0; + while (_move < _expected_start_index) { + + CharSequence _check = charsBefore.subSequence( + _expected_start_index - _move,charsBefore.length()-_move); + if(_check.equals(s)) { + break; + } + _move++; + } + return _move; + } + +} diff --git a/android/KMEA/app/src/test/java/com/tavultesoft/kmea/util/CharSequenceUtilTest.java b/android/KMEA/app/src/test/java/com/tavultesoft/kmea/util/CharSequenceUtilTest.java new file mode 100644 index 0000000000..3dd2e69b31 --- /dev/null +++ b/android/KMEA/app/src/test/java/com/tavultesoft/kmea/util/CharSequenceUtilTest.java @@ -0,0 +1,204 @@ +package com.tavultesoft.kmea.util; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; + +import com.tavultesoft.kmea.util.CharSequenceUtil; + +@RunWith(RobolectricTestRunner.class) +public class CharSequenceUtilTest { + // Smiley emoji U+1F600 = D800 DC3D + private final String SMILEY_HIGH_SURROGATE_PAIR = "\uD800"; + private final String SMILEY_LOW_SURROGATE_PAIR = "\uDC3D"; + private final String SMILEY = SMILEY_HIGH_SURROGATE_PAIR + SMILEY_LOW_SURROGATE_PAIR; + + // Winking emoji U+1F609 = D800 DC3C + private final String WINK = "\uD800\uDC3C"; + + private final String COMPOSING_DOT_ABOVE = "\u0307"; + private final String COMPOSING_CIRCUMFLEX_ACCENT = "\u0302"; + + private final String P_COMPOSING_DOT_ABOVE = "p" + COMPOSING_DOT_ABOVE; + private final String P_COMPOSING_CIRCUMFLEX_ACCENT = "p" + COMPOSING_CIRCUMFLEX_ACCENT; + + //region countSurrogatePairs tests + + @Test + public void test_countSurrogatePairs_invalid_input() { + + // Test invalid input doesn't throw exception + + // null sequence + CharSequence sequence = null; + int numPairs = CharSequenceUtil.countSurrogatePairs(sequence, 1); + Assert.assertEquals(0, numPairs); + + // negative dn + sequence = SMILEY + " Day"; + numPairs = CharSequenceUtil.countSurrogatePairs(sequence, -1); + Assert.assertEquals(0, numPairs); + + // dn longer than the sequence length + numPairs = CharSequenceUtil.countSurrogatePairs(sequence, sequence.length()+5); + Assert.assertEquals(1, numPairs); + } + + @Test + public void test_countSurrogatePairs_zero_pairs() { + + // Test for scenarios that expect 0 surrogate pairs + CharSequence sequence = "Zero emojis"; + int numPairs = CharSequenceUtil.countSurrogatePairs(sequence, sequence.length()); + Assert.assertEquals(0, numPairs); + + // dn doesn't reach the surrogate pair + sequence = "Zero emojis" + SMILEY + "!"; + numPairs = CharSequenceUtil.countSurrogatePairs(sequence, 0); + Assert.assertEquals(0, numPairs); + + numPairs = CharSequenceUtil.countSurrogatePairs(sequence, 1); + Assert.assertEquals(0, numPairs); + } + + @Test + public void test_countSurrogatePairs_one_pair() { + + // Test for scenarios that expect 1 surrogate pair + CharSequence sequence = SMILEY_LOW_SURROGATE_PAIR; + int numPairs = CharSequenceUtil.countSurrogatePairs(sequence, 1); + Assert.assertEquals(1, numPairs); + + sequence = "Have A " + SMILEY; + numPairs = CharSequenceUtil.countSurrogatePairs(sequence, 1); + Assert.assertEquals(1, numPairs); + + numPairs = CharSequenceUtil.countSurrogatePairs(sequence, 2); + Assert.assertEquals(1, numPairs); + + numPairs = CharSequenceUtil.countSurrogatePairs(sequence, 6); + Assert.assertEquals(1, numPairs); + + sequence = "Have A " + SMILEY + WINK + " Day"; + numPairs = CharSequenceUtil.countSurrogatePairs(sequence, 5); + Assert.assertEquals(1, numPairs); + + numPairs = CharSequenceUtil.countSurrogatePairs(sequence, 6); + Assert.assertEquals(1, numPairs); + } + + @Test + public void test_countSurrogatePairs_two_pairs() { + + // Test for scenarios that expect 2 surrogate pairs + CharSequence sequence = "Have A " + SMILEY + WINK + " Day"; + int numPairs = CharSequenceUtil.countSurrogatePairs(sequence, 7); + Assert.assertEquals(2, numPairs); + + numPairs = CharSequenceUtil.countSurrogatePairs(sequence, 8); + Assert.assertEquals(2, numPairs); + } + + //endregion + + // region restoreChars tests + + @Test + public void test_restoreChars_invalid_input() { + // Test invalid input doesn't throw exception + // null sequence + CharSequence expectedChars = null; + CharSequence currentContext = "qwerty"; + CharSequence charsToRestore = CharSequenceUtil.restoreChars(expectedChars, currentContext); + Assert.assertEquals("", charsToRestore); + + expectedChars = "qwerty"; + currentContext = null; + charsToRestore = CharSequenceUtil.restoreChars(expectedChars, currentContext); + Assert.assertEquals("", charsToRestore); + + expectedChars = WINK; + currentContext = "notamatch"; + charsToRestore = CharSequenceUtil.restoreChars(expectedChars, currentContext); + Assert.assertEquals("", charsToRestore); + } + + @Test + public void test_restoreChars_split_surrogate_pair() { + CharSequence expectedChars = "o" + P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT + WINK + "p"; + CharSequence currentContext = P_COMPOSING_CIRCUMFLEX_ACCENT + SMILEY_HIGH_SURROGATE_PAIR; + CharSequence charsToRestore = CharSequenceUtil.restoreChars(expectedChars, currentContext); + Assert.assertEquals("\uDC3C" + "p", charsToRestore); + } + + @Test + public void test_restoreChars_expected_chars() { + CharSequence expectedChars = "qwertyp" + SMILEY + COMPOSING_DOT_ABOVE; + CharSequence currentContext = "qwertyp"; + CharSequence charsToRestore = CharSequenceUtil.restoreChars(expectedChars, currentContext); + Assert.assertEquals(SMILEY + COMPOSING_DOT_ABOVE, charsToRestore); + + expectedChars = "qwertyp" + COMPOSING_CIRCUMFLEX_ACCENT + "p" + COMPOSING_CIRCUMFLEX_ACCENT; + currentContext = "qwertyp"; + charsToRestore = CharSequenceUtil.restoreChars(expectedChars, currentContext); + Assert.assertEquals(COMPOSING_CIRCUMFLEX_ACCENT + "p" + COMPOSING_CIRCUMFLEX_ACCENT, charsToRestore); + + expectedChars = "qwertyp" + COMPOSING_CIRCUMFLEX_ACCENT + "p" + COMPOSING_CIRCUMFLEX_ACCENT; + currentContext = "qwerty"; + charsToRestore = CharSequenceUtil.restoreChars(expectedChars, currentContext); + Assert.assertEquals("p" + COMPOSING_CIRCUMFLEX_ACCENT + "p" + COMPOSING_CIRCUMFLEX_ACCENT, charsToRestore); + } + + //endregion + + // region adjustCursorPosition tests + + @Test + public void test_adjustCursorPosition_invalid_input() { + CharSequence sequence = null; + String s = P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT; + int move = CharSequenceUtil.adjustCursorPosition(sequence, s); + Assert.assertEquals(0, move); + + sequence = P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT; + s = null; + move = CharSequenceUtil.adjustCursorPosition(sequence, s); + Assert.assertEquals(0, move); + } + + @Test + public void test_adjustCursorPosition_split_surrogate_pair() { + CharSequence sequence = SMILEY_LOW_SURROGATE_PAIR + P_COMPOSING_CIRCUMFLEX_ACCENT + SMILEY_HIGH_SURROGATE_PAIR; + String s = P_COMPOSING_CIRCUMFLEX_ACCENT; + int move = CharSequenceUtil.adjustCursorPosition(sequence, s); + Assert.assertEquals(1, move); + } + + @Test + public void test_adjustCursorPosition_move() { + CharSequence charsBefore = P_COMPOSING_DOT_ABOVE + SMILEY; + String s = P_COMPOSING_DOT_ABOVE; + int move = CharSequenceUtil.adjustCursorPosition(charsBefore, s); + Assert.assertEquals(2, move); + + charsBefore = P_COMPOSING_CIRCUMFLEX_ACCENT + SMILEY; + s = P_COMPOSING_CIRCUMFLEX_ACCENT; + move = CharSequenceUtil.adjustCursorPosition(charsBefore, s); + Assert.assertEquals(2, move); + + charsBefore = P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT + SMILEY + P_COMPOSING_DOT_ABOVE; + s = P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT; + move = CharSequenceUtil.adjustCursorPosition(charsBefore, s); + Assert.assertEquals(4, move); + + // Firefox behavior + charsBefore = "o" + P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT + SMILEY + "p"; + s = P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT; + move = CharSequenceUtil.adjustCursorPosition(charsBefore, s); + Assert.assertEquals(3, move); + } + + //endregion + +} diff --git a/android/history.md b/android/history.md index eb92593140..05586fcebe 100644 --- a/android/history.md +++ b/android/history.md @@ -3,6 +3,13 @@ ## 13.0 alpha * Start version 13.0 +## 2019-11-22 12.0.4208 stable +* Bug fix: + * Fix context manipulation to work around Chromium issue (#2281) + +## 2019-11-13 12.0.4207 stable +* No change to Keyman for Android (updated Keyman Web Engine, #2288) + ## 2019-10-30 12.0.4206 stable * Bug fix: * Disable suggestions when system keyboard entering password field (#2255)