chore(web): Merge branch 'refactor/web/layer-group-encapsulation' into feat/web/vkbd-layout-optimization

HISTORY.md

@@ -1,5 +1,15 @@
 # Keyman Version History
 
+## 17.0.304 beta 2024-04-09
+
+* fix(android): atomically updates selection with text (#11188)
+*  (#11178)
+
+## 17.0.303 beta 2024-04-05
+
+* fix(windows): decode uri for Package ID and filename (#11152)
+* fix(common/models): suggestion stability after multiple whitespaces (#11164)
+
 ## 17.0.302 beta 2024-04-04
 
 * fix(mac): load only 80 characters from context when processing keystrokes (#11141)

VERSION.md

@@ -1 +1 @@
-17.0.303
+17.0.305

android/KMAPro/kMAPro/src/main/java/com/keyman/android/SystemKeyboard.java

@@ -130,7 +130,7 @@ public class SystemKeyboard extends InputMethodService implements OnKeyboardEven
   @Override
   public void onUpdateSelection(int oldSelStart, int oldSelEnd, int newSelStart, int newSelEnd, int candidatesStart, int candidatesEnd) {
     super.onUpdateSelection(oldSelStart, oldSelEnd, newSelStart, newSelEnd, candidatesStart, candidatesEnd);
-    KMManager.updateSelectionRange(KMManager.KeyboardType.KEYBOARD_TYPE_SYSTEM, newSelStart, newSelEnd);
+    KMManager.updateSelectionRange(KMManager.KeyboardType.KEYBOARD_TYPE_SYSTEM);
   }
 
   /**
@@ -170,9 +170,7 @@ public class SystemKeyboard extends InputMethodService implements OnKeyboardEven
       ExtractedText icText = ic.getExtractedText(new ExtractedTextRequest(), 0);
       if (icText != null) {
         boolean didUpdateText = KMManager.updateText(KeyboardType.KEYBOARD_TYPE_SYSTEM, icText.text.toString());
-        int selStart = icText.startOffset + icText.selectionStart;
-        int selEnd = icText.startOffset + icText.selectionEnd;
-        boolean didUpdateSelection = KMManager.updateSelectionRange(KeyboardType.KEYBOARD_TYPE_SYSTEM, selStart, selEnd);
+        boolean didUpdateSelection = KMManager.updateSelectionRange(KeyboardType.KEYBOARD_TYPE_SYSTEM);
         if (!didUpdateText || !didUpdateSelection)
           exText = icText;
       }

android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java

@@ -163,7 +163,7 @@ final class KMKeyboard extends WebView {
     return result;
   }
 
-  protected boolean updateSelectionRange(int selStart, int selEnd) {
+  protected boolean updateSelectionRange() {
     boolean result = false;
     InputConnection ic = KMManager.getInputConnection(this.keyboardType);
     if (ic != null) {
@@ -175,6 +175,16 @@ final class KMKeyboard extends WebView {
       String rawText = icText.text.toString();
       updateText(rawText.toString());
 
+      int selStart = icText.selectionStart;
+      int selEnd = icText.selectionEnd;
+
+      int selMin = selStart, selMax = selEnd;
+      if (selStart > selEnd) {
+        // Selection is reversed so "swap"
+        selMin = selEnd;
+        selMax = selStart;
+      }
+
       /*
         The values of selStart & selEnd provided by the system are in code units,
         not code-points.  We need to account for surrogate pairs here.
@@ -193,8 +203,8 @@ final class KMKeyboard extends WebView {
 
       selStart -= pairsAtStart;
       selEnd -= (pairsAtStart + pairsSelected);
+      this.loadJavascript(KMString.format("updateKMSelectionRange(%d,%d)", selStart, selEnd));
     }
-    this.loadJavascript(KMString.format("updateKMSelectionRange(%d,%d)", selStart, selEnd));
     result = true;
 
     return result;

android/KMEA/app/src/main/java/com/keyman/engine/KMManager.java

@@ -2137,23 +2137,40 @@ public final class KMManager {
     return result;
   }
 
+  /**
+   * Updates the active range for selected text.
+   * @deprecated
+   * This method no longer needs the `selStart` and `selEnd` parameters.
+   * <p>Use {@link KMManager#updateSelectionRange(KeyboardType)} instead.</p>
+   *
+   * @param kbType    A value indicating if this request is for the in-app keyboard or the system keyboard
+   * @param selStart  (deprecated) the start index for the range
+   * @param selEnd  (deprecated) the end index for the selected range
+   * @return
+   */
+  @Deprecated
   public static boolean updateSelectionRange(KeyboardType kbType, int selStart, int selEnd) {
+    return updateSelectionRange(kbType);
+  }
+
+  /**
+   * Performs a synchronization check for the active range for selected text,
+   * ensuring it matches the text-editor's current state.
+   * @param kbType  A value indicating if this request is for the in-app or system keyboard.
+   * @return
+   */
+  public static boolean updateSelectionRange(KeyboardType kbType) {
     boolean result = false;
-    int selMin = selStart, selMax = selEnd;
-    if (selStart > selEnd) {
-      // Selection is reversed so "swap"
-      selMin = selEnd;
-      selMax = selStart;
-    }
+
     if (kbType == KeyboardType.KEYBOARD_TYPE_INAPP) {
       if (isKeyboardLoaded(KeyboardType.KEYBOARD_TYPE_INAPP) && !InAppKeyboard.shouldIgnoreSelectionChange()) {
-        result = InAppKeyboard.updateSelectionRange(selMin, selMax);
+        result = InAppKeyboard.updateSelectionRange();
       }
 
       InAppKeyboard.setShouldIgnoreSelectionChange(false);
     } else if (kbType == KeyboardType.KEYBOARD_TYPE_SYSTEM) {
       if (isKeyboardLoaded(KeyboardType.KEYBOARD_TYPE_SYSTEM) && !SystemKeyboard.shouldIgnoreSelectionChange()) {
-        result = SystemKeyboard.updateSelectionRange(selMin, selMax);
+        result = SystemKeyboard.updateSelectionRange();
       }
 
       SystemKeyboard.setShouldIgnoreSelectionChange(false);

android/KMEA/app/src/main/java/com/keyman/engine/KMTextView.java

@@ -63,10 +63,8 @@ public final class KMTextView extends AppCompatEditText {
    */
   public static void updateTextContext() {
     KMTextView textView = (KMTextView) activeView;
-    int selStart = textView.getSelectionStart();
-    int selEnd = textView.getSelectionEnd();
     KMManager.updateText(KeyboardType.KEYBOARD_TYPE_INAPP, textView.getText().toString());
-    if (KMManager.updateSelectionRange(KeyboardType.KEYBOARD_TYPE_INAPP, selStart, selEnd)) {
+    if (KMManager.updateSelectionRange(KeyboardType.KEYBOARD_TYPE_INAPP)) {
       KMManager.resetContext(KeyboardType.KEYBOARD_TYPE_INAPP);
     }
   }
@@ -167,7 +165,7 @@ public final class KMTextView extends AppCompatEditText {
   protected void onSelectionChanged(int selStart, int selEnd) {
     super.onSelectionChanged(selStart, selEnd);
     if (activeView != null && activeView.equals(this)) {
-      if (KMManager.updateSelectionRange(KMManager.KeyboardType.KEYBOARD_TYPE_INAPP, selStart, selEnd)) {
+      if (KMManager.updateSelectionRange(KMManager.KeyboardType.KEYBOARD_TYPE_INAPP)) {
         KMManager.resetContext(KeyboardType.KEYBOARD_TYPE_INAPP);
       }
     }

common/web/gesture-recognizer/src/engine/headless/asyncClosureDispatchQueue.ts

@@ -36,51 +36,53 @@ export class AsyncClosureDispatchQueue {
     return this.queue.length == 0 && !this.waitLock;
   }
 
-  private async setWaitLock(promise: Promise<any>) {
-    this.waitLock = promise;
+  private async triggerNextClosure() {
+    if(this.queue.length == 0) {
+      return;
+    }
+
+    const functor = this.queue.shift();
+
+    // A stand-in so that `ready` doesn't report true while the closure runs.
+    this.waitLock = Promise.resolve();
+
+    /*
+      It is imperative that any errors triggered by the functor do not prevent this method from setting
+      the wait lock that will trigger the following event (if it exists).  Failure to do so will
+      result in all further queued closures never getting the opportunity to run!
+    */
+    let result: undefined | Promise<any>;
+    try {
+      // Is either undefined (return type: void) or is a Promise.
+      result = functor() as undefined | Promise<any>;
+      /* c8 ignore start */
+    } catch (err) {
+      reportError('Error from queued closure', err);
+    }
+    /* c8 ignore end */
+
+    /*
+      Replace the stand-in with the _true_ post-closure wait.
+
+      If the closure returns a Promise, the implication is that the further processing of queued
+      functions should be blocked until that Promise is fulfilled.
+
+      If not, we just add a default delay.
+    */
+    result = result ?? this.defaultWaitFactory();
+    this.waitLock = result;
 
     try {
-      await promise;
+      await result;
     } catch(err) {
       reportError('Async error from queued closure', err);
     }
 
     this.waitLock = null;
+    // if queue is length zero, auto-returns.
     this.triggerNextClosure();
   }
 
-  private async triggerNextClosure() {
-    if(this.queue.length > 0) {
-      const functor = this.queue.shift();
-
-      // A stand-in so that `ready` doesn't report true while the closure runs.
-      this.waitLock = Promise.resolve();
-
-      /*
-        It is imperative that any errors triggered by the functor do not prevent this method from setting
-        the wait lock that will trigger the following event (if it exists).  Failure to do so will
-        result in all further queued closures never getting the opportunity to run!
-      */
-     let result: undefined | Promise<any>;
-      try {
-        // Is either undefined (return type: void) or is a Promise.
-        result = functor() as undefined | Promise<any>;
-        /* c8 ignore start */
-      } catch (err) {
-        reportError('Error from queued closure', err);
-      }
-      /* c8 ignore end */
-
-      /*
-        If the closure returns a Promise, the implication is that the further processing of queued
-        functions should be blocked until that Promise is fulfilled.
-
-        If not, we still add a delay according to the specified default.
-      */
-      this.setWaitLock(result ?? this.defaultWaitFactory());
-    }
-  }
-
   runAsync(closure: QueueClosure) {
     // Check before putting the closure on the internal queue; the check is based in part
     // upon the existing queue length.

common/web/gesture-recognizer/src/engine/headless/gestures/matchers/gestureMatcher.ts

@@ -157,14 +157,14 @@ export class GestureMatcher<Type, StateToken = any> implements PredecessorMatch<
           return;
         case 'full':
           contact = srcContact.constructSubview(false, true);
-          this.addContactInternal(contact, srcContact.path.stats);
+          this.addContactInternal(contact, srcContact.path.stats, true);
           continue;
         case 'partial':
           preserveBaseItem = true;
           // Intentional fall-through
         case 'chop':
           contact = srcContact.constructSubview(true, preserveBaseItem);
-          this.addContactInternal(contact, srcContact.path.stats);
+          this.addContactInternal(contact, srcContact.path.stats, true);
           break;
       }
     }
@@ -367,7 +367,7 @@ export class GestureMatcher<Type, StateToken = any> implements PredecessorMatch<
     return this._result;
   }
 
-  private addContactInternal(simpleSource: GestureSourceSubview<Type>, basePathStats: CumulativePathStats<Type>) {
+  private addContactInternal(simpleSource: GestureSourceSubview<Type>, basePathStats: CumulativePathStats<Type>, whileInitializing?: boolean) {
     // The number of already-active contacts tracked for this gesture
     const existingContacts = this.pathMatchers.length;
 
@@ -491,16 +491,22 @@ export class GestureMatcher<Type, StateToken = any> implements PredecessorMatch<
     const result = contactModel.update();
     if(result?.type == 'reject') {
       /*
-        Refer to the earlier comment in this method re: use of 'cancelled'; we need to
-        prevent any and all further attempts to match against this model  We'd
-        instantly reject it anyway due to its rejected initial state.  Failing to do so
-        can cause an infinite async loop.
+        Refer to the earlier comment in this method re: use of 'cancelled'; we
+        need to prevent any and all further attempts to match against this model
+        We'd instantly reject it anyway due to its rejected initial state.
+        Failing to do so can cause an infinite async loop.
 
-        If we weren't using 'cancelled', 'path' would correspond best with a rejection
-        here, as the decision is made due to the GestureSource's current path being
-        rejected by one of the `PathModel`s comprising the `GestureModel`.
+        If we weren't using 'cancelled', 'path' would correspond best with a
+        rejection here, as the decision is made due to the GestureSource's
+        current path being rejected by one of the `PathModel`s comprising the
+        `GestureModel`.
+
+        If the model's already been initialized, it's possible that a _new_
+        incoming touch needs special handling.  We'll allow one reset.  In the
+        case that it would try to restart itself, the restarted model will
+        instantly fail and thus cancel.
       */
-      this.finalize(false, 'cancelled');
+      this.finalize(false, whileInitializing ? 'cancelled' : 'path');
     }
 
     // Standard path:  trigger either resolution or rejection when the contact model signals either.

core/src/km_core_state_context_set_if_needed.cpp

@@ -15,6 +15,7 @@
 #include "state.hpp"
 #include "debuglog.h"
 #include "core_icu.h"
+#include "kmx/kmx_xstring.h" // for Unicode routines
 
 using namespace km::core;
 
@@ -59,6 +60,12 @@ km_core_state_context_set_if_needed(
     return KM_CORE_CONTEXT_STATUS_INVALID_ARGUMENT;
   }
 
+  // if the app context begins with a trailing surrogate,
+  // skip over it.
+  if (Uni_IsSurrogate2(*new_app_context)) {
+    new_app_context++;
+  }
+
   auto app_context    = km_core_state_app_context(state);
   auto cached_context = km_core_state_context(state);
 

core/tests/unit/ldml/test_context_normalization.cpp

@@ -119,11 +119,23 @@ void test_context_normalization_invalid_unicode() {
   teardown();
 }
 
+void test_context_normalization_lone_trailing_surrogate() {
+                                   // unpaired trail surrogate
+  km_core_cp const application_context[] = { 0xDC01, 0x0020, 0x0020, 0x0000 };
+  km_core_cp const cached_context[] = /* skipped*/ { 0x0020, 0x0020, 0x0000 };
+  setup("k_001_tiny.kmx");
+  assert(km_core_state_context_set_if_needed(test_state, application_context) == KM_CORE_CONTEXT_STATUS_UPDATED);
+  assert(is_identical_context(application_context+1, KM_CORE_DEBUG_CONTEXT_APP)); // first code unit is skipped
+  assert(is_identical_context(cached_context, KM_CORE_DEBUG_CONTEXT_CACHED));
+  teardown();
+}
+
 void test_context_normalization() {
   test_context_normalization_already_nfd();
   test_context_normalization_basic();
   test_context_normalization_hefty();
-  // TODO: we need to strip illegal chars: test_context_normalization_invalid_unicode(); // -- unpaired surrogate, illegals
+  // TODO: see #10392 we need to strip illegal chars: test_context_normalization_invalid_unicode(); // -- unpaired surrogate, illegals
+  test_context_normalization_lone_trailing_surrogate();
 }
 
 //-------------------------------------------------------------------------------------

web/src/engine/osk/src/keyboard-layout/oskLayer.ts

@@ -89,12 +89,12 @@ export default class OSKLayer {
 
     if(this.spaceBarKey) {
       const spacebarLabel = this.spaceBarKey.label;
-      let tParent = <HTMLElement>spacebarLabel.parentNode;
+      let tButton = this.spaceBarKey.btn;
 
-      if (typeof (tParent.className) == 'undefined' || tParent.className == '') {
-        tParent.className = 'kmw-spacebar';
-      } else if (tParent.className.indexOf('kmw-spacebar') == -1) {
-        tParent.className += ' kmw-spacebar';
+      if (typeof (tButton.className) == 'undefined' || tButton.className == '') {
+        tButton.className = 'kmw-spacebar';
+      } else if (tButton.className.indexOf('kmw-spacebar') == -1) {
+        tButton.className += ' kmw-spacebar';
       }
 
       if (spacebarLabel.className != 'kmw-spacebar-caption') {

web/src/engine/osk/src/keyboard-layout/oskLayerGroup.ts

@@ -206,7 +206,7 @@ export default class OSKLayerGroup {
       Assumes there is no fine-tuning of the row ranges to be done - each takes a perfect
       fraction of the overall layer height without any padding above or below.
     */
-    const rowIndex = Math.floor(proportionalCoords.y * layer.rows.length);
+    const rowIndex = Math.max(0, Math.min(layer.rows.length-1, Math.floor(proportionalCoords.y * layer.rows.length)));
     const row = layer.rows[rowIndex];
 
     // Assertion:  row no longer `null`.