Merge pull request #2281 from keymanapp/android-fix-left-deletions

[Android] Workaround to fix left deletions

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;
+    }
   }
-}
+}

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;
+  }
+
+}

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
+
+}

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)