Merge branch 'feat/core/11072-default-modifier' into chore/core/dx-be…

…tter-embedded-test-msg

HISTORY.md

@@ -1,5 +1,18 @@
 # Keyman Version History
 
+## 17.0.297 beta 2024-03-28
+
+* fix(common): properly handle illegal UnicodeSets to prevent crash in kmc-ldml compiler (#11065)
+* fix(core,developer): variable/marker substitution in sets and strings (#11059)
+* fix(developer):  in ldml compiler, generate compiler error if `from=` regex matches empty string (#11070)
+* fix(core): calculate offset correctly when replacing marker in transform (fixes crash) (#11071)
+* feat(developer): support comma in modifiers (#11075)
+* fix(core): actions_normalize() length and dead store fix (#11100)
+* chore(core): optimize ldml_event_state::emit_difference() when no diff (#11094)
+* fix(ios): bad initial in-app layout, delayed banner, deprecated banner toggle (#10929)
+* feat(developer/compilers): better unit test for suggestion accessibility (#11085)
+* fix(core): fix pointer math in actions_normalize() (#11101)
+
 ## 17.0.296 beta 2024-03-27
 
 * fix(developer): in model compiler, give correct key to shorter prefix words when a longer, higher-frequency word is also present (#11074)

VERSION.md

@@ -1 +1 @@
-17.0.297
+17.0.298

common/include/kmx_file.h

@@ -302,7 +302,8 @@ namespace kmx {
 
 #define K_MODIFIERFLAG  0x007F
 #define K_NOTMODIFIERFLAG 0xFF00   // I4548
-#define K_DEFAULTMODFLAG  0x10000  // used by KMX+ for the default modifier
+// Note: DEFAULT_MODIFIER = 0x10000, used by KMX+ for the
+// default modifier flag in layers, > 16 bit so not available here
 
 struct COMP_STORE {
   KMX_DWORD_unaligned dwSystemID;

common/web/keyboard-processor/src/text/outputTarget.ts

@@ -13,23 +13,25 @@ import { Deadkey, DeadkeyTracker } from "./deadkeys.js";
 
 export function isEmptyTransform(transform: Transform) {
   if(!transform) {
-    return false;
+    return true;
   }
   return transform.insert === '' && transform.deleteLeft === 0 && (transform.deleteRight ?? 0) === 0;
 }
 
 export class TextTransform implements Transform {
   readonly insert: string;
   readonly deleteLeft: number;
-  readonly deleteRight?: number;
+  readonly deleteRight: number;
+  readonly erasedSelection: boolean;
 
-  constructor(insert: string, deleteLeft: number, deleteRight?: number) {
+  constructor(insert: string, deleteLeft: number, deleteRight: number, erasedSelection: boolean) {
     this.insert = insert;
     this.deleteLeft = deleteLeft;
-    this.deleteRight = deleteRight || 0;
+    this.deleteRight = deleteRight;
+    this.erasedSelection = erasedSelection;
   }
 
-  public static readonly nil = new TextTransform('', 0, 0);
+  public static readonly nil = new TextTransform('', 0, 0, false);
 }
 
 export class Transcription {
@@ -138,7 +140,7 @@ export default abstract class OutputTarget {
     // caret mid-word..
     const deletedRight = fromRight.substring(0, rightDivergenceIndex + 1)._kmwLength();
 
-    return new TextTransform(insertedText, deletedLeft, deletedRight);
+    return new TextTransform(insertedText, deletedLeft, deletedRight, original.getSelectedText() && !this.getSelectedText());
   }
 
   buildTranscriptionFrom(original: OutputTarget, keyEvent: KeyEvent, readonly: boolean, alternates?: Alternate[]): Transcription {

common/web/keyboard-processor/tests/node/transcriptions.js

@@ -448,7 +448,8 @@ but not himself.`;  // Sheev Palpatine, in the Star Wars prequels.
       assert.deepEqual(transform, {
         insert: '',
         deleteLeft: 0,
-        deleteRight: 0
+        deleteRight: 0,
+        erasedSelection: true
       });
     });
 
@@ -459,7 +460,8 @@ but not himself.`;  // Sheev Palpatine, in the Star Wars prequels.
       const transform = {
         insert: '',
         deleteLeft: 0,
-        deleteRight: 0
+        deleteRight: 0,
+        erasedSelection: true
       };
 
       target.apply(transform);

core/src/kmx/kmx_plus.cpp

@@ -42,7 +42,7 @@ static_assert(LALTFLAG == LDML_KEYS_MOD_ALTL, "LDML modifier bitfield vs. kmx_fi
 static_assert(K_ALTFLAG == LDML_KEYS_MOD_ALT, "LDML modifier bitfield vs. kmx_file.h #define mismatch");
 static_assert(CAPITALFLAG == LDML_KEYS_MOD_CAPS, "LDML modifier bitfield vs. kmx_file.h #define mismatch");
 static_assert(K_SHIFTFLAG == LDML_KEYS_MOD_SHIFT, "LDML modifier bitfield vs. kmx_file.h #define mismatch"); // "either" shift
-static_assert(K_DEFAULTMODFLAG == LDML_KEYS_MOD_DEFAULT, "LDML modifier bitfield vs. kmx_file.h #define mismatch");
+// LDML_KEYS_MOD_DEFAULT is not present in kmx_file.h (>16 bit)
 
 /**
  * \def LDML_IS_VALID_MODIFIER_BITS test whether x is a valid modifier bitfield

core/src/ldml/ldml_vkeys.cpp

@@ -7,6 +7,7 @@
 
 #include "ldml_vkeys.hpp"
 #include "kmx_file.h"
+#include <ldml/keyman_core_ldml.h>
 
 namespace km {
 namespace core {
@@ -68,7 +69,7 @@ vkeys::lookup(km_core_virtual_key vk, uint16_t modifier_state, bool &found) cons
 
   // look for a layer with "default"
   {
-    const vkey_id id_default(vk, (K_DEFAULTMODFLAG));
+    const vkey_id id_default(vk, (LDML_KEYS_MOD_DEFAULT));
     ret = lookup(id_default, found);
     if (found) {
       return ret;

core/tests/unit/kmnkbd/test_actions_normalize.cpp

@@ -457,6 +457,30 @@ void run_actions_normalize_tests() {
     /* action del, output: */            1, U"𐒻𐒷",
     /* app_context: */                   u"𐒻𐒷"
   );
+
+  km_core_context_item items_11112[] = {
+    { KM_CORE_CT_CHAR,   {0,}, { U'𐒻' } },
+    { KM_CORE_CT_CHAR,   {0,}, { U'𐒷' } },
+    KM_CORE_CONTEXT_ITEM_END
+  };
+
+  const char16_t trail_surrogate[] = {
+    0xDCBB,
+    0x0000,
+  };
+
+
+  // regression #11067
+  test_actions_normalize(
+    "A lone surrogate in context (#11112)",
+    /* app context pre transform: */     trail_surrogate,
+    /* cached context post transform: */ u"𐒻𐒷",
+    /* cached context post transform: */ &items_11112[0],
+    /* action del, output: */            0, U"𐒻𐒷",
+    // ---- results ----
+    /* action del, output: */            1, U"𐒻𐒷",
+    /* app_context: */                   u"𐒻𐒷"
+  );
 }
 
 void run_actions_update_app_context_nfu_tests() {

developer/src/kmc-ldml/src/compiler/messages.ts

@@ -5,6 +5,9 @@ const SevWarn = CompilerErrorSeverity.Warn | CompilerErrorNamespace.LdmlKeyboard
 const SevError = CompilerErrorSeverity.Error | CompilerErrorNamespace.LdmlKeyboardCompiler;
 // const SevFatal = CompilerErrorSeverity.Fatal | CompilerErrorNamespace.LdmlKeyboardCompiler;
 
+// sub-numberspace for transform errors
+const SevErrorTransform = SevError | 0xF00;
+
 /**
  * @internal
  */
@@ -49,9 +52,7 @@ export class CompilerMessages {
   static Error_GestureKeyNotFoundInKeyBag = (o:{keyId: string, parentKeyId: string, attribute: string}) =>
   m(this.ERROR_GestureKeyNotFoundInKeyBag, `Key '${def(o.keyId)}' not found in key bag, referenced from other '${def(o.parentKeyId)}' in ${def(o.attribute)}`);
 
-  static ERROR_TransformFromMatchesNothing = SevError | 0x000C;
-  static Error_TransformFromMatchesNothing = (o: { from: string }) =>
-  m(this.ERROR_TransformFromMatchesNothing, `Invalid transfom from="${def(o.from)}": Matches an empty string.`);
+  // 0x000C - available
 
   static ERROR_InvalidVersion = SevError | 0x000D;
   static Error_InvalidVersion = (o:{version: string}) =>
@@ -173,14 +174,27 @@ export class CompilerMessages {
   static Error_UnparseableReorderSet = (o: { from: string, set: string }) =>
   m(this.ERROR_UnparseableReorderSet, `Illegal UnicodeSet "${def(o.set)}" in reorder "${def(o.from)}`);
 
-  static ERROR_UnparseableTransformFrom = SevError | 0x0029;
-  static Error_UnparseableTransformFrom = (o: { from: string, message: string }) =>
-  m(this.ERROR_UnparseableTransformFrom, `Invalid transfom from "${def(o.from)}": "${def(o.message)}"`);
+  // Available: 0x029
 
   static ERROR_InvalidQuadEscape = SevError | 0x0030;
   static Error_InvalidQuadEscape = (o: { cp: number }) =>
-  m(this.ERROR_InvalidQuadEscape, `Invalid escape "\\u${util.hexQuad(o?.cp || 0)}", use "\\u{${def(o?.cp?.toString(16))}}" instead.`);
+  m(this.ERROR_InvalidQuadEscape, `Invalid escape "\\u${util.hexQuad(o?.cp || 0)}". Hint: Use "\\u{${def(o?.cp?.toString(16))}}"`);
 
-}
+  //
+  // Transform syntax errors begin at ...F00 (SevErrorTransform)
+
+  // This is a bit of a catch-all and represents messages bubbling up from the underlying regex engine
+  static ERROR_UnparseableTransformFrom   = SevErrorTransform | 0x00;
+  static Error_UnparseableTransformFrom   = (o: { from: string, message: string }) =>
+  m(this.ERROR_UnparseableTransformFrom,    `Invalid transform from="${def(o.from)}": "${def(o.message)}"`);
 
+  static ERROR_IllegalTransformDollarsign = SevErrorTransform | 0x01;
+  static Error_IllegalTransformDollarsign = (o: { from: string }) =>
+  m(this.ERROR_IllegalTransformDollarsign,  `Invalid transform from="${def(o.from)}": Unescaped dollar-sign ($) is not valid transform syntax.`,
+                                            '**Hint**: Use `\\$` to match a literal dollar-sign.');
 
+  static ERROR_TransformFromMatchesNothing = SevErrorTransform | 0x02;
+  static Error_TransformFromMatchesNothing = (o: { from: string }) =>
+  m(this.ERROR_TransformFromMatchesNothing, `Invalid transfom from="${def(o.from)}": Matches an empty string.`);
+
+}

developer/src/kmc-ldml/src/compiler/tran.ts

@@ -199,19 +199,24 @@ export abstract class TransformCompiler<T extends TransformCompilerType, TranBas
    */
   private isValidRegex(cookedFrom: string, from: string) : boolean {
     // check for any unescaped dollar sign here
-    if (/(?<!\\)\$/.test(cookedFrom)) {
-      this.callbacks.reportMessage(CompilerMessages.Error_UnparseableTransformFrom({ from, message: `Transform contains '$' but did not match any variable substitution. '$' is not valid in transform syntax.` }));
+    if (/(?<!\\)(?:\\\\)*\$/.test(cookedFrom)) {
+      this.callbacks.reportMessage(CompilerMessages.Error_IllegalTransformDollarsign({ from }));
       return false;
     }
     // Verify that the regex is syntactically valid
     try {
       const rg = new RegExp(cookedFrom + '$', 'ug');
+      // Tests against the regex:
+
       // does it match an empty string?
       if (rg.test('')) {
         this.callbacks.reportMessage(CompilerMessages.Error_TransformFromMatchesNothing({ from }));
         return false;
       }
     } catch (e) {
+      // We're exposing the internal regex error message here.
+      // In the future, CLDR plans to expose the EBNF for the transform,
+      // at which point we would have more precise validation prior to getting to this point.
       this.callbacks.reportMessage(CompilerMessages.Error_UnparseableTransformFrom({ from, message: e.message }));
       return false;
     }

developer/src/kmc-ldml/test/fixtures/sections/tran/fail-bad-from-1.xml → developer/src/kmc-ldml/test/fixtures/sections/tran/fail-IllegalTransformDollarsign-1.xml

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 
 <keyboard3 xmlns="https://schemas.unicode.org/cldr/45/keyboard3" locale="mt" conformsTo="45">
-  <info name="vars-fail-7" />
+  <info name="dollarsign-fail-1" />
 
   <keys />
 

developer/src/kmc-ldml/test/fixtures/sections/tran/fail-IllegalTransformDollarsign-2.xml

@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<keyboard3 xmlns="https://schemas.unicode.org/cldr/45/keyboard3" locale="mt" conformsTo="45">
+  <info name="dollarsign-fail-2" />
+  <keys />
+  <transforms type="simple">
+    <transformGroup>
+      <transform from="\\$" /> <!-- illegal -->
+    </transformGroup>
+  </transforms>
+
+</keyboard3>

developer/src/kmc-ldml/test/fixtures/sections/tran/fail-IllegalTransformDollarsign-3.xml

@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<keyboard3 xmlns="https://schemas.unicode.org/cldr/45/keyboard3" locale="mt" conformsTo="45">
+  <info name="dollarsign-fail-3" />
+  <keys />
+  <transforms type="simple">
+    <transformGroup>
+      <transform from="$" /> <!-- illegal -->
+    </transformGroup>
+  </transforms>
+
+</keyboard3>

developer/src/kmc-ldml/test/fixtures/sections/tran/ok-1.xml

@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<keyboard3 xmlns="https://schemas.unicode.org/cldr/45/keyboard3" locale="mt" conformsTo="45">
+  <info name="ok-1-dollarsign" />
+  <keys />
+  <transforms type="simple">
+    <transformGroup>
+      <transform from="\\\$" /> <!-- OK: dollarsign is escaped -->
+    </transformGroup>
+  </transforms>
+
+</keyboard3>

developer/src/kmc-ldml/test/test-tran.ts

@@ -335,15 +335,20 @@ describe('tran', function () {
       ],
     },
     // cases that share the same error code
-    ...[1].map(n => ({
-      subpath: `sections/tran/fail-bad-from-${n}.xml`,
+    ...[1, 2].map(n => ({
+      subpath: `sections/tran/fail-IllegalTransformDollarsign-${n}.xml`,
       errors: [
         {
-          code: CompilerMessages.ERROR_UnparseableTransformFrom,
+          code: CompilerMessages.ERROR_IllegalTransformDollarsign,
           matchMessage: /.*/,
         }
       ],
     })),
+    // successful compile
+    ...[1].map(n => ({
+      subpath: `sections/tran/ok-${n}.xml`,
+      errors: false,
+    })),
     // cases that share the same error code
     ...[1, 2, 3].map(n => ({
       subpath: `sections/tran/fail-matches-nothing-${n}.xml`,

developer/src/tike/xml/layoutbuilder/builder.js

@@ -18,8 +18,8 @@ $(function() {
   this.saveSelection = function() {
     let key = builder.selectedKey(), subKey = builder.selectedSubKey();
     return {
-      id: key ? $(key).data('id') : null,
-      subId: subKey ? $(subKey).data('id') : null
+      id: key.length ? $(key).data('id') : null,
+      subId: subKey.length ? $(subKey).data('id') : null
     };
   }
 
@@ -971,7 +971,7 @@ $(function() {
 
   $('#kbd-scroll-container').on('scroll', function () {
     const key = builder.selectedKey();
-    if(key) {
+    if(key.length) {
       builder.moveWedgesAround(key[0]);
     }
   });

linux/debian/changelog

@@ -1,8 +1,10 @@
-keyman (17.0.279-2) UNRELEASED; urgency=medium
+keyman (17.0.295-1) unstable; urgency=medium
 
   * Remove ibus-keyman.post{inst,rm} (closes: #1034040)
+  * New upstream release.
+  * Re-release to Debian
 
- -- Eberhard Beilharz <[email protected]>  Tue, 05 Mar 2024 10:41:54 +0100
+ -- Eberhard Beilharz <[email protected]>  Wed, 27 Mar 2024 17:14:32 +0100
 
 keyman (17.0.279-1) unstable; urgency=medium
 

web/src/app/webview/src/contextManager.ts

@@ -1,4 +1,4 @@
-import { type Keyboard, Mock, OutputTarget, Transcription, findCommonSubstringEndIndex } from '@keymanapp/keyboard-processor';
+import { type Keyboard, Mock, OutputTarget, Transcription, findCommonSubstringEndIndex, isEmptyTransform } from '@keymanapp/keyboard-processor';
 import { KeyboardStub } from 'keyman/engine/package-cache';
 import { ContextManagerBase, ContextManagerConfiguration } from 'keyman/engine/main';
 import { WebviewConfiguration } from './configuration.js';
@@ -41,7 +41,9 @@ export class ContextHost extends Mock {
 
       // Signal the necessary text changes to the embedding app, if it exists.
       if(this.oninserttext) {
-        this.oninserttext(transform.deleteLeft, transform.insert, transform.deleteRight);
+        if(!isEmptyTransform(transform) || transform.erasedSelection) {
+          this.oninserttext(transform.deleteLeft, transform.insert, transform.deleteRight);
+        }
       }
     }