From f60b47a65b867467f1ecbc74a72305f128847a4f Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Wed, 30 Oct 2019 23:41:47 +0700 Subject: [PATCH 01/27] fix(android): Adjust for Android left deletions --- .../app/src/main/java/com/tavultesoft/kmea/KMManager.java | 5 +++++ 1 file changed, 5 insertions(+) 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..05517310a6 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 @@ -2038,11 +2038,16 @@ public final class KMManager { CharSequence chars = ic.getTextBeforeCursor(1, 0); if (chars != null && chars.length() > 0) { char c = chars.charAt(0); + int codePoint = ((String)chars).codePointAt(0); SystemKeyboardShouldIgnoreSelectionChange = true; if (Character.isLowSurrogate(c)) { ic.deleteSurroundingText(2, 0); } else { ic.deleteSurroundingText(1, 0); + // TODO: Investigate why base character + combining diacritic gets deleted + if (Character.getType(codePoint) == Character.NON_SPACING_MARK){ + i++; + } } } } From deb6b6f4b0abda5ddbe35bbf98a6def97ec39429 Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Thu, 31 Oct 2019 23:16:44 +0700 Subject: [PATCH 02/27] fix(android): Also increment when deleting SMP --- .../KMEA/app/src/main/java/com/tavultesoft/kmea/KMManager.java | 3 +++ 1 file changed, 3 insertions(+) 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 05517310a6..4da0c85623 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 @@ -11,6 +11,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; import java.util.HashMap; +import java.util.stream.IntStream; import android.annotation.SuppressLint; import android.app.Dialog; @@ -2038,10 +2039,12 @@ public final class KMManager { CharSequence chars = ic.getTextBeforeCursor(1, 0); if (chars != null && chars.length() > 0) { char c = chars.charAt(0); + IntStream codePoints = chars.codePoints(); int codePoint = ((String)chars).codePointAt(0); SystemKeyboardShouldIgnoreSelectionChange = true; if (Character.isLowSurrogate(c)) { ic.deleteSurroundingText(2, 0); + i++; } else { ic.deleteSurroundingText(1, 0); // TODO: Investigate why base character + combining diacritic gets deleted From 750bd6b86090ee5722378324b736cac2eacaac5e Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Fri, 1 Nov 2019 15:04:36 +0700 Subject: [PATCH 03/27] fix(android): Start implementing algorithm to compensate for deletes --- .../java/com/tavultesoft/kmea/KMManager.java | 41 +++++++++++++++---- 1 file changed, 34 insertions(+), 7 deletions(-) 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 4da0c85623..64abe45d7b 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 @@ -2035,25 +2035,52 @@ public final class KMManager { } // Perform left-deletions + // TODO: Chromium has a bug where deleteSurroundingText deletes an entire grapheme cluster + // instead of one code-point. + // 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 + CharSequence charsBackup = ic.getTextBeforeCursor(dn*2 + 16, 0); + IntStream codePoints = charsBackup.codePoints(); + + // Count the number of surrogate pairs in this buffer, backwards from the end + // until we reach dn codepoints. + // Unfortunately, can't foreach charSequence in reverse + int numPairs = 0; + int lastIndex = charsBackup.length()-1; + int counter=0; + if (dn > 1) { + do { + if ((lastIndex - counter > 0) && Character.isLowSurrogate(charsBackup.charAt(lastIndex - counter))) { + numPairs++; + } + counter++; + } while (counter < dn && numPairs < dn); + } + // Delete dn+numPairs code points from right of charsBackup + charsBackup = charsBackup.subSequence(0, (charsBackup.length()-1) - (dn + 2*numPairs)); + + ic.deleteSurroundingText(dn+numPairs, 0); + + // Now see if we need to re-insert characters + CharSequence newChars = ic.getTextBeforeCursor((dn*2+16) - (dn+numPairs), 0); + + /* + // Perform left-deletions (legacy) for (int i = 0; i < dn; i++) { + int codePoint = codePoints CharSequence chars = ic.getTextBeforeCursor(1, 0); if (chars != null && chars.length() > 0) { char c = chars.charAt(0); - IntStream codePoints = chars.codePoints(); - int codePoint = ((String)chars).codePointAt(0); SystemKeyboardShouldIgnoreSelectionChange = true; if (Character.isLowSurrogate(c)) { ic.deleteSurroundingText(2, 0); - i++; } else { ic.deleteSurroundingText(1, 0); - // TODO: Investigate why base character + combining diacritic gets deleted - if (Character.getType(codePoint) == Character.NON_SPACING_MARK){ - i++; - } } } } + */ // Perform right-deletions for (int i = 0; i < dr; i++) { From ab37dea0897e19028f1d8892fd70c10ba95567d7 Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Fri, 1 Nov 2019 23:14:56 +0700 Subject: [PATCH 04/27] fix(android): Continue to implement algorithm --- .../java/com/tavultesoft/kmea/KMManager.java | 94 ++++++++++--------- 1 file changed, 49 insertions(+), 45 deletions(-) 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 64abe45d7b..a68aeec202 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 @@ -309,6 +309,53 @@ public final class KMManager { } } + /* + // TODO: Chromium has a bug where deleteSurroundingText deletes an entire grapheme cluster + // instead of one code-point. + // 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 + // + // Visibility public so we can run a unit test + */ + public static void performLeftDeletions(InputConnection ic, int dn) { + int originalBufferLength = dn*2 + 16; // characters + //CharSequence desired = ic.getTextBeforeCursor(dn, 0); + CharSequence charsBackup = ic.getTextBeforeCursor(originalBufferLength, 0); + IntStream codePoints = charsBackup.codePoints(); + + // Count the number of surrogate pairs in this buffer, backwards from the end + // until we reach dn codepoints. + // Unfortunately, can't foreach charSequence in reverse + int numPairs = 0; + int lastIndex = charsBackup.length()-1; + int counter=0; + do { + if ((lastIndex - counter > 0) && Character.isLowSurrogate(charsBackup.charAt(lastIndex - counter))) { + numPairs++; + } + counter++; + } while (counter < dn && numPairs < 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 = ic.getTextBeforeCursor(originalBufferLength-dn, 0); + + // Now see if we need to re-insert characters + if (expectedChars.length() != newContext.length()) { + String newCharsString = expectedChars.toString(); + String newContextString = newContext.toString(); + int index = newCharsString.indexOf(newContextString); + if (index > -1) { + expectedChars = expectedChars.subSequence(index+newContextString.length(), expectedChars.length()); + ic.setComposingText(expectedChars, 0); + } + } + Log.d(TAG, "composing"); + } + public static void hideSystemKeyboard() { if (SystemKeyboard != null) { SystemKeyboard.hideKeyboard(); @@ -2035,52 +2082,9 @@ public final class KMManager { } // Perform left-deletions - // TODO: Chromium has a bug where deleteSurroundingText deletes an entire grapheme cluster - // instead of one code-point. - // 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 - CharSequence charsBackup = ic.getTextBeforeCursor(dn*2 + 16, 0); - IntStream codePoints = charsBackup.codePoints(); - - // Count the number of surrogate pairs in this buffer, backwards from the end - // until we reach dn codepoints. - // Unfortunately, can't foreach charSequence in reverse - int numPairs = 0; - int lastIndex = charsBackup.length()-1; - int counter=0; - if (dn > 1) { - do { - if ((lastIndex - counter > 0) && Character.isLowSurrogate(charsBackup.charAt(lastIndex - counter))) { - numPairs++; - } - counter++; - } while (counter < dn && numPairs < dn); + if (dn > 0) { + performLeftDeletions(ic, dn); } - // Delete dn+numPairs code points from right of charsBackup - charsBackup = charsBackup.subSequence(0, (charsBackup.length()-1) - (dn + 2*numPairs)); - - ic.deleteSurroundingText(dn+numPairs, 0); - - // Now see if we need to re-insert characters - CharSequence newChars = ic.getTextBeforeCursor((dn*2+16) - (dn+numPairs), 0); - - /* - // Perform left-deletions (legacy) - for (int i = 0; i < dn; i++) { - int codePoint = codePoints - 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); - } - } - } - */ // Perform right-deletions for (int i = 0; i < dr; i++) { From f18ba4d5339a0403749fa58958d5b268b95a3f0a Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Sat, 2 Nov 2019 22:59:52 +0700 Subject: [PATCH 05/27] fix(android): advance cursor when re-inserting chars --- .../KMEA/app/src/main/java/com/tavultesoft/kmea/KMManager.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 a68aeec202..b77c75767d 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 @@ -349,8 +349,9 @@ public final class KMManager { String newContextString = newContext.toString(); int index = newCharsString.indexOf(newContextString); if (index > -1) { + // Restore expectedChars that Chromium deleted, and advance the cursor by expectedChars.length() expectedChars = expectedChars.subSequence(index+newContextString.length(), expectedChars.length()); - ic.setComposingText(expectedChars, 0); + ic.setComposingText(expectedChars, expectedChars.length()); } } Log.d(TAG, "composing"); From ea8a08a1a700940de95478490aa8ab8d07f4baa0 Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Sun, 3 Nov 2019 22:23:35 +0700 Subject: [PATCH 06/27] chore(android): cleanup unused code --- .../KMEA/app/src/main/java/com/tavultesoft/kmea/KMManager.java | 3 --- 1 file changed, 3 deletions(-) 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 b77c75767d..d29474717d 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 @@ -11,7 +11,6 @@ import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; import java.util.HashMap; -import java.util.stream.IntStream; import android.annotation.SuppressLint; import android.app.Dialog; @@ -320,9 +319,7 @@ public final class KMManager { */ public static void performLeftDeletions(InputConnection ic, int dn) { int originalBufferLength = dn*2 + 16; // characters - //CharSequence desired = ic.getTextBeforeCursor(dn, 0); CharSequence charsBackup = ic.getTextBeforeCursor(originalBufferLength, 0); - IntStream codePoints = charsBackup.codePoints(); // Count the number of surrogate pairs in this buffer, backwards from the end // until we reach dn codepoints. From 2bfb3142d4579ad3427961d0948e30860f8ffd9d Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Wed, 6 Nov 2019 09:26:43 +0700 Subject: [PATCH 07/27] fix(android): Use commitText instead of setComposingText This does a better job of updating the cursor so the next character gets correctly inserted --- .../KMEA/app/src/main/java/com/tavultesoft/kmea/KMManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 d29474717d..a969e15128 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 @@ -348,7 +348,7 @@ public final class KMManager { if (index > -1) { // Restore expectedChars that Chromium deleted, and advance the cursor by expectedChars.length() expectedChars = expectedChars.subSequence(index+newContextString.length(), expectedChars.length()); - ic.setComposingText(expectedChars, expectedChars.length()); + ic.commitText(expectedChars, expectedChars.length()); } } Log.d(TAG, "composing"); From 95c0206309d4e6e6c1c6978c82ff7c3f95ef2ebe Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Wed, 6 Nov 2019 10:24:19 +0700 Subject: [PATCH 08/27] chore: cleanup * Make performLeftDeletions private * Add try catch block to guard against out of bounds errors --- .../java/com/tavultesoft/kmea/KMManager.java | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) 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 a969e15128..a9a27f8728 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 @@ -314,10 +314,8 @@ public final class KMManager { // 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 - // - // Visibility public so we can run a unit test */ - public static void performLeftDeletions(InputConnection ic, int dn) { + private static void performLeftDeletions(InputConnection ic, int dn) { int originalBufferLength = dn*2 + 16; // characters CharSequence charsBackup = ic.getTextBeforeCursor(originalBufferLength, 0); @@ -327,31 +325,34 @@ public final class KMManager { int numPairs = 0; int lastIndex = charsBackup.length()-1; int counter=0; - do { - if ((lastIndex - counter > 0) && Character.isLowSurrogate(charsBackup.charAt(lastIndex - counter))) { - numPairs++; - } - counter++; - } while (counter < dn && numPairs < dn); + try { + do { + if ((lastIndex - counter > 0) && Character.isLowSurrogate(charsBackup.charAt(lastIndex - counter))) { + numPairs++; + } + counter++; + } while (counter < dn && numPairs < 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 = ic.getTextBeforeCursor(originalBufferLength-dn, 0); + // 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 = ic.getTextBeforeCursor(originalBufferLength - dn, 0); - // Now see if we need to re-insert characters - if (expectedChars.length() != newContext.length()) { - String newCharsString = expectedChars.toString(); - String newContextString = newContext.toString(); - int index = newCharsString.indexOf(newContextString); - if (index > -1) { - // Restore expectedChars that Chromium deleted, and advance the cursor by expectedChars.length() - expectedChars = expectedChars.subSequence(index+newContextString.length(), expectedChars.length()); - ic.commitText(expectedChars, expectedChars.length()); + // Now see if we need to re-insert characters + if (expectedChars.length() != newContext.length()) { + String newCharsString = expectedChars.toString(); + String newContextString = newContext.toString(); + int index = newCharsString.indexOf(newContextString); + if (index > -1) { + // Restore expectedChars that Chromium deleted, and advance the cursor by expectedChars.length() + expectedChars = expectedChars.subSequence(index + newContextString.length(), expectedChars.length()); + ic.commitText(expectedChars, expectedChars.length()); + } } + } catch (Exception e) { + Log.e(TAG, "Error in performLeftDeletions: " + e); } - Log.d(TAG, "composing"); } public static void hideSystemKeyboard() { From 8a74b8aa90c8f0a23eba1949da2aa3431f430a57 Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Wed, 6 Nov 2019 13:11:45 +0700 Subject: [PATCH 09/27] fix(android): Skip if there's no left context to delete --- .../app/src/main/java/com/tavultesoft/kmea/KMManager.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 a9a27f8728..b088670935 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 @@ -318,12 +318,17 @@ public final class KMManager { private static void performLeftDeletions(InputConnection ic, int dn) { int originalBufferLength = dn*2 + 16; // characters CharSequence charsBackup = ic.getTextBeforeCursor(originalBufferLength, 0); + int lastIndex = charsBackup.length()-1; + + // Exit if there's no context to delete + if (lastIndex < 0) { + return; + } // Count the number of surrogate pairs in this buffer, backwards from the end // until we reach dn codepoints. // Unfortunately, can't foreach charSequence in reverse int numPairs = 0; - int lastIndex = charsBackup.length()-1; int counter=0; try { do { From 87178b0640836909b08d43faef5e565e03b074d8 Mon Sep 17 00:00:00 2001 From: Sandra Forberger Date: Thu, 7 Nov 2019 10:05:48 +0700 Subject: [PATCH 10/27] fix[android]: fix cursor after insert --- .../java/com/tavultesoft/kmea/KMManager.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) 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 b088670935..0421df7555 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 @@ -2106,11 +2106,36 @@ public final class KMManager { if (s.length() > 0) { SystemKeyboardShouldIgnoreSelectionChange = true; + ic.commitText(s, s.length()); + + adjustCursorPosition(ic, s); } ic.endBatchEdit(); } + + /** + * Adjust cursor position after insert the new text. + * @param ic the inputconnection + * @param s the inserted text + */ + private void adjustCursorPosition(InputConnection ic, String s) { + CharSequence _charbefore = ic.getTextBeforeCursor(s.length()*2,0); + + int _expected_start_index = _charbefore.length() - s.length(); + int _move = 0; + while (_move < _expected_start_index) { + + CharSequence _check = _charbefore.subSequence( + _expected_start_index - _move,_charbefore.length()-_move); + if(_check.equals(s)) + break; + _move++; + } + if(_move>0) + ic.commitText("", -_move); + } }); } @@ -2119,4 +2144,6 @@ public final class KMManager { IMService.getCurrentInputConnection().sendKeyEvent(new KeyEvent(KeyEvent.ACTION_UP, keyEventCode)); } } + + } \ No newline at end of file From 8daa93a8e48af7eda045895ec4e4c1ddd5830691 Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Fri, 8 Nov 2019 15:35:37 +0700 Subject: [PATCH 11/27] fix(android): Start refactoring to add unit tests --- .../java/com/tavultesoft/kmea/KMManager.java | 45 +++++++++--- .../kmmanager/CountSurrogatePairsTest.java | 69 +++++++++++++++++++ 2 files changed, 103 insertions(+), 11 deletions(-) create mode 100644 android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/CountSurrogatePairsTest.java 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 0421df7555..6df2f1dd43 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 @@ -308,6 +308,39 @@ public final class KMManager { } } + /** + * Count the number of surrogate pairs starting from the end of a character sequence + * until we reach dn codepoints. + * 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 ((lastIndex - counter > 0) && Character.isLowSurrogate(sequence.charAt(lastIndex - counter))) { + numPairs++; + } + counter++; + } while (counter < dn && numPairs < dn); + + return numPairs; + } catch (Exception e) { + Log.e(TAG, "Error in countSurrogatePairs: " + e); + if (counter >= dn) { + return numPairs; + } + return 0; + } + } + /* // TODO: Chromium has a bug where deleteSurroundingText deletes an entire grapheme cluster // instead of one code-point. @@ -325,18 +358,8 @@ public final class KMManager { return; } - // Count the number of surrogate pairs in this buffer, backwards from the end - // until we reach dn codepoints. - // Unfortunately, can't foreach charSequence in reverse - int numPairs = 0; - int counter=0; try { - do { - if ((lastIndex - counter > 0) && Character.isLowSurrogate(charsBackup.charAt(lastIndex - counter))) { - numPairs++; - } - counter++; - } while (counter < dn && numPairs < dn); + int numPairs = countSurrogatePairs(charsBackup, dn); // Chop dn+numPairs code points from the end of charsBackup // subSequence indices are start(inclusive) to end(exclusive) diff --git a/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/CountSurrogatePairsTest.java b/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/CountSurrogatePairsTest.java new file mode 100644 index 0000000000..818783f641 --- /dev/null +++ b/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/CountSurrogatePairsTest.java @@ -0,0 +1,69 @@ +package com.tavultesoft.kmea.kmmanager; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; + +import com.tavultesoft.kmea.KMManager; + +@RunWith(RobolectricTestRunner.class) +public class CountSurrogatePairsTest { + // Smiley emoji U+1F600 = D800 DC3D + private final String SMILEY = "\uD800\uDC3D"; + + // Winking emoji U+1F609 = D800 DC3C + private final String WINK = "\uD800\uDC3C"; + + + @Test + public void test_invalid_input() { + + // Test invalid input doesn't throw exception + CharSequence sequence = null; + int numPairs = KMManager.countSurrogatePairs(sequence, 1); + Assert.assertEquals(0, numPairs); + + sequence = "test"; + numPairs = KMManager.countSurrogatePairs(sequence, -1); + Assert.assertEquals(0, numPairs); + + numPairs = KMManager.countSurrogatePairs(sequence, sequence.length()+5); + Assert.assertEquals(0, numPairs); + } + + @Test + public void test_zero_pair() { + CharSequence sequence = "Zero emojis"; + int numPairs = KMManager.countSurrogatePairs(sequence, sequence.length()); + Assert.assertEquals(0, numPairs); + } + + @Test + public void test_one_pair() { + + // Test for scenarios that expect 1 surrogate pair + CharSequence sequence = "HaveA" + SMILEY + "Day"; + int numPairs = KMManager.countSurrogatePairs(sequence, 4); + Assert.assertEquals(1, numPairs); + + sequence = "HaveA" + SMILEY + WINK; + numPairs = KMManager.countSurrogatePairs(sequence, 1); + Assert.assertEquals(1, numPairs); + + numPairs = KMManager.countSurrogatePairs(sequence, 2); + Assert.assertEquals(1, numPairs); + } + + @Test + public void test_two_pairs() { + + // Test for scenarios that expect 2 surrogate pairs + CharSequence sequence = "HaveA" + SMILEY + WINK; + int numPairs = KMManager.countSurrogatePairs(sequence, 3); + Assert.assertEquals(2, numPairs); + + numPairs = KMManager.countSurrogatePairs(sequence, 4); + Assert.assertEquals(2, numPairs); + } +} From cb7a87986a2bf5f7ab38b3419fc35d4f50896e49 Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Fri, 8 Nov 2019 21:51:48 +0700 Subject: [PATCH 12/27] fix(android): refactor `restoreChars()` Split out method `restoreChars()` so it can be run in unit test * also adjust CountSurrogatePairsTest --- .../java/com/tavultesoft/kmea/KMManager.java | 68 +++++++++++-------- .../kmmanager/CountSurrogatePairsTest.java | 44 ++++++++---- 2 files changed, 73 insertions(+), 39 deletions(-) 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 6df2f1dd43..39141b561f 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 @@ -310,7 +310,7 @@ public final class KMManager { /** * Count the number of surrogate pairs starting from the end of a character sequence - * until we reach dn codepoints. + * 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 @@ -325,19 +325,43 @@ public final class KMManager { int lastIndex = sequence.length()-1; try { do { - if ((lastIndex - counter > 0) && Character.isLowSurrogate(sequence.charAt(lastIndex - counter))) { + if (Character.isLowSurrogate(sequence.charAt(lastIndex - counter))) { numPairs++; } counter++; - } while (counter < dn && numPairs < dn); - + } while ((lastIndex - counter >= 0) && (counter < dn) && (numPairs < dn)); return numPairs; } catch (Exception e) { Log.e(TAG, "Error in countSurrogatePairs: " + e); - if (counter >= dn) { - return numPairs; + return numPairs; + } + } + + /** + * Determine if a character sequence needs to be re-inserted. If newContext is a + * subSequence of expectedChars, returns the character sequence that needs to be restored. + * @param expectedChars - expected character sequence + * @param newContext - current character sequence + * @return CharSequence. If length is 0, no characters need to be restored + */ + public static CharSequence restoreChars(CharSequence expectedChars, CharSequence newContext) { + CharSequence charsToRestore = ""; + + try { + // Now see if we need to re-insert characters + if (expectedChars.length() != newContext.length()) { + String newCharsString = expectedChars.toString(); + String newContextString = newContext.toString(); + int index = newCharsString.indexOf(newContextString); + if (index > -1) { + charsToRestore = expectedChars.subSequence(index + newContextString.length(), expectedChars.length()); + } } - return 0; + + return charsToRestore; + } catch (Exception e) { + Log.e(TAG, "Error in restoreChars: " + e); + return charsToRestore; } } @@ -358,28 +382,18 @@ public final class KMManager { return; } - try { - int numPairs = countSurrogatePairs(charsBackup, dn); + int numPairs = 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 = ic.getTextBeforeCursor(originalBufferLength - dn, 0); + // 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 = ic.getTextBeforeCursor(originalBufferLength - dn, 0); - // Now see if we need to re-insert characters - if (expectedChars.length() != newContext.length()) { - String newCharsString = expectedChars.toString(); - String newContextString = newContext.toString(); - int index = newCharsString.indexOf(newContextString); - if (index > -1) { - // Restore expectedChars that Chromium deleted, and advance the cursor by expectedChars.length() - expectedChars = expectedChars.subSequence(index + newContextString.length(), expectedChars.length()); - ic.commitText(expectedChars, expectedChars.length()); - } - } - } catch (Exception e) { - Log.e(TAG, "Error in performLeftDeletions: " + e); + CharSequence charsToRestore = restoreChars(expectedChars, newContext); + if (charsToRestore.length() > 0) { + // Restore expectedChars that Chromium deleted, and advance the cursor by expectedChars.length() + ic.commitText(charsToRestore, charsToRestore.length()); } } diff --git a/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/CountSurrogatePairsTest.java b/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/CountSurrogatePairsTest.java index 818783f641..3900be8d23 100644 --- a/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/CountSurrogatePairsTest.java +++ b/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/CountSurrogatePairsTest.java @@ -20,50 +20,70 @@ public class CountSurrogatePairsTest { public void test_invalid_input() { // Test invalid input doesn't throw exception + + // null sequence CharSequence sequence = null; int numPairs = KMManager.countSurrogatePairs(sequence, 1); Assert.assertEquals(0, numPairs); - sequence = "test"; + // negative dn + sequence = SMILEY + " Day"; numPairs = KMManager.countSurrogatePairs(sequence, -1); Assert.assertEquals(0, numPairs); + // dn longer than the sequence length numPairs = KMManager.countSurrogatePairs(sequence, sequence.length()+5); - Assert.assertEquals(0, numPairs); + Assert.assertEquals(1, numPairs); } @Test - public void test_zero_pair() { + public void test_zero_pairs() { + + // Test for scenarios that expect 0 surrogate pairs CharSequence sequence = "Zero emojis"; int numPairs = KMManager.countSurrogatePairs(sequence, sequence.length()); Assert.assertEquals(0, numPairs); + + // dn doesn't reach the surrogate pair + sequence = "Zero emojis" + SMILEY + "!"; + numPairs = KMManager.countSurrogatePairs(sequence, 0); + Assert.assertEquals(0, numPairs); + + numPairs = KMManager.countSurrogatePairs(sequence, 1); + Assert.assertEquals(0, numPairs); } @Test public void test_one_pair() { // Test for scenarios that expect 1 surrogate pair - CharSequence sequence = "HaveA" + SMILEY + "Day"; - int numPairs = KMManager.countSurrogatePairs(sequence, 4); - Assert.assertEquals(1, numPairs); - - sequence = "HaveA" + SMILEY + WINK; - numPairs = KMManager.countSurrogatePairs(sequence, 1); + CharSequence sequence = "Have A " + SMILEY; + int numPairs = KMManager.countSurrogatePairs(sequence, 1); Assert.assertEquals(1, numPairs); numPairs = KMManager.countSurrogatePairs(sequence, 2); Assert.assertEquals(1, numPairs); + + numPairs = KMManager.countSurrogatePairs(sequence, 6); + Assert.assertEquals(1, numPairs); + + sequence = "Have A " + SMILEY + WINK + " Day"; + numPairs = KMManager.countSurrogatePairs(sequence, 5); + Assert.assertEquals(1, numPairs); + + numPairs = KMManager.countSurrogatePairs(sequence, 6); + Assert.assertEquals(1, numPairs); } @Test public void test_two_pairs() { // Test for scenarios that expect 2 surrogate pairs - CharSequence sequence = "HaveA" + SMILEY + WINK; - int numPairs = KMManager.countSurrogatePairs(sequence, 3); + CharSequence sequence = "Have A " + SMILEY + WINK + " Day"; + int numPairs = KMManager.countSurrogatePairs(sequence, 7); Assert.assertEquals(2, numPairs); - numPairs = KMManager.countSurrogatePairs(sequence, 4); + numPairs = KMManager.countSurrogatePairs(sequence, 8); Assert.assertEquals(2, numPairs); } } From 1d521913e4f04f332f5c058114b470938ab18f62 Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Sat, 9 Nov 2019 08:01:55 +0700 Subject: [PATCH 13/27] fix(android): Add unit test `RestoreCharsTest` --- .../java/com/tavultesoft/kmea/KMManager.java | 14 ++--- .../kmea/kmmanager/RestoreCharsTest.java | 60 +++++++++++++++++++ 2 files changed, 67 insertions(+), 7 deletions(-) create mode 100644 android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/RestoreCharsTest.java 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 39141b561f..319bf03dc0 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 @@ -341,20 +341,20 @@ public final class KMManager { * Determine if a character sequence needs to be re-inserted. If newContext is a * subSequence of expectedChars, returns the character sequence that needs to be restored. * @param expectedChars - expected character sequence - * @param newContext - current character sequence + * @param currentContext - current character sequence * @return CharSequence. If length is 0, no characters need to be restored */ - public static CharSequence restoreChars(CharSequence expectedChars, CharSequence newContext) { + public static CharSequence restoreChars(CharSequence expectedChars, CharSequence currentContext) { CharSequence charsToRestore = ""; try { // Now see if we need to re-insert characters - if (expectedChars.length() != newContext.length()) { - String newCharsString = expectedChars.toString(); - String newContextString = newContext.toString(); - int index = newCharsString.indexOf(newContextString); + if (expectedChars.length() != currentContext.length()) { + String expectedCharsString = expectedChars.toString(); + String currentContextString = currentContext.toString(); + int index = expectedCharsString.indexOf(currentContextString); if (index > -1) { - charsToRestore = expectedChars.subSequence(index + newContextString.length(), expectedChars.length()); + charsToRestore = expectedChars.subSequence(index + currentContextString.length(), expectedChars.length()); } } diff --git a/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/RestoreCharsTest.java b/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/RestoreCharsTest.java new file mode 100644 index 0000000000..00fb927ea9 --- /dev/null +++ b/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/RestoreCharsTest.java @@ -0,0 +1,60 @@ +package com.tavultesoft.kmea.kmmanager; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; + +import com.tavultesoft.kmea.KMManager; + +@RunWith(RobolectricTestRunner.class) +public class RestoreCharsTest { + // Smiley emoji U+1F600 = D800 DC3D + private final String SMILEY = "\uD800\uDC3D"; + + // 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"; + + @Test + public void test_invalid_input() { + + // Test invalid input doesn't throw exception + // null sequence + CharSequence expectedChars = null; + CharSequence currentContext = "qwerty"; + CharSequence charsToRestore = KMManager.restoreChars(expectedChars, currentContext); + Assert.assertEquals("", charsToRestore); + + expectedChars = "qwerty"; + currentContext = null; + charsToRestore = KMManager.restoreChars(expectedChars, currentContext); + Assert.assertEquals("", charsToRestore); + + expectedChars = "qwerty"; + currentContext = "notamatch"; + charsToRestore = KMManager.restoreChars(expectedChars, currentContext); + Assert.assertEquals("", charsToRestore); + } + + @Test + public void test_chars() { + + CharSequence expectedChars = "qwertyp" + SMILEY + COMPOSING_DOT_ABOVE; + CharSequence currentContext = "qwertyp"; + CharSequence charsToRestore = KMManager.restoreChars(expectedChars, currentContext); + Assert.assertEquals(SMILEY + COMPOSING_DOT_ABOVE, charsToRestore); + + expectedChars = "qwertyp" + COMPOSING_CIRCUMFLEX_ACCENT + "p" + COMPOSING_CIRCUMFLEX_ACCENT; + currentContext = "qwertyp"; + charsToRestore = KMManager.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 = KMManager.restoreChars(expectedChars, currentContext); + Assert.assertEquals("p" + COMPOSING_CIRCUMFLEX_ACCENT + "p" + COMPOSING_CIRCUMFLEX_ACCENT, charsToRestore); + } +} \ No newline at end of file From d66d029406404e0532d4a640284b94b5acfa5dac Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Mon, 11 Nov 2019 23:16:27 +0700 Subject: [PATCH 14/27] fix(android): change adjustCursorPosition to use CharSequence Changed signature in preparation for unit testing --- .../java/com/tavultesoft/kmea/KMManager.java | 63 ++++++++++--------- 1 file changed, 35 insertions(+), 28 deletions(-) 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 319bf03dc0..8b78feb01e 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 @@ -308,6 +308,8 @@ public final class KMManager { } } + // These aren't public for KMManager API. Just for unit testing: + /** * Count the number of surrogate pairs starting from the end of a character sequence * until we reach dn codepoints or searched the entire sequence. @@ -338,11 +340,12 @@ public final class KMManager { } /** - * Determine if a character sequence needs to be re-inserted. If newContext is a + * 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. If length is 0, no characters need to be restored + * @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 = ""; @@ -354,6 +357,7 @@ public final class KMManager { 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()); } } @@ -365,6 +369,29 @@ public final class KMManager { } } + /** + * 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) { + 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; + } + + // End of public methods for unit testing + /* // TODO: Chromium has a bug where deleteSurroundingText deletes an entire grapheme cluster // instead of one code-point. @@ -2145,34 +2172,15 @@ public final class KMManager { SystemKeyboardShouldIgnoreSelectionChange = true; ic.commitText(s, s.length()); - - adjustCursorPosition(ic, s); + CharSequence charsBefore = ic.getTextBeforeCursor(s.length()*2, 0); + int move = adjustCursorPosition(charsBefore, s); + if (move > 0) { + ic.commitText("", -move); + } } ic.endBatchEdit(); } - - /** - * Adjust cursor position after insert the new text. - * @param ic the inputconnection - * @param s the inserted text - */ - private void adjustCursorPosition(InputConnection ic, String s) { - CharSequence _charbefore = ic.getTextBeforeCursor(s.length()*2,0); - - int _expected_start_index = _charbefore.length() - s.length(); - int _move = 0; - while (_move < _expected_start_index) { - - CharSequence _check = _charbefore.subSequence( - _expected_start_index - _move,_charbefore.length()-_move); - if(_check.equals(s)) - break; - _move++; - } - if(_move>0) - ic.commitText("", -_move); - } }); } @@ -2182,5 +2190,4 @@ public final class KMManager { } } - -} \ No newline at end of file +} From 002f378a5165a45426670a638d4024c34bbd0fbe Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Tue, 12 Nov 2019 15:26:38 +0700 Subject: [PATCH 15/27] fix(android): Add unit test for adjust cursor position Still wip for adjusting cursor in Firefox --- .../java/com/tavultesoft/kmea/KMManager.java | 35 ++++++++++++++- .../kmmanager/AdjustCursorPositionTest.java | 45 +++++++++++++++++++ 2 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/AdjustCursorPositionTest.java 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 8b78feb01e..704cf9e5d5 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 @@ -9,6 +9,7 @@ import java.io.FileFilter; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.text.Normalizer; import java.util.ArrayList; import java.util.HashMap; @@ -376,6 +377,10 @@ public final class KMManager { * @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 _expected_start_index = charsBefore.length() - s.length(); int _move = 0; while (_move < _expected_start_index) { @@ -402,6 +407,14 @@ public final class KMManager { private static void performLeftDeletions(InputConnection ic, int dn) { int originalBufferLength = dn*2 + 16; // characters CharSequence charsBackup = ic.getTextBeforeCursor(originalBufferLength, 0); + + + if (Character.isHighSurrogate(charsBackup.charAt(charsBackup.length()-1))) { + // Firefox sometimes splits a surrogate pair so move the cursor back + ic.commitText("", -1); + charsBackup = ic.getTextBeforeCursor(originalBufferLength, 0); + } + int lastIndex = charsBackup.length()-1; // Exit if there's no context to delete @@ -2170,11 +2183,31 @@ public final class KMManager { if (s.length() > 0) { SystemKeyboardShouldIgnoreSelectionChange = true; - ic.commitText(s, s.length()); + + CharSequence check = ic.getTextBeforeCursor(1, 0); + if (check != null && Character.isHighSurrogate(check.charAt(0))) { + ic.commitText("", -1); + } + // Compensate for surrogate pairs and combining marks + /* + int adjustedLength = s.length(); + for(char c: s.toCharArray()) { + if (Character.isLowSurrogate(c)) { + adjustedLength--; + } else if (Character.getType(c) == Character.NON_SPACING_MARK || + Character.getType(c) == Character.COMBINING_SPACING_MARK) { + adjustedLength -= 2; + } + } + ic.commitText(s, adjustedLength); + */ + + //String userAgent = System.getProperty("http.agent"); CharSequence charsBefore = ic.getTextBeforeCursor(s.length()*2, 0); int move = adjustCursorPosition(charsBefore, s); if (move > 0) { + Log.d(TAG, "charsBefore: " + charsBefore.toString() + ", s: " + s + ", move: " + move); ic.commitText("", -move); } } diff --git a/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/AdjustCursorPositionTest.java b/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/AdjustCursorPositionTest.java new file mode 100644 index 0000000000..931bed4297 --- /dev/null +++ b/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/AdjustCursorPositionTest.java @@ -0,0 +1,45 @@ +package com.tavultesoft.kmea.kmmanager; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; + +import com.tavultesoft.kmea.KMManager; + +@RunWith(RobolectricTestRunner.class) +public class AdjustCursorPositionTest { + // Smiley emoji U+1F600 = D800 DC3D + private final String SMILEY = "\uD800\uDC3D"; + + // 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_CIRCUMFLEX_ACCENT = "p" + COMPOSING_CIRCUMFLEX_ACCENT; + + @Test + public void test_invalid_input() { + CharSequence charsBefore = null; + String s = P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT; + int move = KMManager.adjustCursorPosition(charsBefore, s); + Assert.assertEquals(0, move); + } + + @Test + public void test_move_3() { + CharSequence charsBefore = "o" + P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT + + SMILEY + "q"; + String s = P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT; + + int move = KMManager.adjustCursorPosition(charsBefore, s); + Assert.assertEquals(3, move); + + charsBefore = "g" + P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT + "lyf"; + move = KMManager.adjustCursorPosition(charsBefore, s); + Assert.assertEquals(3, move); + + } + +} \ No newline at end of file From c4c85af98fffd5929c8d4eaa56528809a5234d8b Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Tue, 12 Nov 2019 22:18:52 +0700 Subject: [PATCH 16/27] chore(android): Add logging and adjust unit test constant --- .../java/com/tavultesoft/kmea/KMManager.java | 3 ++- .../kmea/kmmanager/RestoreCharsTest.java | 19 ++++++++++++++----- 2 files changed, 16 insertions(+), 6 deletions(-) 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 704cf9e5d5..f9e4ec33d7 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 @@ -412,6 +412,7 @@ public final class KMManager { if (Character.isHighSurrogate(charsBackup.charAt(charsBackup.length()-1))) { // Firefox sometimes splits a surrogate pair so move the cursor back ic.commitText("", -1); + Log.d(TAG, "adjust trimming high surrogate pair from charsBackup: " + charsBackup.toString()); charsBackup = ic.getTextBeforeCursor(originalBufferLength, 0); } @@ -2207,7 +2208,7 @@ public final class KMManager { CharSequence charsBefore = ic.getTextBeforeCursor(s.length()*2, 0); int move = adjustCursorPosition(charsBefore, s); if (move > 0) { - Log.d(TAG, "charsBefore: " + charsBefore.toString() + ", s: " + s + ", move: " + move); + Log.d(TAG, "adjusting cursor charsBefore: " + charsBefore.toString() + ", s: " + s + ", move: " + move); ic.commitText("", -move); } } diff --git a/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/RestoreCharsTest.java b/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/RestoreCharsTest.java index 00fb927ea9..95cde35078 100644 --- a/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/RestoreCharsTest.java +++ b/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/RestoreCharsTest.java @@ -17,6 +17,7 @@ public class RestoreCharsTest { private final String COMPOSING_DOT_ABOVE = "\u0307"; private final String COMPOSING_CIRCUMFLEX_ACCENT = "\u0302"; + private final String P_COMPOSING_CIRCUMFLEX_ACCENT = "p" + COMPOSING_CIRCUMFLEX_ACCENT; @Test public void test_invalid_input() { @@ -33,12 +34,20 @@ public class RestoreCharsTest { charsToRestore = KMManager.restoreChars(expectedChars, currentContext); Assert.assertEquals("", charsToRestore); - expectedChars = "qwerty"; + expectedChars = WINK; currentContext = "notamatch"; charsToRestore = KMManager.restoreChars(expectedChars, currentContext); Assert.assertEquals("", charsToRestore); } + @Test + public void test_split_surrogate_pair() { + CharSequence expectedChars = "o" + P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT + WINK + "p"; + CharSequence currentContext = P_COMPOSING_CIRCUMFLEX_ACCENT + "\uD800"; + CharSequence charsToRestore = KMManager.restoreChars(expectedChars, currentContext); + Assert.assertEquals("\uDC3C" + "p", charsToRestore); + } + @Test public void test_chars() { @@ -47,14 +56,14 @@ public class RestoreCharsTest { CharSequence charsToRestore = KMManager.restoreChars(expectedChars, currentContext); Assert.assertEquals(SMILEY + COMPOSING_DOT_ABOVE, charsToRestore); - expectedChars = "qwertyp" + COMPOSING_CIRCUMFLEX_ACCENT + "p" + COMPOSING_CIRCUMFLEX_ACCENT; + expectedChars = "qwerty" + P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT; currentContext = "qwertyp"; charsToRestore = KMManager.restoreChars(expectedChars, currentContext); - Assert.assertEquals(COMPOSING_CIRCUMFLEX_ACCENT + "p" + COMPOSING_CIRCUMFLEX_ACCENT, charsToRestore); + Assert.assertEquals(COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT, charsToRestore); - expectedChars = "qwertyp" + COMPOSING_CIRCUMFLEX_ACCENT + "p" + COMPOSING_CIRCUMFLEX_ACCENT; + expectedChars = "qwerty" + P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT; currentContext = "qwerty"; charsToRestore = KMManager.restoreChars(expectedChars, currentContext); - Assert.assertEquals("p" + COMPOSING_CIRCUMFLEX_ACCENT + "p" + COMPOSING_CIRCUMFLEX_ACCENT, charsToRestore); + Assert.assertEquals(P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT, charsToRestore); } } \ No newline at end of file From 6fde9c1739c0dc7141d50d51d960144069e696da Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Wed, 13 Nov 2019 08:31:54 +0700 Subject: [PATCH 17/27] chore(android): move utility methods to new class Refactor charSequence utilities to CharSequenceUtil.java --- .../java/com/tavultesoft/kmea/KMManager.java | 95 +--------- .../kmea/util/CharSequenceUtil.java | 94 ++++++++++ .../kmmanager/AdjustCursorPositionTest.java | 45 ----- .../kmmanager/CountSurrogatePairsTest.java | 89 --------- .../kmea/kmmanager/RestoreCharsTest.java | 69 ------- .../kmea/util/CharSequenceUtilTest.java | 171 ++++++++++++++++++ 6 files changed, 269 insertions(+), 294 deletions(-) create mode 100644 android/KMEA/app/src/main/java/com/tavultesoft/kmea/util/CharSequenceUtil.java delete mode 100644 android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/AdjustCursorPositionTest.java delete mode 100644 android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/CountSurrogatePairsTest.java delete mode 100644 android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/RestoreCharsTest.java create mode 100644 android/KMEA/app/src/test/java/com/tavultesoft/kmea/util/CharSequenceUtilTest.java 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 f9e4ec33d7..250c48960b 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 @@ -61,6 +61,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; @@ -309,94 +310,6 @@ public final class KMManager { } } - // These aren't public for KMManager API. Just for unit testing: - - /** - * 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 _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; - } - - // End of public methods for unit testing - /* // TODO: Chromium has a bug where deleteSurroundingText deletes an entire grapheme cluster // instead of one code-point. @@ -423,7 +336,7 @@ public final class KMManager { return; } - int numPairs = countSurrogatePairs(charsBackup, dn); + int numPairs = CharSequenceUtil.countSurrogatePairs(charsBackup, dn); // Chop dn+numPairs code points from the end of charsBackup // subSequence indices are start(inclusive) to end(exclusive) @@ -431,7 +344,7 @@ public final class KMManager { ic.deleteSurroundingText(dn + numPairs, 0); CharSequence newContext = ic.getTextBeforeCursor(originalBufferLength - dn, 0); - CharSequence charsToRestore = restoreChars(expectedChars, newContext); + CharSequence charsToRestore = CharSequenceUtil.restoreChars(expectedChars, newContext); if (charsToRestore.length() > 0) { // Restore expectedChars that Chromium deleted, and advance the cursor by expectedChars.length() ic.commitText(charsToRestore, charsToRestore.length()); @@ -2206,7 +2119,7 @@ public final class KMManager { //String userAgent = System.getProperty("http.agent"); CharSequence charsBefore = ic.getTextBeforeCursor(s.length()*2, 0); - int move = adjustCursorPosition(charsBefore, s); + int move = CharSequenceUtil.adjustCursorPosition(charsBefore, s); if (move > 0) { Log.d(TAG, "adjusting cursor charsBefore: " + charsBefore.toString() + ", s: " + s + ", move: " + move); ic.commitText("", -move); 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..694c765ef9 --- /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 _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; + } + +} \ No newline at end of file diff --git a/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/AdjustCursorPositionTest.java b/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/AdjustCursorPositionTest.java deleted file mode 100644 index 931bed4297..0000000000 --- a/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/AdjustCursorPositionTest.java +++ /dev/null @@ -1,45 +0,0 @@ -package com.tavultesoft.kmea.kmmanager; - -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.robolectric.RobolectricTestRunner; - -import com.tavultesoft.kmea.KMManager; - -@RunWith(RobolectricTestRunner.class) -public class AdjustCursorPositionTest { - // Smiley emoji U+1F600 = D800 DC3D - private final String SMILEY = "\uD800\uDC3D"; - - // 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_CIRCUMFLEX_ACCENT = "p" + COMPOSING_CIRCUMFLEX_ACCENT; - - @Test - public void test_invalid_input() { - CharSequence charsBefore = null; - String s = P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT; - int move = KMManager.adjustCursorPosition(charsBefore, s); - Assert.assertEquals(0, move); - } - - @Test - public void test_move_3() { - CharSequence charsBefore = "o" + P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT + - SMILEY + "q"; - String s = P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT; - - int move = KMManager.adjustCursorPosition(charsBefore, s); - Assert.assertEquals(3, move); - - charsBefore = "g" + P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT + "lyf"; - move = KMManager.adjustCursorPosition(charsBefore, s); - Assert.assertEquals(3, move); - - } - -} \ No newline at end of file diff --git a/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/CountSurrogatePairsTest.java b/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/CountSurrogatePairsTest.java deleted file mode 100644 index 3900be8d23..0000000000 --- a/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/CountSurrogatePairsTest.java +++ /dev/null @@ -1,89 +0,0 @@ -package com.tavultesoft.kmea.kmmanager; - -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.robolectric.RobolectricTestRunner; - -import com.tavultesoft.kmea.KMManager; - -@RunWith(RobolectricTestRunner.class) -public class CountSurrogatePairsTest { - // Smiley emoji U+1F600 = D800 DC3D - private final String SMILEY = "\uD800\uDC3D"; - - // Winking emoji U+1F609 = D800 DC3C - private final String WINK = "\uD800\uDC3C"; - - - @Test - public void test_invalid_input() { - - // Test invalid input doesn't throw exception - - // null sequence - CharSequence sequence = null; - int numPairs = KMManager.countSurrogatePairs(sequence, 1); - Assert.assertEquals(0, numPairs); - - // negative dn - sequence = SMILEY + " Day"; - numPairs = KMManager.countSurrogatePairs(sequence, -1); - Assert.assertEquals(0, numPairs); - - // dn longer than the sequence length - numPairs = KMManager.countSurrogatePairs(sequence, sequence.length()+5); - Assert.assertEquals(1, numPairs); - } - - @Test - public void test_zero_pairs() { - - // Test for scenarios that expect 0 surrogate pairs - CharSequence sequence = "Zero emojis"; - int numPairs = KMManager.countSurrogatePairs(sequence, sequence.length()); - Assert.assertEquals(0, numPairs); - - // dn doesn't reach the surrogate pair - sequence = "Zero emojis" + SMILEY + "!"; - numPairs = KMManager.countSurrogatePairs(sequence, 0); - Assert.assertEquals(0, numPairs); - - numPairs = KMManager.countSurrogatePairs(sequence, 1); - Assert.assertEquals(0, numPairs); - } - - @Test - public void test_one_pair() { - - // Test for scenarios that expect 1 surrogate pair - CharSequence sequence = "Have A " + SMILEY; - int numPairs = KMManager.countSurrogatePairs(sequence, 1); - Assert.assertEquals(1, numPairs); - - numPairs = KMManager.countSurrogatePairs(sequence, 2); - Assert.assertEquals(1, numPairs); - - numPairs = KMManager.countSurrogatePairs(sequence, 6); - Assert.assertEquals(1, numPairs); - - sequence = "Have A " + SMILEY + WINK + " Day"; - numPairs = KMManager.countSurrogatePairs(sequence, 5); - Assert.assertEquals(1, numPairs); - - numPairs = KMManager.countSurrogatePairs(sequence, 6); - Assert.assertEquals(1, numPairs); - } - - @Test - public void test_two_pairs() { - - // Test for scenarios that expect 2 surrogate pairs - CharSequence sequence = "Have A " + SMILEY + WINK + " Day"; - int numPairs = KMManager.countSurrogatePairs(sequence, 7); - Assert.assertEquals(2, numPairs); - - numPairs = KMManager.countSurrogatePairs(sequence, 8); - Assert.assertEquals(2, numPairs); - } -} diff --git a/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/RestoreCharsTest.java b/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/RestoreCharsTest.java deleted file mode 100644 index 95cde35078..0000000000 --- a/android/KMEA/app/src/test/java/com/tavultesoft/kmea/kmmanager/RestoreCharsTest.java +++ /dev/null @@ -1,69 +0,0 @@ -package com.tavultesoft.kmea.kmmanager; - -import org.junit.Assert; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.robolectric.RobolectricTestRunner; - -import com.tavultesoft.kmea.KMManager; - -@RunWith(RobolectricTestRunner.class) -public class RestoreCharsTest { - // Smiley emoji U+1F600 = D800 DC3D - private final String SMILEY = "\uD800\uDC3D"; - - // 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_CIRCUMFLEX_ACCENT = "p" + COMPOSING_CIRCUMFLEX_ACCENT; - - @Test - public void test_invalid_input() { - - // Test invalid input doesn't throw exception - // null sequence - CharSequence expectedChars = null; - CharSequence currentContext = "qwerty"; - CharSequence charsToRestore = KMManager.restoreChars(expectedChars, currentContext); - Assert.assertEquals("", charsToRestore); - - expectedChars = "qwerty"; - currentContext = null; - charsToRestore = KMManager.restoreChars(expectedChars, currentContext); - Assert.assertEquals("", charsToRestore); - - expectedChars = WINK; - currentContext = "notamatch"; - charsToRestore = KMManager.restoreChars(expectedChars, currentContext); - Assert.assertEquals("", charsToRestore); - } - - @Test - public void test_split_surrogate_pair() { - CharSequence expectedChars = "o" + P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT + WINK + "p"; - CharSequence currentContext = P_COMPOSING_CIRCUMFLEX_ACCENT + "\uD800"; - CharSequence charsToRestore = KMManager.restoreChars(expectedChars, currentContext); - Assert.assertEquals("\uDC3C" + "p", charsToRestore); - } - - @Test - public void test_chars() { - - CharSequence expectedChars = "qwertyp" + SMILEY + COMPOSING_DOT_ABOVE; - CharSequence currentContext = "qwertyp"; - CharSequence charsToRestore = KMManager.restoreChars(expectedChars, currentContext); - Assert.assertEquals(SMILEY + COMPOSING_DOT_ABOVE, charsToRestore); - - expectedChars = "qwerty" + P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT; - currentContext = "qwertyp"; - charsToRestore = KMManager.restoreChars(expectedChars, currentContext); - Assert.assertEquals(COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT, charsToRestore); - - expectedChars = "qwerty" + P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT; - currentContext = "qwerty"; - charsToRestore = KMManager.restoreChars(expectedChars, currentContext); - Assert.assertEquals(P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT, charsToRestore); - } -} \ No newline at end of file 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..80627ddede --- /dev/null +++ b/android/KMEA/app/src/test/java/com/tavultesoft/kmea/util/CharSequenceUtilTest.java @@ -0,0 +1,171 @@ +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 = "\uD800\uDC3D"; + + // 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; + + // Section countSurrogaetPairs 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 = "Have A " + SMILEY; + int 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); + } + + + @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 + "\uD800"; + 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); + } + + @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_move_3() { + CharSequence charsBefore = "o" + P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT + + SMILEY + P_COMPOSING_DOT_ABOVE; + String s = P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT; + + int move = CharSequenceUtil.adjustCursorPosition(charsBefore, s); + Assert.assertEquals(4, move); + + charsBefore = "g" + P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT + "lyf"; + move = CharSequenceUtil.adjustCursorPosition(charsBefore, s); + Assert.assertEquals(3, move); + + } +} From 29b201cd00a3d42e3431d5b3bcd125a3dbe8fbab Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Wed, 13 Nov 2019 15:43:23 +0700 Subject: [PATCH 18/27] fix(android): Still trying to adjust the cursor on Firefox * Also updated CharSequenceUtilTest to reflect expected rota keyboard output --- .../java/com/tavultesoft/kmea/KMManager.java | 45 +++++++++------- .../kmea/util/CharSequenceUtil.java | 6 ++- .../kmea/util/CharSequenceUtilTest.java | 53 +++++++++++++++---- 3 files changed, 73 insertions(+), 31 deletions(-) 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 250c48960b..37bf3ebdc0 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 @@ -321,12 +321,15 @@ public final class KMManager { int originalBufferLength = dn*2 + 16; // characters CharSequence charsBackup = ic.getTextBeforeCursor(originalBufferLength, 0); - if (Character.isHighSurrogate(charsBackup.charAt(charsBackup.length()-1))) { // Firefox sometimes splits a surrogate pair so move the cursor back + String origChars = charsBackup.toString(); + ic.commitText("", -1); - Log.d(TAG, "adjust trimming high surrogate pair from charsBackup: " + charsBackup.toString()); charsBackup = ic.getTextBeforeCursor(originalBufferLength, 0); + Log.d(TAG, "adjusting high surrogate pair from charsBackup was: " + origChars + + ", now: " + charsBackup.toString()); + } int lastIndex = charsBackup.length()-1; @@ -348,6 +351,7 @@ public final class KMManager { if (charsToRestore.length() > 0) { // Restore expectedChars that Chromium deleted, and advance the cursor by expectedChars.length() ic.commitText(charsToRestore, charsToRestore.length()); + Log.d(TAG, "performLeftDeletions commitText(" + charsToRestore.toString() + ")"); } } @@ -2098,31 +2102,33 @@ public final class KMManager { if (s.length() > 0) { SystemKeyboardShouldIgnoreSelectionChange = true; ic.commitText(s, s.length()); + Log.d(TAG, "adjusting commitText s:" + s.toString()); CharSequence check = ic.getTextBeforeCursor(1, 0); - if (check != null && Character.isHighSurrogate(check.charAt(0))) { - ic.commitText("", -1); - } - // Compensate for surrogate pairs and combining marks - /* - int adjustedLength = s.length(); - for(char c: s.toCharArray()) { - if (Character.isLowSurrogate(c)) { - adjustedLength--; - } else if (Character.getType(c) == Character.NON_SPACING_MARK || - Character.getType(c) == Character.COMBINING_SPACING_MARK) { - adjustedLength -= 2; - } - } - ic.commitText(s, adjustedLength); - */ - //String userAgent = System.getProperty("http.agent"); + if (check != null && Character.isHighSurrogate(check.charAt(0))) { + // Firefox sometimes splits a surrogate pair so move the cursor back + String origChars = check.toString(); + + ic.commitText("", -1); + check = ic.getTextBeforeCursor(1, 0); + Log.d(TAG, "adjusting high surrogate pair from check was: " + origChars + + ", now: " + check.toString()); + } + CharSequence charsBefore = ic.getTextBeforeCursor(s.length()*2, 0); int move = CharSequenceUtil.adjustCursorPosition(charsBefore, s); if (move > 0) { Log.d(TAG, "adjusting cursor charsBefore: " + charsBefore.toString() + ", s: " + s + ", move: " + move); ic.commitText("", -move); + + // Do another check for Firefox? + charsBefore = ic.getTextBeforeCursor(s.length()*2, 0); + move = CharSequenceUtil.adjustCursorPosition(charsBefore, s); + if (move > 0) { + Log.d(TAG, "adjusting cursor 2 charsBefore: " + charsBefore.toString() + ", s: " + s + ", move: " + move); + ic.commitText("", -move); + } } } @@ -2136,5 +2142,4 @@ public final class KMManager { IMService.getCurrentInputConnection().sendKeyEvent(new KeyEvent(KeyEvent.ACTION_UP, keyEventCode)); } } - } 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 index 694c765ef9..1045883c31 100644 --- 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 @@ -59,6 +59,9 @@ public final class CharSequenceUtil { } } + Log.d(TAG, "adjusting expectedChars: " + expectedChars.toString() + + ", currentContext: " + currentContext.toString() + + ", charsToRestore: " + charsToRestore.toString()); return charsToRestore; } catch (Exception e) { Log.e(TAG, "Error in restoreChars: " + e); @@ -77,6 +80,7 @@ public final class CharSequenceUtil { return 0; } + int numPairs = countSurrogatePairs(charsBefore, s.length()); int _expected_start_index = charsBefore.length() - s.length(); int _move = 0; while (_move < _expected_start_index) { @@ -91,4 +95,4 @@ public final class CharSequenceUtil { return _move; } -} \ No newline at end of file +} 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 index 80627ddede..3dd2e69b31 100644 --- 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 @@ -10,7 +10,9 @@ import com.tavultesoft.kmea.util.CharSequenceUtil; @RunWith(RobolectricTestRunner.class) public class CharSequenceUtilTest { // Smiley emoji U+1F600 = D800 DC3D - private final String SMILEY = "\uD800\uDC3D"; + 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"; @@ -21,7 +23,7 @@ public class CharSequenceUtilTest { private final String P_COMPOSING_DOT_ABOVE = "p" + COMPOSING_DOT_ABOVE; private final String P_COMPOSING_CIRCUMFLEX_ACCENT = "p" + COMPOSING_CIRCUMFLEX_ACCENT; - // Section countSurrogaetPairs Tests + //region countSurrogatePairs tests @Test public void test_countSurrogatePairs_invalid_input() { @@ -64,10 +66,14 @@ public class CharSequenceUtilTest { public void test_countSurrogatePairs_one_pair() { // Test for scenarios that expect 1 surrogate pair - CharSequence sequence = "Have A " + SMILEY; + 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); @@ -94,6 +100,9 @@ public class CharSequenceUtilTest { Assert.assertEquals(2, numPairs); } + //endregion + + // region restoreChars tests @Test public void test_restoreChars_invalid_input() { @@ -118,7 +127,7 @@ public class CharSequenceUtilTest { @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 + "\uD800"; + CharSequence currentContext = P_COMPOSING_CIRCUMFLEX_ACCENT + SMILEY_HIGH_SURROGATE_PAIR; CharSequence charsToRestore = CharSequenceUtil.restoreChars(expectedChars, currentContext); Assert.assertEquals("\uDC3C" + "p", charsToRestore); } @@ -141,6 +150,10 @@ public class CharSequenceUtilTest { 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; @@ -155,17 +168,37 @@ public class CharSequenceUtilTest { } @Test - public void test_adjustCursorPosition_move_3() { - CharSequence charsBefore = "o" + P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT + - SMILEY + P_COMPOSING_DOT_ABOVE; - String s = P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT; + 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); - charsBefore = "g" + P_COMPOSING_CIRCUMFLEX_ACCENT + P_COMPOSING_CIRCUMFLEX_ACCENT + "lyf"; + // 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 + } From b880638ed024b3bd9706f9c9809a460c48bf0ee6 Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Thu, 14 Nov 2019 11:08:17 +0700 Subject: [PATCH 19/27] fix(android): Use BreakIterator to determine cursor when commiting text --- .../java/com/tavultesoft/kmea/KMManager.java | 46 ++++++++++--------- 1 file changed, 24 insertions(+), 22 deletions(-) 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 37bf3ebdc0..8a2f920eba 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 @@ -9,7 +9,7 @@ import java.io.FileFilter; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; -import java.text.Normalizer; +import java.text.BreakIterator; import java.util.ArrayList; import java.util.HashMap; @@ -329,7 +329,6 @@ public final class KMManager { charsBackup = ic.getTextBeforeCursor(originalBufferLength, 0); Log.d(TAG, "adjusting high surrogate pair from charsBackup was: " + origChars + ", now: " + charsBackup.toString()); - } int lastIndex = charsBackup.length()-1; @@ -2101,34 +2100,37 @@ public final class KMManager { if (s.length() > 0) { SystemKeyboardShouldIgnoreSelectionChange = true; - ic.commitText(s, s.length()); - Log.d(TAG, "adjusting commitText s:" + s.toString()); - CharSequence check = ic.getTextBeforeCursor(1, 0); - - if (check != null && Character.isHighSurrogate(check.charAt(0))) { - // Firefox sometimes splits a surrogate pair so move the cursor back - String origChars = check.toString(); - - ic.commitText("", -1); - check = ic.getTextBeforeCursor(1, 0); - Log.d(TAG, "adjusting high surrogate pair from check was: " + origChars + - ", now: " + check.toString()); + // Using BreakIterator to count grapheme clusters + BreakIterator boundary = BreakIterator.getCharacterInstance(); + boundary.setText(s); + int count = 0; + while (boundary.next() != BreakIterator.DONE) { + count++; } + // Commit the string s and adjust the cursor by the number of grapheme clusters + ic.commitText(s, count); + Log.d(TAG, "adjusting commitText s:" + s.toString() + ", count: " + count + ", s.length: " + s.length()); + + // After inserting the string s and adjusting the cursor count clusters, + // Chrome and Firefox sometimes have the cursor too far, so + // check if we need to adjust the cursor position CharSequence charsBefore = ic.getTextBeforeCursor(s.length()*2, 0); + if (charsBefore != null && Character.isHighSurrogate(charsBefore.charAt(charsBefore.length()-1))) { + // Firefox sometimes splits a surrogate pair so move the cursor back + String origChars = charsBefore.toString(); + ic.commitText("", -1); + charsBefore = ic.getTextBeforeCursor(s.length()*2, 0); + Log.d(TAG, "adjusting high surrogate pair from check was: " + origChars + + ", now: " + charsBefore.toString()); + } + + // Adjust the cursor by "move" codepoints int move = CharSequenceUtil.adjustCursorPosition(charsBefore, s); if (move > 0) { Log.d(TAG, "adjusting cursor charsBefore: " + charsBefore.toString() + ", s: " + s + ", move: " + move); ic.commitText("", -move); - - // Do another check for Firefox? - charsBefore = ic.getTextBeforeCursor(s.length()*2, 0); - move = CharSequenceUtil.adjustCursorPosition(charsBefore, s); - if (move > 0) { - Log.d(TAG, "adjusting cursor 2 charsBefore: " + charsBefore.toString() + ", s: " + s + ", move: " + move); - ic.commitText("", -move); - } } } From 99ce557cf19797fcdb7083223622f9d009e4b6a5 Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Thu, 14 Nov 2019 11:44:28 +0700 Subject: [PATCH 20/27] fix(android): refactor method that removes split surrogate pair --- .../java/com/tavultesoft/kmea/KMManager.java | 114 ++++++++++-------- 1 file changed, 61 insertions(+), 53 deletions(-) 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 8a2f920eba..f9a28ad56b 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 @@ -310,50 +310,6 @@ public final class KMManager { } } - /* - // TODO: Chromium has a bug where deleteSurroundingText deletes an entire grapheme cluster - // instead of one code-point. - // 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 = ic.getTextBeforeCursor(originalBufferLength, 0); - - if (Character.isHighSurrogate(charsBackup.charAt(charsBackup.length()-1))) { - // Firefox sometimes splits a surrogate pair so move the cursor back - String origChars = charsBackup.toString(); - - ic.commitText("", -1); - charsBackup = ic.getTextBeforeCursor(originalBufferLength, 0); - Log.d(TAG, "adjusting high surrogate pair from charsBackup was: " + origChars + - ", now: " + charsBackup.toString()); - } - - 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 = ic.getTextBeforeCursor(originalBufferLength - dn, 0); - - CharSequence charsToRestore = CharSequenceUtil.restoreChars(expectedChars, newContext); - if (charsToRestore.length() > 0) { - // Restore expectedChars that Chromium deleted, and advance the cursor by expectedChars.length() - ic.commitText(charsToRestore, charsToRestore.length()); - Log.d(TAG, "performLeftDeletions commitText(" + charsToRestore.toString() + ")"); - } - } - public static void hideSystemKeyboard() { if (SystemKeyboard != null) { SystemKeyboard.hideKeyboard(); @@ -2116,15 +2072,7 @@ public final class KMManager { // After inserting the string s and adjusting the cursor count clusters, // Chrome and Firefox sometimes have the cursor too far, so // check if we need to adjust the cursor position - CharSequence charsBefore = ic.getTextBeforeCursor(s.length()*2, 0); - if (charsBefore != null && Character.isHighSurrogate(charsBefore.charAt(charsBefore.length()-1))) { - // Firefox sometimes splits a surrogate pair so move the cursor back - String origChars = charsBefore.toString(); - ic.commitText("", -1); - charsBefore = ic.getTextBeforeCursor(s.length()*2, 0); - Log.d(TAG, "adjusting high surrogate pair from check was: " + origChars + - ", now: " + charsBefore.toString()); - } + CharSequence charsBefore = getCharacterSequence(ic, s.length()*2); // Adjust the cursor by "move" codepoints int move = CharSequenceUtil.adjustCursorPosition(charsBefore, s); @@ -2143,5 +2091,65 @@ 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. + // 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 = ic.getTextBeforeCursor(originalBufferLength - dn, 0); + + CharSequence charsToRestore = CharSequenceUtil.restoreChars(expectedChars, newContext); + if (charsToRestore.length() > 0) { + // Restore expectedChars that Chromium deleted, and advance the cursor by expectedChars.length() + ic.commitText(charsToRestore, charsToRestore.length()); + Log.d(TAG, "performLeftDeletions commitText(" + charsToRestore.toString() + ")"); + } + } + + /** + * Get a character sequence from the InputConnection. + * Sometimes Firefox will split a surrogate pair, 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); + + // 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); + Log.d(TAG, "getCharacterSequence adjusting high surrogate pair from " + origChars + + ", now " + sequence.toString()); + } + + return sequence; + } } } From ce98d7ac0b84e8d1b510f302cedeaeaf953b46e7 Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Mon, 18 Nov 2019 11:45:31 +0700 Subject: [PATCH 21/27] fix(android): Remove use of BreakIterator Specifying newCursorPosition `1` fixed the cursor placement after inserting text. --- .../java/com/tavultesoft/kmea/KMManager.java | 27 +++---------------- 1 file changed, 3 insertions(+), 24 deletions(-) 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 f9a28ad56b..a4497c9ed2 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 @@ -9,7 +9,6 @@ import java.io.FileFilter; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; -import java.text.BreakIterator; import java.util.ArrayList; import java.util.HashMap; @@ -2057,29 +2056,9 @@ public final class KMManager { if (s.length() > 0) { SystemKeyboardShouldIgnoreSelectionChange = true; - // Using BreakIterator to count grapheme clusters - BreakIterator boundary = BreakIterator.getCharacterInstance(); - boundary.setText(s); - int count = 0; - while (boundary.next() != BreakIterator.DONE) { - count++; - } - - // Commit the string s and adjust the cursor by the number of grapheme clusters - ic.commitText(s, count); - Log.d(TAG, "adjusting commitText s:" + s.toString() + ", count: " + count + ", s.length: " + s.length()); - - // After inserting the string s and adjusting the cursor count clusters, - // Chrome and Firefox sometimes have the cursor too far, so - // check if we need to adjust the cursor position - CharSequence charsBefore = getCharacterSequence(ic, s.length()*2); - - // Adjust the cursor by "move" codepoints - int move = CharSequenceUtil.adjustCursorPosition(charsBefore, s); - if (move > 0) { - Log.d(TAG, "adjusting cursor charsBefore: " + charsBefore.toString() + ", s: " + s + ", move: " + move); - ic.commitText("", -move); - } + // Commit the string s. Use newCursorPosition 1 so cursor will end up after the string. + ic.commitText(s, 1); + Log.d(TAG, "adjusting commitText s:" + s.toString()); } ic.endBatchEdit(); From d82026c366afca17a85291e83251554a46ecb5e1 Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Mon, 18 Nov 2019 13:18:16 +0700 Subject: [PATCH 22/27] chore(android): Add comment to Chromium issue Also remove debugging logs --- .../main/java/com/tavultesoft/kmea/KMManager.java | 13 ++++++------- .../com/tavultesoft/kmea/util/CharSequenceUtil.java | 4 ---- 2 files changed, 6 insertions(+), 11 deletions(-) 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 a4497c9ed2..efba94c51e 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 @@ -2058,7 +2058,6 @@ public final class KMManager { // Commit the string s. Use newCursorPosition 1 so cursor will end up after the string. ic.commitText(s, 1); - Log.d(TAG, "adjusting commitText s:" + s.toString()); } ic.endBatchEdit(); @@ -2073,7 +2072,9 @@ public final class KMManager { /* // TODO: Chromium has a bug where deleteSurroundingText deletes an entire grapheme cluster - // instead of one code-point. + // 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 @@ -2099,9 +2100,9 @@ public final class KMManager { CharSequence charsToRestore = CharSequenceUtil.restoreChars(expectedChars, newContext); if (charsToRestore.length() > 0) { - // Restore expectedChars that Chromium deleted, and advance the cursor by expectedChars.length() - ic.commitText(charsToRestore, charsToRestore.length()); - Log.d(TAG, "performLeftDeletions commitText(" + charsToRestore.toString() + ")"); + // Restore expectedChars that Chromium deleted. + // Use newCusorPosition 1 so cursor will be after the inserted string + ic.commitText(charsToRestore, 1); } } @@ -2124,8 +2125,6 @@ public final class KMManager { String origChars = sequence.toString(); ic.commitText("", -1); sequence = ic.getTextBeforeCursor(length, 0); - Log.d(TAG, "getCharacterSequence adjusting high surrogate pair from " + origChars + - ", now " + sequence.toString()); } return sequence; 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 index 1045883c31..717ffbdc97 100644 --- 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 @@ -58,10 +58,6 @@ public final class CharSequenceUtil { charsToRestore = expectedChars.subSequence(index + currentContextString.length(), expectedChars.length()); } } - - Log.d(TAG, "adjusting expectedChars: " + expectedChars.toString() + - ", currentContext: " + currentContext.toString() + - ", charsToRestore: " + charsToRestore.toString()); return charsToRestore; } catch (Exception e) { Log.e(TAG, "Error in restoreChars: " + e); From e346566fecc442b197a12d961138e62d759850b7 Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Tue, 19 Nov 2019 11:21:56 +0700 Subject: [PATCH 23/27] chore(android): Update history.md --- android/history.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/android/history.md b/android/history.md index eb92593140..23dd58ec97 100644 --- a/android/history.md +++ b/android/history.md @@ -3,6 +3,13 @@ ## 13.0 alpha * Start version 13.0 +## 2019-11-17 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) From 6887aeacaba6c0bf8b91438a616b3cc794b205ea Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Tue, 19 Nov 2019 11:52:30 +0700 Subject: [PATCH 24/27] chore(android): Forgot hash --- android/history.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/android/history.md b/android/history.md index 23dd58ec97..9df9ad6f1a 100644 --- a/android/history.md +++ b/android/history.md @@ -5,7 +5,7 @@ ## 2019-11-17 12.0.4208 stable * Bug fix: - * Fix context manipulation to work around Chromium issue (2281) + * 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) From b29e112a4a3ee87710ba7cc6e4d14bd0d0a7ecd1 Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Thu, 21 Nov 2019 11:27:20 +0700 Subject: [PATCH 25/27] fix(android): Fix NPE when deleting empty context --- .../KMEA/app/src/main/java/com/tavultesoft/kmea/KMManager.java | 3 +++ android/history.md | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) 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 efba94c51e..015814e54b 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 @@ -2119,6 +2119,9 @@ public final class KMManager { } 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))) { diff --git a/android/history.md b/android/history.md index 9df9ad6f1a..8e9abe5b2c 100644 --- a/android/history.md +++ b/android/history.md @@ -3,7 +3,7 @@ ## 13.0 alpha * Start version 13.0 -## 2019-11-17 12.0.4208 stable +## 2019-11-21 12.0.4208 stable * Bug fix: * Fix context manipulation to work around Chromium issue (#2281) From 197f47438d88014f10b37e6a03847336e05b7920 Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Thu, 21 Nov 2019 13:32:24 +0700 Subject: [PATCH 26/27] fix(android): Remove split surrogate pair from beginning of context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In a scenario where the context is 8 smiley emojis followed by `p̂p̂` A backspace was removing an entire `p̂` sequence. Because the new context was crossing a surrogate pair. --- .../main/java/com/tavultesoft/kmea/KMManager.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) 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 015814e54b..2c029d6eef 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 @@ -2096,7 +2096,8 @@ public final class KMManager { // subSequence indices are start(inclusive) to end(exclusive) CharSequence expectedChars = charsBackup.subSequence(0, charsBackup.length() - (dn + numPairs)); ic.deleteSurroundingText(dn + numPairs, 0); - CharSequence newContext = ic.getTextBeforeCursor(originalBufferLength - dn, 0); + //CharSequence newContext = ic.getTextBeforeCursor(originalBufferLength - dn, 0); + CharSequence newContext = getCharacterSequence(ic, originalBufferLength - dn); CharSequence charsToRestore = CharSequenceUtil.restoreChars(expectedChars, newContext); if (charsToRestore.length() > 0) { @@ -2108,7 +2109,8 @@ public final class KMManager { /** * Get a character sequence from the InputConnection. - * Sometimes Firefox will split a surrogate pair, so chop that and update the cursor + * 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 @@ -2130,6 +2132,12 @@ public final class KMManager { 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; } } From 811eb2ac6f88963eea652ad82a5c2cd6ddf809b1 Mon Sep 17 00:00:00 2001 From: Darcy Wong Date: Thu, 21 Nov 2019 13:38:26 +0700 Subject: [PATCH 27/27] chore(android): Remove commented code and update history.md --- .../KMEA/app/src/main/java/com/tavultesoft/kmea/KMManager.java | 1 - android/history.md | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) 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 2c029d6eef..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 @@ -2096,7 +2096,6 @@ public final class KMManager { // subSequence indices are start(inclusive) to end(exclusive) CharSequence expectedChars = charsBackup.subSequence(0, charsBackup.length() - (dn + numPairs)); ic.deleteSurroundingText(dn + numPairs, 0); - //CharSequence newContext = ic.getTextBeforeCursor(originalBufferLength - dn, 0); CharSequence newContext = getCharacterSequence(ic, originalBufferLength - dn); CharSequence charsToRestore = CharSequenceUtil.restoreChars(expectedChars, newContext); diff --git a/android/history.md b/android/history.md index 8e9abe5b2c..05586fcebe 100644 --- a/android/history.md +++ b/android/history.md @@ -3,7 +3,7 @@ ## 13.0 alpha * Start version 13.0 -## 2019-11-21 12.0.4208 stable +## 2019-11-22 12.0.4208 stable * Bug fix: * Fix context manipulation to work around Chromium issue (#2281)