From c389ad0df8176ef1550e05f85da51a220bc9ff26 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Tue, 22 Feb 2022 10:35:25 +1100 Subject: [PATCH 1/5] fix(android): selection on Android Part of #5853. Building on the fixes in KeymanWeb, we do similar patches for Android. --- .../KMEA/app/src/main/assets/android-host.js | 56 +++++++++++++------ .../java/com/tavultesoft/kmea/KMManager.java | 49 ++++++++++------ 2 files changed, 71 insertions(+), 34 deletions(-) diff --git a/android/KMEA/app/src/main/assets/android-host.js b/android/KMEA/app/src/main/assets/android-host.js index 2e614fc8f7..172c0459f3 100644 --- a/android/KMEA/app/src/main/assets/android-host.js +++ b/android/KMEA/app/src/main/assets/android-host.js @@ -1,3 +1,10 @@ +let _debug = 0; + +// Android harness attachment +if(window.parent && window.parent.jsInterface && !window.jsInterface) { + window.jsInterface = window.parent.jsInterface; +} + var device = window.jsInterface.getDeviceType(); var oskHeight = Math.ceil(window.jsInterface.getKeyboardHeight() / window.devicePixelRatio); var oskWidth = 0; @@ -19,7 +26,7 @@ function init() { //window.console.log('Device type = '+device); //window.console.log('Keyboard height = '+oskHeight); var kmw=com.keyman.singleton; - kmw.init({'app':device,'fonts':'packages/'}); + kmw.init({'app':device,'fonts':'packages/',root:'./'}); kmw['util']['setOption']('attachType','manual'); kmw['oninserttext'] = insertText; kmw['showKeyboardList'] = showMenu; @@ -48,6 +55,7 @@ function init() { } function notifyHost(event, params) { + console_debug('notifyHost(event='+event+',params='+params+')'); // TODO: Update all other host notifications to use notifyHost instead of directly setting window.location.hash window.setTimeout(function() { // We use a timeout so that the navigation doesn't cause the calling function to abort after the call @@ -142,6 +150,7 @@ function setSpacebarText(mode) { * @param dr Number of post-caret code points to delete. (optional) */ function insertText(dn, s, dr) { + console_debug('insertText(dn='+dn+',s='+s+',dr='+dr+')'); dr = dr || 0; // Sets a default value of zero when dr is undefined //window.console.log('insertText('+ dn +', ' + s +', ' + dr + ');'); window.jsInterface.insertText(dn, s, dr); @@ -189,29 +198,43 @@ function setNumericLayer() { } function updateKMText(text) { + var ta = document.getElementById('ta'); + console_debug('updateKMText(text='+text+') ta.value='+ta.value); + if(text == undefined) { text = ''; } - var ta = document.getElementById('ta'); - var kmw = window['keyman']; - var resetContext = ta.value != text; - ta.value = text; - kmw['setActiveElement'](ta); - if(resetContext) { - kmw.resetContext(); + if(ta.value != text) { + ta.value = text; + window.resetContext(); + } +} + +function console_debug(s) { + if(_debug) { + console.debug(s); } } function updateKMSelectionRange(start, end) { var ta = document.getElementById('ta'); - var kmw = window['keyman']; - var resetContext = (ta.selectionStart != start || ta.selectionEnd != end); - ta.selectionStart = ta._KeymanWebSelectionStart = start; - ta.selectionEnd = ta._KeymanWebSelectionEnd = end; - kmw['setActiveElement'](ta); - if(resetContext) { - kmw.resetContext(); + console_debug('updateKMSelectionRange('+start+','+end+'): ta.selectionStart='+ta.selectionStart+' '+ + '['+ta._KeymanWebSelectionStart+'] ta.selectionEnd='+ta.selectionEnd+' '+ta._KeymanWebSelectionEnd); + + var selDirection = 'forward'; + if(start > end) { + let e0 = end; + end = start; + start = e0; + selDirection = 'backward'; + } + + if(ta.selectionStart != start || ta.selectionEnd != end || ta.selectionDirection != selDirection) { + ta.selectionStart = ta._KeymanWebSelectionStart = start; + ta.selectionEnd = ta._KeymanWebSelectionEnd = end; + ta.selectionDirection = selDirection; + keyman.resetContext(); } } @@ -301,6 +324,7 @@ function executePopupKey(keyID, keyText) { } function executeHardwareKeystroke(code, shift, lstates, eventModifiers) { + console_debug('executeHardwareKeystroke(code='+code+',shift='+shift+',lstates='+lstates+',eventModifiers='+eventModifiers+')'); var kmw=window['keyman']; //window.console.log('executeHardwareKeystroke:('+code+', ' + shift + ', ' + lstates + ');'); try { @@ -342,7 +366,7 @@ function toHex(theString) { * Reference: Issue #5376 */ function checkTextArea() { - var uaRe = /Chrome\/([0-9]*)./g; + var uaRe = /Chrome\/([0-9]*)\./g; var chromeMajorVersion = uaRe.exec(navigator.userAgent); if (chromeMajorVersion && parseInt(chromeMajorVersion[1]) <= 37) { var ta = document.getElementById('ta'); 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 54d470f47d..75ba10a71e 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 @@ -2683,17 +2683,24 @@ public final class KMManager { start = temp; } - if (dn <= 0) { + int deleteLeft = dn; + + if(start != end && dn == 1 && s.length() == 0) { + /* Handle backspace with a selection: just delete selection */ + deleteLeft = 0; + } + + if (deleteLeft <= 0) { if (start == end) { if (s.length() > 0 && s.charAt(0) == '\n') { textView.keyDownUp(KeyEvent.KEYCODE_ENTER); - } else { - // *** TO DO: Try to find a solution to the bug on API < 17, insert overwrites on next line - if (s.length() > 0) { + } else if (s.length() > 0) { + // *** TO DO: Try to find a solution to the bug on API < 17, insert overwrites on next line InAppKeyboardShouldIgnoreTextChange = true; InAppKeyboardShouldIgnoreSelectionChange = true; textView.getText().insert(start, s); - } + } else { + textView.getText().delete(start, end); } } else { if (s.length() > 0 && s.charAt(0) == '\n') { @@ -2712,21 +2719,25 @@ public final class KMManager { } } } else { - for (int i = 0; i < dn; i++) { - CharSequence chars = textView.getText().subSequence(0, start); - if (chars != null && chars.length() > 0) { - char c = chars.charAt(start - 1); - InAppKeyboardShouldIgnoreTextChange = true; - InAppKeyboardShouldIgnoreSelectionChange = true; - if (Character.isLowSurrogate(c)) { - textView.getText().delete(start - 2, end); - } else { - textView.getText().delete(start - 1, end); - } + if(start == end) { + for (int i = 0; i < deleteLeft; i++) { + CharSequence chars = textView.getText().subSequence(0, start); + if (chars != null && chars.length() > 0) { + char c = chars.charAt(start - 1); + InAppKeyboardShouldIgnoreTextChange = true; + InAppKeyboardShouldIgnoreSelectionChange = true; + if (Character.isLowSurrogate(c)) { + textView.getText().delete(start - 2, end); + } else { + textView.getText().delete(start - 1, end); + } - start = textView.getSelectionStart(); - end = textView.getSelectionEnd(); + start = textView.getSelectionStart(); + end = textView.getSelectionEnd(); + } } + } else { + Log.w(TAG, "Unexpected request to selection"); } if (s.length() > 0) { @@ -2736,6 +2747,8 @@ public final class KMManager { } } + // Collapse the selection + textView.setSelection(start + s.length()); textView.endBatchEdit(); } }); From 9ba4b9645c4f8a23250e91e7dcc93a67aaeb9e48 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 23 Feb 2022 15:07:58 +1100 Subject: [PATCH 2/5] fix(android): further selection tweaks --- .../KMEA/app/src/main/assets/android-host.js | 4 +- .../java/com/tavultesoft/kmea/KMManager.java | 59 ++++++++++++------- 2 files changed, 40 insertions(+), 23 deletions(-) diff --git a/android/KMEA/app/src/main/assets/android-host.js b/android/KMEA/app/src/main/assets/android-host.js index 172c0459f3..21da042c5a 100644 --- a/android/KMEA/app/src/main/assets/android-host.js +++ b/android/KMEA/app/src/main/assets/android-host.js @@ -1,4 +1,4 @@ -let _debug = 0; +var _debug = 1; // Android harness attachment if(window.parent && window.parent.jsInterface && !window.jsInterface) { @@ -224,7 +224,7 @@ function updateKMSelectionRange(start, end) { var selDirection = 'forward'; if(start > end) { - let e0 = end; + var e0 = end; end = start; start = e0; selDirection = 'backward'; 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 75ba10a71e..cd5c07aace 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 @@ -2719,25 +2719,29 @@ public final class KMManager { } } } else { - if(start == end) { - for (int i = 0; i < deleteLeft; i++) { - CharSequence chars = textView.getText().subSequence(0, start); - if (chars != null && chars.length() > 0) { - char c = chars.charAt(start - 1); - InAppKeyboardShouldIgnoreTextChange = true; - InAppKeyboardShouldIgnoreSelectionChange = true; - if (Character.isLowSurrogate(c)) { - textView.getText().delete(start - 2, end); - } else { - textView.getText().delete(start - 1, end); - } - - start = textView.getSelectionStart(); - end = textView.getSelectionEnd(); + if(start != end) { + // Delete the selection + InAppKeyboardShouldIgnoreTextChange = true; + InAppKeyboardShouldIgnoreSelectionChange = true; + textView.getText().delete(start, end); + textView.setSelection(start); + end = start; + } + for (int i = 0; i < deleteLeft; i++) { + CharSequence chars = textView.getText().subSequence(0, start); + if (chars != null && chars.length() > 0) { + char c = chars.charAt(start - 1); + InAppKeyboardShouldIgnoreTextChange = true; + InAppKeyboardShouldIgnoreSelectionChange = true; + if (Character.isLowSurrogate(c)) { + textView.getText().delete(start - 2, end); + } else { + textView.getText().delete(start - 1, end); } + + start = textView.getSelectionStart(); + end = textView.getSelectionEnd(); } - } else { - Log.w(TAG, "Unexpected request to selection"); } if (s.length() > 0) { @@ -2802,6 +2806,7 @@ public final class KMManager { // This annotation is required in Jelly Bean and later: @JavascriptInterface public void insertText(final int dn, final String s, final int dr) { + // TODO: Unify in-app and system insertText Handler mainLoop = new Handler(Looper.getMainLooper()); mainLoop.post(new Runnable() { public void run() { @@ -2826,11 +2831,19 @@ public final class KMManager { ic.beginBatchEdit(); + int deleteLeft = dn; + // Delete any existing selected text. ExtractedText icText = ic.getExtractedText(new ExtractedTextRequest(), 0); if (icText != null) { // This can be null if the input connection becomes invalid. int start = icText.startOffset + icText.selectionStart; int end = icText.startOffset + icText.selectionEnd; + if (end < start) { + // Swap start/end for backward selection + int temp = start; + start = end; + end = temp; + } if (end > start) { if (s.length() == 0) { ic.setSelection(start, start); @@ -2842,6 +2855,10 @@ public final class KMManager { ic.setSelection(start, start); ic.deleteSurroundingText(0, end - start); } + + // KeymanWeb tells us how to delete the selection, but we don't + // want to do that twice + deleteLeft = 0; } } @@ -2852,8 +2869,8 @@ public final class KMManager { } // Perform left-deletions - if (dn > 0) { - performLeftDeletions(ic, dn); + if (deleteLeft > 0) { + performLeftDeletions(ic, deleteLeft); } // Perform right-deletions @@ -2888,8 +2905,8 @@ public final class KMManager { } /* - // TODO: Chromium has a bug where deleteSurroundingText deletes an entire grapheme cluster - // instead of one code-point. See Chromium issue #1024738 + // Chromium up until version M81 had a bug where deleteSurroundingText deletes an entire + // grapheme cluster instead of one code-point. See Chromium issue #1024738 // https://bugs.chromium.org/p/chromium/issues/detail?id=1024738 // // We'll retrieve up to (dn*2+16) characters before the cursor to collect enough characters From e7be5f62a456addabe97f941f15a02da23e82f3d Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Fri, 25 Feb 2022 16:04:05 +1100 Subject: [PATCH 3/5] fix(android): ignore delete-left if selection exists in-app If there is an active selection, we delete the selection before inserting text. In this case, we also ignore the deleteLeft value coming from KeymanWeb, so we won't delete twice. Note: in the future, we will hopefully refactor this to use OutputTarget, which will change the way this function works and will probably clean up some of the spaghetti. --- .../KMEA/app/src/main/java/com/tavultesoft/kmea/KMManager.java | 1 + 1 file changed, 1 insertion(+) 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 cd5c07aace..92564dd9f1 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 @@ -2726,6 +2726,7 @@ public final class KMManager { textView.getText().delete(start, end); textView.setSelection(start); end = start; + deleteLeft = 0; } for (int i = 0; i < deleteLeft; i++) { CharSequence chars = textView.getText().subSequence(0, start); From e0bba0da02ee4fe9e9fdf89858a32ec0e34d189f Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Fri, 25 Feb 2022 16:05:51 +1100 Subject: [PATCH 4/5] fix(android): count number of surrogate pairs correctly when deleting The system keyboard, when deleting-left, would not count the number of surrogate pairs correctly in the text to delete. This would often leave it deleting half a surrogate pair. Rather than change the countSurrogatePairs function, I opted to write this inline. The countSurrogatePairs function is used elsewhere, it appears correctly. --- .../main/java/com/tavultesoft/kmea/KMManager.java | 12 +++++++++++- 1 file changed, 11 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 92564dd9f1..963c74d9a7 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 @@ -2925,7 +2925,17 @@ public final class KMManager { return; } - int numPairs = CharSequenceUtil.countSurrogatePairs(charsBackup, dn); + // Count the number of characters which are surrogate pairs + int index = lastIndex, dnx = dn, numPairs = 0; + while(index > 0 && dnx > 0) { + if(Character.isLowSurrogate(charsBackup.charAt(index)) && + Character.isHighSurrogate(charsBackup.charAt(index-1))) { + numPairs++; + index--; + } + index--; + dnx--; + } // Chop dn+numPairs code points from the end of charsBackup // subSequence indices are start(inclusive) to end(exclusive) From db9822147fe1a54e8a564d240d5ca1d56dba75e2 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Mon, 28 Feb 2022 15:06:32 +1100 Subject: [PATCH 5/5] chore(android): disable debugging --- android/KMEA/app/src/main/assets/android-host.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/android/KMEA/app/src/main/assets/android-host.js b/android/KMEA/app/src/main/assets/android-host.js index 21da042c5a..d86c6d158c 100644 --- a/android/KMEA/app/src/main/assets/android-host.js +++ b/android/KMEA/app/src/main/assets/android-host.js @@ -1,4 +1,4 @@ -var _debug = 1; +var _debug = 0; // Android harness attachment if(window.parent && window.parent.jsInterface && !window.jsInterface) {