diff --git a/.github/workflows/deb-packaging.yml b/.github/workflows/deb-packaging.yml index 8752fe316db..a3e7ee2a498 100644 --- a/.github/workflows/deb-packaging.yml +++ b/.github/workflows/deb-packaging.yml @@ -151,6 +151,16 @@ jobs: ref: '${{ github.event.client_payload.buildSha }}' sparse-checkout: '.github/actions/' + binary_packages_unreleased: + name: Build binary packages for next Ubuntu version + needs: sourcepackage + strategy: + fail-fast: true + matrix: + dist: [noble] + + runs-on: ubuntu-latest + steps: - name: Build uses: ./.github/actions/build-binary-packages with: diff --git a/HISTORY.md b/HISTORY.md index 9a9785984bb..f5935dc02fa 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -75,6 +75,81 @@ * chore(common): move to 18.0 alpha (#10713) * chore: move to 18.0 alpha +## 17.0.306 beta 2024-04-11 + +* docs(ios): updates iOS app help for 17.0 banner changes (#11200) +* chore(oem/fv): Add fv sguuxs and update keyboard versions (#11198) + +## 17.0.305 beta 2024-04-10 + +* fix(core): skip leading trail surrogate char in km_core_state_context_set_if_needed() (#11169) +* change(web): merges split async method in gesture engine (#11142) +* fix(web): blocks nextLayer for keys quickly typed when multitapping to new layer when final tap is held (#11189) +* refactor(web): OSK spacebar-label updates now managed by layer object (#11175) + +## 17.0.304 beta 2024-04-09 + +* fix(android): atomically updates selection with text (#11188) +* fix(web): fix crash in nearest-key row lookup when touch moves out-of-bounds (#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) + +## 17.0.301 beta 2024-04-03 + +* feat(core): support modifiers=other (#11118) +* chore(core): dx better err message on embedded test vkeys (#11119) +* fix(web): key preview stickiness 🪠 (#10778) +* fix(web): early gesture-match abort when unable to extend existing gestures 🪠 (#10836) +* fix(web): infinite model-match replacement looping 🪠 (#10838) +* fix(web): proper gesture-match sequencing 🪠 (#10840) +* change(web): input-event sequentialization 🪠 (#10843) +* fix(web): proper linkage of sources to events 🪠 (#10960) +* fix(developer): handle buffer boundaries in four cases (#11137) +* chore(linux): Build packages for next Ubuntu version separately (#11153) +* fix(common): upgrade sentry-cli to 2.31.0 (#11151) +* fix(android/app): Track previous device orientation for SystemKeyboard (#11134) +* change(web): reworks nearest-key detection to avoid layout reflow (#11129) + +## 17.0.300 beta 2024-04-02 + +* change(web): keyboard swaps keep original keyboards active until fully ready (#11108) +* fix(android/engine): Swap selection range if reversed (#11127) +* test(developer): keyboard info compiler unit tests (#11000) + +## 17.0.299 beta 2024-04-01 + +* fix(ios): address crash by reading full code point rather than code unit when trimming initial directional-mark (#11113) +* fix(mac): delete correct number of characters from current context when processing BMP or SMP deletes (#11086) +* feat(developer): disallow stray dollarsign in from pattern (#11117) + +## 17.0.298 beta 2024-03-29 + +* chore(linux): Update debian changelog (#11096) +* fix(web): prevent layer switch key from erasing selection (#11032) +* fix(developer): prevent error when scrolling touch layout editor with no selected key (#11109) +* fix(common): make `isEmptyTransform` return true if passed a nullish transform (#11110) + +## 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) diff --git a/android/KMAPro/kMAPro/src/main/java/com/keyman/android/SystemKeyboard.java b/android/KMAPro/kMAPro/src/main/java/com/keyman/android/SystemKeyboard.java index 7e705869308..85c14dfcb96 100644 --- a/android/KMAPro/kMAPro/src/main/java/com/keyman/android/SystemKeyboard.java +++ b/android/KMAPro/kMAPro/src/main/java/com/keyman/android/SystemKeyboard.java @@ -42,6 +42,7 @@ public class SystemKeyboard extends InputMethodService implements OnKeyboardEven private static View inputView = null; private static ExtractedText exText = null; private KMHardwareKeyboardInterpreter interpreter = null; + private int lastOrientation = Configuration.ORIENTATION_UNDEFINED; private static final String TAG = "SystemKeyboard"; @@ -129,7 +130,7 @@ public View onCreateInputView() { @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); } /** @@ -169,9 +170,7 @@ public void onStartInput(EditorInfo attribute, boolean restarting) { 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; } @@ -197,7 +196,10 @@ public void onUpdateExtractingVisibility(EditorInfo ei) { @Override public void onConfigurationChanged(Configuration newConfig) { super.onConfigurationChanged(newConfig); - KMManager.onConfigurationChanged(newConfig); + if (newConfig.orientation != lastOrientation) { + lastOrientation = newConfig.orientation; + KMManager.onConfigurationChanged(newConfig); + } } @Override @@ -220,8 +222,9 @@ public void onComputeInsets(InputMethodService.Insets outInsets) { wm.getDefaultDisplay().getSize(size); int inputViewHeight = 0; - if (inputView != null) + if (inputView != null) { inputViewHeight = inputView.getHeight(); + } int bannerHeight = KMManager.getBannerHeight(this); int kbHeight = KMManager.getKeyboardHeight(this); diff --git a/android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java b/android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java index bdd0f4f65c4..a356a20ad55 100644 --- a/android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java +++ b/android/KMEA/app/src/main/java/com/keyman/engine/KMKeyboard.java @@ -163,7 +163,7 @@ protected boolean updateText(String text) { 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 @@ protected boolean updateSelectionRange(int selStart, int selEnd) { 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 @@ protected boolean updateSelectionRange(int selStart, int selEnd) { 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; diff --git a/android/KMEA/app/src/main/java/com/keyman/engine/KMManager.java b/android/KMEA/app/src/main/java/com/keyman/engine/KMManager.java index a2b0baa7930..b31fe425066 100644 --- a/android/KMEA/app/src/main/java/com/keyman/engine/KMManager.java +++ b/android/KMEA/app/src/main/java/com/keyman/engine/KMManager.java @@ -2137,17 +2137,40 @@ public static boolean updateText(KeyboardType kbType, String text) { return result; } + /** + * Updates the active range for selected text. + * @deprecated + * This method no longer needs the `selStart` and `selEnd` parameters. + *

Use {@link KMManager#updateSelectionRange(KeyboardType)} instead.

+ * + * @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; + if (kbType == KeyboardType.KEYBOARD_TYPE_INAPP) { if (isKeyboardLoaded(KeyboardType.KEYBOARD_TYPE_INAPP) && !InAppKeyboard.shouldIgnoreSelectionChange()) { - result = InAppKeyboard.updateSelectionRange(selStart, selEnd); + result = InAppKeyboard.updateSelectionRange(); } InAppKeyboard.setShouldIgnoreSelectionChange(false); } else if (kbType == KeyboardType.KEYBOARD_TYPE_SYSTEM) { if (isKeyboardLoaded(KeyboardType.KEYBOARD_TYPE_SYSTEM) && !SystemKeyboard.shouldIgnoreSelectionChange()) { - result = SystemKeyboard.updateSelectionRange(selStart, selEnd); + result = SystemKeyboard.updateSelectionRange(); } SystemKeyboard.setShouldIgnoreSelectionChange(false); diff --git a/android/KMEA/app/src/main/java/com/keyman/engine/KMTextView.java b/android/KMEA/app/src/main/java/com/keyman/engine/KMTextView.java index 70774a4e2c7..3c5959fc9d6 100644 --- a/android/KMEA/app/src/main/java/com/keyman/engine/KMTextView.java +++ b/android/KMEA/app/src/main/java/com/keyman/engine/KMTextView.java @@ -63,10 +63,8 @@ public KMTextView(Context context, AttributeSet attrs, int defStyle) { */ 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 @@ protected void onTextChanged(CharSequence text, int start, int lengthBefore, int 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); } } diff --git a/common/include/kmx_file.h b/common/include/kmx_file.h index cd379489895..748ba32d920 100644 --- a/common/include/kmx_file.h +++ b/common/include/kmx_file.h @@ -302,6 +302,9 @@ namespace kmx { #define K_MODIFIERFLAG 0x007F #define K_NOTMODIFIERFLAG 0xFF00 // I4548 +// Note: OTHER_MODIFIER = 0x10000, used by KMX+ for the +// other modifier flag in layers, > 16 bit so not available here. +// See keys_mod_other in keyman_core_ldml.ts struct COMP_STORE { KMX_DWORD_unaligned dwSystemID; diff --git a/common/web/gesture-recognizer/src/engine/headless/asyncClosureDispatchQueue.ts b/common/web/gesture-recognizer/src/engine/headless/asyncClosureDispatchQueue.ts new file mode 100644 index 00000000000..a15265cb928 --- /dev/null +++ b/common/web/gesture-recognizer/src/engine/headless/asyncClosureDispatchQueue.ts @@ -0,0 +1,98 @@ +import { timedPromise } from "@keymanapp/web-utils"; +import { reportError } from "../reportError.js"; + +export type QueueClosure = () => (Promise | void); + +/** + This class is modeled somewhat after Swift's `DispatchQueue` class, but with + the twist that each closure may return a `Promise` (in Swift: a `Future`) to + lock out further closure processing until the `Promise` resolves. +*/ +export class AsyncClosureDispatchQueue { + private queue: QueueClosure[]; + private waitLock: Promise; + private defaultWaitFactory: () => Promise; + + /** + * + * @param defaultWaitFactory A factory returning Promises to use for default + * delays between tasks. If not specified, Promises corresponding to + * setTimeout(0) will be used, allowing the microqueue task to flush between + * tasks. + */ + constructor(defaultWaitFactory?: () => Promise) { + // We only need to trigger events if the queue has no prior entries and there isn't an + // active wait-lock; for the latter, we'll auto-trigger the next function when it unlocks. + this.queue = []; + + this.defaultWaitFactory = defaultWaitFactory || (() => { return timedPromise(0) }); + } + + get defaultWait() { + return this.defaultWaitFactory(); + } + + get ready() { + return this.queue.length == 0 && !this.waitLock; + } + + 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; + try { + // Is either undefined (return type: void) or is a Promise. + result = functor() as undefined | Promise; + /* 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 result; + } catch(err) { + reportError('Async error from queued closure', err); + } + + this.waitLock = null; + // if queue is length zero, auto-returns. + this.triggerNextClosure(); + } + + runAsync(closure: QueueClosure) { + // Check before putting the closure on the internal queue; the check is based in part + // upon the existing queue length. + const isReady = this.ready; + + this.queue.push(closure); + + // If `!isReady`, the next closure will automatically be triggered when possible. + if(isReady) { + this.triggerNextClosure(); + } + } +} \ No newline at end of file diff --git a/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/gestureMatcher.ts b/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/gestureMatcher.ts index 4606ff2071f..8a3c14fc6ee 100644 --- a/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/gestureMatcher.ts +++ b/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/gestureMatcher.ts @@ -157,14 +157,14 @@ export class GestureMatcher 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; } } @@ -193,10 +193,16 @@ export class GestureMatcher implements PredecessorMatch< // Easy peasy - resolutions only need & have the one defined action type. action = this.model.resolutionAction; } else { - // Some gesture types may wish to restart with a new base item if they fail due to - // it changing during its lifetime or due to characteristics of the contact-point's - // path. - if(this.model.rejectionActions?.[cause]) { + /* + Some gesture types may wish to restart with a new base item if they fail due to + it changing during its lifetime or due to characteristics of the contact-point's + path. + + If a gesture model match is outright-cancelled, matcher restarts should be completely + blocked. One notable reason: if a model-match is _immediately_ cancelled due to + initial conditions, reattempting it can cause an infinite (async) loop! + */ + if(cause != 'cancelled' && this.model.rejectionActions?.[cause]) { action = this.model.rejectionActions[cause]; action.item = 'none'; } @@ -361,7 +367,7 @@ export class GestureMatcher implements PredecessorMatch< return this._result; } - private addContactInternal(simpleSource: GestureSourceSubview, basePathStats: CumulativePathStats) { + private addContactInternal(simpleSource: GestureSourceSubview, basePathStats: CumulativePathStats, whileInitializing?: boolean) { // The number of already-active contacts tracked for this gesture const existingContacts = this.pathMatchers.length; @@ -437,6 +443,7 @@ export class GestureMatcher implements PredecessorMatch< } } + // Check that initial "item" and "state" properties are legal for this type of gesture. if(contactSpec.model.allowsInitialState) { const initialStateCheck = contactSpec.model.allowsInitialState( simpleSource.currentSample, @@ -450,13 +457,59 @@ export class GestureMatcher implements PredecessorMatch< // pathMatcher for a source that failed to meet initial conditions. this.pathMatchers.pop(); - this.finalize(false, 'path'); + /* + To prevent any further retries for the model (via rejectionActions), we list the + cause as 'cancelled'. 'Cancelled' match attempts will never be retried, and we + wish to prevent an infinite (async) loop from retrying something we know will + auto-cancel. (That loop would automatically end upon a different model's match + or upon all possible models failing to match at the same time, but it's still + far from ideal.) + + The rejection-action mechanism in MatcherSelector's `replacer` method (refer to + https://github.com/keymanapp/keyman/blob/be867604e4b2650a60e69dc6bbe0b6115315efff/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/matcherSelector.ts#L559-L575) + already blocks paths that are rejected synchronously by this method. Use of + 'cancelled' is thus not necessary for avoiding the loop-scenario, but it does + add an extra layer of protection. Also, it's more explicit about the fact that + we _are_ permanently cancelling any and all future attempts to match against + it in the future for the affected `GestureSource`(s). + + If we weren't using 'cancelled', 'item' would correspond best with a rejection + here, as the decision is made due to a validation check against the initial item. + */ + this.finalize(false, 'cancelled'); } } - contactModel.update(); - // Now that we've done the initial-state check, we can check for instantly-matching path models. + /* + Now that we've done the initial-state check, we can check for instantly-matching and + instantly-rejecting path models... particularly from from causes other than initial-item + and state, such as rejection due to an extra touch. + + KMW example: longpresses cancel when a new touch comes in during the longpress timer; + they should never become valid again for that base touch. + */ + 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. + + 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, whileInitializing ? 'cancelled' : 'path'); + } + // Standard path: trigger either resolution or rejection when the contact model signals either. contactModel.promise.then((resolution) => { this.finalize(resolution.type == 'resolve', resolution.cause); }); diff --git a/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/matcherSelector.ts b/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/matcherSelector.ts index 9d9b894f54b..c8a2cefa1a1 100644 --- a/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/matcherSelector.ts +++ b/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/matcherSelector.ts @@ -15,6 +15,15 @@ interface GestureSourceTracker { */ source: GestureSource; matchPromise: ManagedPromise>; + /** + * Set to `true` during the timeout period needed to complete existing trackers & + * initialize new ones. Once that process is complete, set to false. + * + * This is needed to ensure that failure to extend an existing gesture doesn't + * result in outright selection-failure before attempting to match as a + * newly-started gesture. + */ + preserve: boolean; } export interface MatcherSelection { @@ -225,10 +234,22 @@ export class MatcherSelector extends EventEmitter(); + + this.pendingMatchSetup = childLock.corePromise; + // If a prior call is still waiting on the `await` below, wait for it to clear // entirely before proceeding; there could be effects for how the next part below is processed. - await this.pendingMatchSetup; + + await parentLockPromise; + + if(this.pendingMatchSetup == childLock.corePromise) { + this.pendingMatchSetup = null; + } + childLock.resolve(); // allow the next matchGesture call through. } if(sourceNotYetStaged) { @@ -257,7 +278,8 @@ export class MatcherSelector extends EventEmitter = { source: src, - matchPromise: matchPromise + matchPromise: matchPromise, + preserve: true }; this._sourceSelector.push(sourceSelectors); @@ -303,14 +325,20 @@ export class MatcherSelector extends EventEmitter(); - this.pendingMatchSetup = pendingMatchGesture.corePromise; + const matchingLock = new ManagedPromise(); + this.pendingMatchSetup = matchingLock.corePromise; + await timedPromise(0); // A second one, in case of a deferred modipress completion (via awaitNested) // (which itself needs a macroqueue wait) await timedPromise(0); - this.pendingMatchSetup = null; - pendingMatchGesture.resolve(); + + // Only clear the promise if no extra entries were added to the implied `matchGesture` queue. + if(this.pendingMatchSetup == matchingLock.corePromise) { + this.pendingMatchSetup = null; + } + + matchingLock.resolve(); // stateToken may have shifted by the time we regain control here. const incomingStateToken = this.stateToken; @@ -359,6 +387,10 @@ export class MatcherSelector extends EventEmitter { + tracker.preserve = false; + }) + // If in a sustain mode, no models for new sources may launch; // only existing sequences are allowed to continue. if(this.sustainMode && unmatchedSource) { @@ -653,7 +685,7 @@ export class MatcherSelector extends EventEmitter { readonly resolutionAction: GestureResolutionSpec; - readonly rejectionActions?: Partial>; + /* + Do NOT allow 'cancelled' rejection-actions. If 'cancelled', the corresponding `GestureSource`s + can no longer be valid matches for the GestureModel under any condition. + + Generally, this is due to the underlying sources themselves being cancelled, but this can also + arise under the following combination of conditions: + - a model instantly rejects... + - whenever a new `GestureSource` starts and matches an instantly-rejecting `PathModel` for this + `GestureModel` (cause: 'path') + - when it fails initial-state validation (cause: 'item') + - a corresponding rejection action has been defined. + - For example, it also rejects under certain path conditions (for its original `GestureSource`) + that are recoverable. + + Upon receiving an incoming extra GestureSource, the model would instantly reject (cause: 'path') + and could attempt to restart if specified to do so by a 'path' rejection action. In such a case, + it would instantly reject again due to the same reason. Instant rejection of a replacement model + during a rejection action is reported as 'cancellation'. + */ + + readonly rejectionActions?: Partial, RejectionReplace>>; + // If there is a 'gesture stack' associated with the gesture chain, it's auto-popped // upon completion of the chain. Optional-chaining can sustain the chain while the // potential child gesture is still a possibility. diff --git a/common/web/gesture-recognizer/src/engine/headless/inputEngineBase.ts b/common/web/gesture-recognizer/src/engine/headless/inputEngineBase.ts index 3acd723c426..683bee0b6b5 100644 --- a/common/web/gesture-recognizer/src/engine/headless/inputEngineBase.ts +++ b/common/web/gesture-recognizer/src/engine/headless/inputEngineBase.ts @@ -56,15 +56,17 @@ export abstract class InputEngineBase extends return source; } + public fulfillInputStart(touchpoint: GestureSource) {} + /** * Calls to this method will cancel any touchpoints whose internal IDs are _not_ included in the parameter. * Designed to facilitate recovery from error cases and peculiar states that sometimes arise when debugging. * @param identifiers */ - maintainTouchpointsWithIds(identifiers: number[]) { - const identifiersToMaintain = identifiers.map((internal_id) => this.identifierMap[internal_id]); + maintainTouchpoints(touchpoints: GestureSource[]) { + touchpoints ||= []; this._activeTouchpoints - .filter((source) => !identifiersToMaintain.includes(source.rawIdentifier)) + .filter((source) => !touchpoints.includes(source)) // Will trigger `.dropTouchpoint` later in the event chain. .forEach((source) => source.terminate(true)); } diff --git a/common/web/gesture-recognizer/src/engine/headless/touchpointCoordinator.ts b/common/web/gesture-recognizer/src/engine/headless/touchpointCoordinator.ts index ff812771874..157da4b99b4 100644 --- a/common/web/gesture-recognizer/src/engine/headless/touchpointCoordinator.ts +++ b/common/web/gesture-recognizer/src/engine/headless/touchpointCoordinator.ts @@ -5,9 +5,9 @@ import { MatcherSelection, MatcherSelector } from "./gestures/matchers/matcherSe import { GestureSequence } from "./gestures/matchers/gestureSequence.js"; import { GestureModelDefs, getGestureModel, getGestureModelSet } from "./gestures/specs/gestureModelDefs.js"; import { GestureModel } from "./gestures/specs/gestureModel.js"; -import { timedPromise } from "@keymanapp/web-utils"; import { InputSample } from "./inputSample.js"; import { GestureDebugPath } from "./gestureDebugPath.js"; +import { reportError } from "../reportError.js"; interface EventMap { /** @@ -223,8 +223,35 @@ export class TouchpointCoordinator extends Even const selector = this.currentSelector; touchpoint.setGestureMatchInspector(this.buildGestureMatchInspector(selector)); - this.emit('inputstart', touchpoint); + /* + If there's an error in code receiving this event, we must not let that break the flow of + event input processing - we may still have a locking Promise corresponding to our active + GestureSource. (See: next comment) + */ + try { + this.emit('inputstart', touchpoint); + } catch (err) { + reportError("Error from 'inputstart' event listener", err); + } + + /* + If an `InputEventEngine` internally utilizes the `AsyncClosureDispatchQueue`, this is the point + at which we are now safe to process further events. The correct 'stateToken' has been identified + and all GestureMatcher possibilities for the source have been launched; path updates may resume _and_ + new incoming paths may now be safely handled. As such, we can now fulfill any Promise returned by + a closure defined within its `inputStart` method for the `GestureSource` under consideration. + + It is quite important that we _do_ fulfill the `Promise` if it exists - further event processing will + be blocked for such engines until this is done! (Hence the try-catch above) + */ + this.inputEngines.forEach((engine) => { + engine.fulfillInputStart(touchpoint); + }); + + // ---------------------------------------- + + // All gesture-matching is prepared; now we await the source's first gesture model match. const selection = await selectionPromise; // Any related 'push' mechanics that may still be lingering are currently handled by GestureSequence @@ -255,6 +282,7 @@ export class TouchpointCoordinator extends Even // Could track sequences easily enough; the question is how to tell when to 'let go'. + // No try-catch because only there's no critical code after it. this.emit('recognizedgesture', gestureSequence); } diff --git a/common/web/gesture-recognizer/src/engine/index.ts b/common/web/gesture-recognizer/src/engine/index.ts index 76e4cbe4840..3da7cc16dac 100644 --- a/common/web/gesture-recognizer/src/engine/index.ts +++ b/common/web/gesture-recognizer/src/engine/index.ts @@ -1,5 +1,6 @@ import { validateModelDefs } from './headless/gestures/specs/modelDefValidator.js'; +export { AsyncClosureDispatchQueue, QueueClosure } from './headless/asyncClosureDispatchQueue.js'; export { CumulativePathStats } from './headless/cumulativePathStats.js'; export { GestureModelDefs } from './headless/gestures/specs/gestureModelDefs.js'; export { GestureRecognizer } from "./gestureRecognizer.js"; diff --git a/common/web/gesture-recognizer/src/engine/inputEventEngine.ts b/common/web/gesture-recognizer/src/engine/inputEventEngine.ts index 8f434b71df8..eb3c262ebb4 100644 --- a/common/web/gesture-recognizer/src/engine/inputEventEngine.ts +++ b/common/web/gesture-recognizer/src/engine/inputEventEngine.ts @@ -2,6 +2,7 @@ import { InputEngineBase } from "./headless/inputEngineBase.js"; import { InputSample } from "./headless/inputSample.js"; import { GestureSource } from "./headless/gestureSource.js"; import { GestureRecognizerConfiguration } from "./index.js"; +import { reportError } from "./reportError.js"; export function processSampleClientCoords(config: GestureRecognizerConfiguration, clientX: number, clientY: number) { const targetRect = config.targetRoot.getBoundingClientRect(); @@ -13,12 +14,12 @@ export function processSampleClientCoords(config: GestureRecog } as InputSample; } -export abstract class InputEventEngine extends InputEngineBase { +export abstract class InputEventEngine extends InputEngineBase { abstract registerEventHandlers(): void; abstract unregisterEventHandlers(): void; - protected buildSampleFor(clientX: number, clientY: number, target: EventTarget, timestamp: number, source: GestureSource): InputSample { - const sample: InputSample = { + protected buildSampleFor(clientX: number, clientY: number, target: EventTarget, timestamp: number, source: GestureSource): InputSample { + const sample: InputSample = { ...processSampleClientCoords(this.config, clientX, clientY), t: timestamp, stateToken: source?.stateToken ?? this.stateToken @@ -31,7 +32,7 @@ export abstract class InputEventEngine extends Inpu return sample; } - protected onInputStart(identifier: number, sample: InputSample, target: EventTarget, isFromTouch: boolean) { + protected onInputStart(identifier: number, sample: InputSample, target: EventTarget, isFromTouch: boolean) { const touchpoint = this.createTouchpoint(identifier, isFromTouch); touchpoint.update(sample); @@ -47,49 +48,49 @@ export abstract class InputEventEngine extends Inpu this.dropTouchpoint(touchpoint); }); - this.emit('pointstart', touchpoint); + try { + this.emit('pointstart', touchpoint); + } catch(err) { + reportError('Engine-internal error while initializing gesture matching for new source', err); + } + + return touchpoint; } - protected onInputMove(identifier: number, sample: InputSample, target: EventTarget) { - const activePoint = this.getTouchpointWithId(identifier); - if(!activePoint) { + protected onInputMove(touchpoint: GestureSource, sample: InputSample, target: EventTarget) { + if(!touchpoint) { return; } - activePoint.update(sample); + try { + touchpoint.update(sample); + } catch(err) { + reportError('Error occurred while updating source', err); + } } - protected onInputMoveCancel(identifier: number, sample: InputSample, target: EventTarget) { - const touchpoint = this.getTouchpointWithId(identifier); + protected onInputMoveCancel(touchpoint: GestureSource, sample: InputSample, target: EventTarget) { if(!touchpoint) { return; } - touchpoint.update(sample); - touchpoint.path.terminate(true); + try { + touchpoint.update(sample); + touchpoint.path.terminate(true); + } catch(err) { + reportError('Error occurred while cancelling further input for source', err); + } } - protected onInputEnd(identifier: number, target: EventTarget) { - const touchpoint = this.getTouchpointWithId(identifier); + protected onInputEnd(touchpoint: GestureSource, target: EventTarget) { if(!touchpoint) { return; } - const lastEntry = touchpoint.path.stats.lastSample; - const sample = this.buildSampleFor(lastEntry.clientX, lastEntry.clientY, target, lastEntry.t, touchpoint); - - /* While an 'end' event immediately follows a 'move' if it occurred simultaneously, - * this is decidedly _not_ the case if the touchpoint was held for a while without - * moving, even at the point of its release. - * - * We'll never need to worry about the touchpoint moving here, and thus we don't - * need to worry about `currentHoveredItem` changing. We're only concerned with - * recording the _timing_ of the touchpoint's release. - */ - if(sample.t != lastEntry.t) { - touchpoint.update(sample); + try { + touchpoint.path.terminate(false); + } catch(err) { + reportError('Error occurred while finalizing input for source', err); } - - this.getTouchpointWithId(identifier)?.path.terminate(false); } } \ No newline at end of file diff --git a/common/web/gesture-recognizer/src/engine/mouseEventEngine.ts b/common/web/gesture-recognizer/src/engine/mouseEventEngine.ts index 42baf53299b..a84f4294fe8 100644 --- a/common/web/gesture-recognizer/src/engine/mouseEventEngine.ts +++ b/common/web/gesture-recognizer/src/engine/mouseEventEngine.ts @@ -1,10 +1,11 @@ import { GestureRecognizerConfiguration } from "./configuration/gestureRecognizerConfiguration.js"; import { InputEventEngine } from "./inputEventEngine.js"; -import { InputSample } from "./headless/inputSample.js"; import { Nonoptional } from "./nonoptional.js"; import { ZoneBoundaryChecker } from "./configuration/zoneBoundaryChecker.js"; +import { GestureSource } from "./headless/gestureSource.js"; -export class MouseEventEngine extends InputEventEngine { +// Does NOT use the AsyncClosureDispatchQueue... simply because there can only ever be one mouse touchpoint. +export class MouseEventEngine extends InputEventEngine { private readonly _mouseStart: typeof MouseEventEngine.prototype.onMouseStart; private readonly _mouseMove: typeof MouseEventEngine.prototype.onMouseMove; private readonly _mouseEnd: typeof MouseEventEngine.prototype.onMouseEnd; @@ -12,7 +13,10 @@ export class MouseEventEngine extends InputEv private hasActiveClick: boolean = false; private disabledSafeBounds: number = 0; - public constructor(config: Nonoptional>) { + private currentSource: GestureSource = null; + private readonly activeIdentifier = 0; + + public constructor(config: Nonoptional>) { super(config); // We use this approach, rather than .bind, because _this_ version allows hook @@ -25,20 +29,6 @@ export class MouseEventEngine extends InputEv private get eventRoot(): HTMLElement { return this.config.mouseEventRoot; } - private get activeIdentifier(): number { - return 0; - } - - // public static forPredictiveBanner(banner: SuggestionBanner, handlerRoot: SuggestionManager) { - // const config: GestureRecognizerConfiguration = { - // targetRoot: banner.getDiv(), - // // document.body is the event root b/c we need to track the mouse if it leaves - // // the VisualKeyboard's hierarchy. - // eventRoot: document.body, - // }; - - // return new MouseEventEngine(config); - // } registerEventHandlers() { this.eventRoot.addEventListener('mousedown', this._mouseStart, true); @@ -66,10 +56,9 @@ export class MouseEventEngine extends InputEv } } - private buildSampleFromEvent(event: MouseEvent, identifier: number) { + private buildSampleFromEvent(event: MouseEvent) { // WILL be null for newly-starting `GestureSource`s / contact points. - const source = this.getTouchpointWithId(identifier); - return this.buildSampleFor(event.clientX, event.clientY, event.target, performance.now(), source); + return this.buildSampleFor(event.clientX, event.clientY, event.target, performance.now(), this.currentSource); } onMouseStart(event: MouseEvent) { @@ -81,8 +70,7 @@ export class MouseEventEngine extends InputEv this.preventPropagation(event); - const identifier = this.activeIdentifier; - const sample = this.buildSampleFromEvent(event, identifier); + const sample = this.buildSampleFromEvent(event); if(!ZoneBoundaryChecker.inputStartOutOfBoundsCheck(sample, this.config)) { // If we started very close to a safe zone border, remember which one(s). @@ -90,36 +78,46 @@ export class MouseEventEngine extends InputEv this.disabledSafeBounds = ZoneBoundaryChecker.inputStartSafeBoundProximityCheck(sample, this.config); } - this.onInputStart(identifier, sample, event.target, false); + const touchpoint = this.onInputStart(this.activeIdentifier, sample, event.target, false); + this.currentSource = touchpoint; + + const cleanup = () => { + this.currentSource = null; + } + + touchpoint.path.on('complete', cleanup); + touchpoint.path.on('invalidated', cleanup); } onMouseMove(event: MouseEvent) { - if(!this.hasActiveTouchpoint(this.activeIdentifier)) { + const source = this.currentSource; + if(!source) { return; } - const sample = this.buildSampleFromEvent(event, this.activeIdentifier); + const sample = this.buildSampleFromEvent(event); if(!event.buttons) { if(this.hasActiveClick) { this.hasActiveClick = false; - this.onInputMoveCancel(this.activeIdentifier, sample, event.target); + this.onInputMoveCancel(source, sample, event.target); } return; } this.preventPropagation(event); - const config = this.getConfigForId(this.activeIdentifier); + const config = source.currentRecognizerConfig; if(!ZoneBoundaryChecker.inputMoveCancellationCheck(sample, config, this.disabledSafeBounds)) { - this.onInputMove(this.activeIdentifier, sample, event.target); + this.onInputMove(source, sample, event.target); } else { - this.onInputMoveCancel(this.activeIdentifier, sample, event.target); + this.onInputMoveCancel(source, sample, event.target); } } onMouseEnd(event: MouseEvent) { - if(!this.hasActiveTouchpoint(this.activeIdentifier)) { + const source = this.currentSource; + if(!source) { return; } @@ -127,6 +125,6 @@ export class MouseEventEngine extends InputEv this.hasActiveClick = false; } - this.onInputEnd(this.activeIdentifier, event.target); + this.onInputEnd(source, event.target); } } \ No newline at end of file diff --git a/common/web/gesture-recognizer/src/engine/reportError.ts b/common/web/gesture-recognizer/src/engine/reportError.ts new file mode 100644 index 00000000000..076a987bdd6 --- /dev/null +++ b/common/web/gesture-recognizer/src/engine/reportError.ts @@ -0,0 +1,9 @@ +export function reportError(baseMsg: string, err: Error) { + // Our mobile-app Sentry logging will listen for this and log it. + if(err instanceof Error) { + console.error(`${baseMsg}: ${err.message}\n\n${err.stack}`); + } else { + console.error(baseMsg); + console.error(err); + } +} \ No newline at end of file diff --git a/common/web/gesture-recognizer/src/engine/touchEventEngine.ts b/common/web/gesture-recognizer/src/engine/touchEventEngine.ts index a2fc2b6b2c6..611e205b5e4 100644 --- a/common/web/gesture-recognizer/src/engine/touchEventEngine.ts +++ b/common/web/gesture-recognizer/src/engine/touchEventEngine.ts @@ -1,9 +1,11 @@ import { GestureRecognizerConfiguration } from "./configuration/gestureRecognizerConfiguration.js"; import { InputEventEngine } from "./inputEventEngine.js"; -import { InputSample } from "./headless/inputSample.js"; import { Nonoptional } from "./nonoptional.js"; import { ZoneBoundaryChecker } from "./configuration/zoneBoundaryChecker.js"; import { GestureSource } from "./headless/gestureSource.js"; +import { ManagedPromise } from "@keymanapp/web-utils"; +import { AsyncClosureDispatchQueue } from "./headless/asyncClosureDispatchQueue.js"; +import { GesturePath } from "./index.js"; function touchListToArray(list: TouchList) { const arr: Touch[] = []; @@ -14,14 +16,19 @@ function touchListToArray(list: TouchList) { return arr; } -export class TouchEventEngine extends InputEventEngine { +export class TouchEventEngine extends InputEventEngine { private readonly _touchStart: typeof TouchEventEngine.prototype.onTouchStart; private readonly _touchMove: typeof TouchEventEngine.prototype.onTouchMove; private readonly _touchEnd: typeof TouchEventEngine.prototype.onTouchEnd; + protected readonly eventDispatcher = new AsyncClosureDispatchQueue(); + private safeBoundMaskMap: {[id: number]: number} = {}; + // This map works synchronously with the actual event handlers. + private pendingSourcePromises: Map>> = new Map(); + private inputStartSignalMap: Map, ManagedPromise> = new Map(); - public constructor(config: Nonoptional>) { + public constructor(config: Nonoptional>) { super(config); // We use this approach, rather than .bind, because _this_ version allows hook @@ -35,17 +42,6 @@ export class TouchEventEngine extends InputEv return this.config.touchEventRoot; } - // public static forPredictiveBanner(banner: SuggestionBanner, handlerRoot: SuggestionManager) { - // const config: GestureRecognizerConfiguration = { - // targetRoot: banner.getDiv(), - // // document.body is the event root b/c we need to track the mouse if it leaves - // // the VisualKeyboard's hierarchy. - // eventRoot: banner.getDiv(), - // }; - - // return new TouchEventEngine(config); - // } - registerEventHandlers() { // The 'passive' property ensures we can prevent MouseEvent followups from TouchEvents. // It is only specified during `addEventListener`, not during `removeEventListener`. @@ -76,7 +72,7 @@ export class TouchEventEngine extends InputEv } } - public dropTouchpoint(source: GestureSource) { + public dropTouchpoint(source: GestureSource) { super.dropTouchpoint(source); for(const key of Object.keys(this.safeBoundMaskMap)) { @@ -86,9 +82,21 @@ export class TouchEventEngine extends InputEv } } - private buildSampleFromTouch(touch: Touch, timestamp: number) { + public fulfillInputStart(touchpoint: GestureSource>) { + const lock = this.inputStartSignalMap.get(touchpoint); + if(lock) { + this.inputStartSignalMap.delete(touchpoint); + lock.resolve(); + } + }; + + public hasActiveTouchpoint(identifier: number): boolean { + const baseResult = super.hasActiveTouchpoint(identifier); + return baseResult || !!this.pendingSourcePromises.has(identifier); + } + + private buildSampleFromTouch(touch: Touch, timestamp: number, source: GestureSource) { // WILL be null for newly-starting `GestureSource`s / contact points. - const source = this.getTouchpointWithId(touch.identifier); return this.buildSampleFor(touch.clientX, touch.clientY, touch.target, timestamp, source); } @@ -106,89 +114,244 @@ export class TouchEventEngine extends InputEv // during a touchstart.) const allTouches = touchListToArray(event.touches); const newTouches = touchListToArray(event.changedTouches); - // Maintain all touches in the `.touches` array that are NOT marked as `.changedTouches` (and therefore, new) - this.maintainTouchpointsWithIds(allTouches - .filter((touch1) => newTouches.findIndex(touch2 => touch1.identifier == touch2.identifier) == -1) - .map((touch) => touch.identifier) - ); - - // Ensure the same timestamp is used for all touches being updated. - const timestamp = performance.now(); - - // During a touch-start, only _new_ touch contact points are listed here; - // we shouldn't signal "input start" for any previously-existing touch points, - // so `.changedTouches` is the best way forward. + const oldTouches = allTouches.filter((touch1) => { + return newTouches.findIndex(touch2 => touch1.identifier == touch2.identifier) == -1; + }); + + // Any 'old touches' should have pre-existing entries in our promise-map that are still current, as + // the promise-map is maintained 100% synchronously with incoming events. + const oldSourcePromises = oldTouches.map((touch) => this.pendingSourcePromises.get(touch.identifier)); + + this.eventDispatcher.runAsync(async () => { + const oldSources = await Promise.all(oldSourcePromises); + // Maintain all touches in the `.touches` array that are NOT marked as `.changedTouches` (and therefore, new) + this.maintainTouchpoints(oldSources); + + return this.eventDispatcher.defaultWait; + }); + + /* + We create Promises that can be set and retrieved synchronously with the actual event handlers + in order to prevent issues from tricky asynchronous identifier-to-source mapping attempts. + + As these Promises are set (and thus, retrievable) synchronously with the actual event handlers, + we can closure-capture them for use in the internally-asynchronous processing closures. + + `capturedSourcePromises` will be useful for closure-capture binding the new Promise(s) to + the closure to be queued. `this.pendingSourcePromises` facilitates similar closure-capture + patterns within the touchMove and touchEnd handlers for their queued closures. + */ + const capturedSourcePromises = new Map>>(); for(let i=0; i < event.changedTouches.length; i++) { const touch = event.changedTouches.item(i); - const sample = this.buildSampleFromTouch(touch, timestamp); - - if(!ZoneBoundaryChecker.inputStartOutOfBoundsCheck(sample, this.config)) { - // If we started very close to a safe zone border, remember which one(s). - // This is important for input-sequence cancellation check logic. - this.safeBoundMaskMap[touch.identifier] = ZoneBoundaryChecker.inputStartSafeBoundProximityCheck(sample, this.config); - } else { - // This touchpoint shouldn't be considered; do not signal a touchstart for it. - continue; + const promise = new ManagedPromise>(); + this.pendingSourcePromises.set(touch.identifier, promise); + capturedSourcePromises.set(touch.identifier, promise); + } + + /* + When multiple touchpoints are active, we need to ensure a specific order of events. + The easiest way to ensure the exact order involves programmatic delay of their + processing, essentially "sequentializing" the events into a deterministic order. + + It also helps to ensure that any path updates are only emitted when all listeners + for that path have been prepared - and other parts of the engine cause that to happen + asynchronously in certain situations. Within KMW, one such case is when a simple-tap + with `nextLayer` defined is auto-completed by a new incoming touch, triggering an + instant layer-change. + */ + this.eventDispatcher.runAsync(() => { + // Ensure the same timestamp is used for all touches being updated. + const timestamp = performance.now(); + let touchpoint: GestureSource = null; + + // During a touch-start, only _new_ touch contact points are listed here; + // we shouldn't signal "input start" for any previously-existing touch points, + // so `.changedTouches` is the best way forward. + for(let i=0; i < event.changedTouches.length; i++) { + const touch = event.changedTouches.item(i); + const touchId = touch.identifier; + const sample = this.buildSampleFromTouch(touch, timestamp, null); + + if(!ZoneBoundaryChecker.inputStartOutOfBoundsCheck(sample, this.config)) { + // If we started very close to a safe zone border, remember which one(s). + // This is important for input-sequence cancellation check logic. + this.safeBoundMaskMap[touchId] = ZoneBoundaryChecker.inputStartSafeBoundProximityCheck(sample, this.config); + } else { + // This touchpoint shouldn't be considered; do not signal a touchstart for it. + let sourcePromise = capturedSourcePromises.get(touchId); + sourcePromise.resolve(null); + continue; + } + + touchpoint = this.onInputStart(touchId, sample, event.target, true); + + /* + We use the closure-captured version bound to this specific closure, rather than the + most recent one for the touch-identifier - under heavy rapid typing, it's possible that + the touch-identifier has been reused. + + The resolved Promise may then be used to retrieve the correct source in the other event + handlers' closures. + */ + let sourcePromise = capturedSourcePromises.get(touchId); + sourcePromise.resolve(touchpoint); + + /* + Ensure we only do the cleanup if and when it hasn't already been replaced by new events later. + + Must be done for EACH source - we can't risk leaving a lingering entry once we've dismissed + processing for the source. Failure to do so may result in blocking touch events that should + no longer be manipulated by this engine by affecting `hasActiveTouchpoint`. + */ + const cleanup = () => { + /* + If delays accumulate significantly, it is possible that when this queued closure is run, + a different touchpoint is reusing the same identifier. Don't delete the entry if our + entry has been replaced. + */ + if(this.pendingSourcePromises.get(touchId) == sourcePromise) { + this.pendingSourcePromises.delete(touchId); + } + } + + touchpoint.path.on('complete', cleanup); + touchpoint.path.on('invalidated', cleanup); } - this.onInputStart(touch.identifier, sample, event.target, true); - } + if(touchpoint) { + // This 'lock' should only be released when the last simultaneously-registered touch is published via + // gesture-recognizer event. + let eventSignalPromise = new ManagedPromise(); + this.inputStartSignalMap.set(touchpoint, eventSignalPromise); + + return eventSignalPromise.corePromise; + } + }); } onTouchMove(event: TouchEvent) { - let propagationActive = true; - // Ensure the same timestamp is used for all touches being updated. - const timestamp = performance.now(); - - this.maintainTouchpointsWithIds(touchListToArray(event.touches) - .map((touch) => touch.identifier) - ); - - // Do not change to `changedTouches` - we need a sample for all active touches in order - // to facilitate path-update synchronization for multi-touch gestures. - // - // May be worth doing changedTouches _first_ though. - for(let i=0; i < event.touches.length; i++) { + for(let i = 0; i < event.touches.length; i++) { const touch = event.touches.item(i); - - if(!this.hasActiveTouchpoint(touch.identifier)) { - continue; - } - - if(propagationActive) { + if(this.hasActiveTouchpoint(touch.identifier)) { this.preventPropagation(event); - propagationActive = false; + break; } + } - const config = this.getConfigForId(touch.identifier); - - const sample = this.buildSampleFromTouch(touch, timestamp); + /* + Using the Promise map built in touchStart, we can retrieve a Promise for the source linked + to this event and closure-capture it for the closure queued below. + */ + const capturedSourcePromises = new Map>>(); + for(let i = 0; i < event.touches.length; i++) { + const touchId = event.touches.item(i).identifier; + // If the source's gesture is finalized or cancelled but touch events are ongoing, + // with no delay between event and its processing, the map entry here will be cleared. + capturedSourcePromises.set(touchId, this.pendingSourcePromises.get(touchId)?.corePromise); + } - if(!ZoneBoundaryChecker.inputMoveCancellationCheck(sample, config, this.safeBoundMaskMap[touch.identifier])) { - this.onInputMove(touch.identifier, sample, touch.target); - } else { - this.onInputMoveCancel(touch.identifier, sample, touch.target); + this.eventDispatcher.runAsync(async () => { + const touches = await Promise.all(capturedSourcePromises.values()); + this.maintainTouchpoints(touches); + + return this.eventDispatcher.defaultWait; + }); + + /* + When multiple touchpoints are active, we need to ensure a specific order of events. + The easiest way to ensure the exact order involves programmatic delay of their + processing, essentially "sequentializing" the events into a deterministic order. + + It also helps to ensure that any path updates are only emitted when all listeners + for that path have been prepared - and other parts of the engine cause that to happen + asynchronously in certain situations. Within KMW, one such case is when a simple-tap + with `nextLayer` defined is auto-completed by a new incoming touch, triggering an + instant layer-change. + */ + this.eventDispatcher.runAsync(async () => { + // Ensure the same timestamp is used for all touches being updated. + const timestamp = performance.now(); + + // Do not change to `changedTouches` - we need a sample for all active touches in order + // to facilitate path-update synchronization for multi-touch gestures. + // + // May be worth doing changedTouches _first_ though. + for(let i=0; i < event.touches.length; i++) { + const touch = event.touches.item(i); + const touchId = touch.identifier; + + + // Only lists touch contact points that have been lifted; touchmove is + // raised separately if any movement occurred. + // + // If the promise object could not be assigned, we `await undefined` - + // which JS converts to `await Promise.resolve(undefined)`. It's safe. + const source = await capturedSourcePromises.get(touchId); + if(!source || source.isPathComplete) { + continue; + } + + const config = source.currentRecognizerConfig; + const sample = this.buildSampleFromTouch(touch, timestamp, source); + + if(!ZoneBoundaryChecker.inputMoveCancellationCheck(sample, config, this.safeBoundMaskMap[touchId])) { + this.onInputMove(source, sample, touch.target); + } else { + this.onInputMoveCancel(source, sample, touch.target); + } } - } + + /* + Since we're operating within an async function, a Promise return-type + is implied. That cancels out the default wait, but we want to ensure + that the default wait is applied here. + */ + return this.eventDispatcher.defaultWait; + }); } onTouchEnd(event: TouchEvent) { - let propagationActive = true; - - // Only lists touch contact points that have been lifted; touchmove is raised separately if any movement occurred. - for(let i=0; i < event.changedTouches.length; i++) { + for(let i = 0; i < event.changedTouches.length; i++) { const touch = event.changedTouches.item(i); - - if(!this.hasActiveTouchpoint(touch.identifier)) { - continue; - } - - if(propagationActive) { + if(this.hasActiveTouchpoint(touch.identifier)) { this.preventPropagation(event); - propagationActive = false; + break; } + } - this.onInputEnd(touch.identifier, event.target); + /* + Using the Promise map built in touchStart, we can retrieve a Promise for the source linked + to this event and closure-capture it for the closure queued below. + */ + const capturedSourcePromises = new Map>>(); + // Any ending touches don't show up in event.touches - only in event.changedTouches! + for(let i = 0; i < event.changedTouches.length; i++) { + const touchId = event.changedTouches.item(i).identifier; + // If the source's gesture is finalized or cancelled but touch events are ongoing, + // with no delay between event and its processing, the map entry here will be cleared. + const promiseToCapture = this.pendingSourcePromises.get(touchId)?.corePromise; + capturedSourcePromises.set(touchId, promiseToCapture); } + + this.eventDispatcher.runAsync(async () => { + // Only lists touch contact points that have been lifted; touchmove is + // raised separately if any movement occurred. + // + // If the promise object could not be assigned, we `await undefined` - + // which JS converts to `await Promise.resolve(undefined)`. It's safe. + for(let i=0; i < event.changedTouches.length; i++) { + const touch = event.changedTouches.item(i); + + const source = await capturedSourcePromises.get(touch.identifier); + if(!source || source.isPathComplete) { + continue; + } + + this.onInputEnd(source, event.target); + } + + return this.eventDispatcher.defaultWait; + }); } } \ No newline at end of file diff --git a/common/web/gesture-recognizer/src/test/auto/browser/cases/canary.js b/common/web/gesture-recognizer/src/test/auto/browser/cases/canary.js index debee09c995..746fc409cf3 100644 --- a/common/web/gesture-recognizer/src/test/auto/browser/cases/canary.js +++ b/common/web/gesture-recognizer/src/test/auto/browser/cases/canary.js @@ -78,10 +78,14 @@ describe("'Canary' checks", function() { // Ensure that the expected handler is called. let fakeHandler = sinon.fake(); - this.controller.recognizer.on('inputstart', fakeHandler) + this.controller.recognizer.on('inputstart', fakeHandler); fireEvent(); - await Promise.resolve(); + await new Promise((resolve) => { + window.setTimeout(resolve, 0); + }).then(() => new Promise((resolve) => { + window.setTimeout(resolve, 0); + })); assert.isTrue(fakeHandler.called, "Unit test attempt failed: handler was not called successfully."); }); @@ -99,10 +103,12 @@ describe("'Canary' checks", function() { // Ensure that the expected handler is called. let fakeHandler = sinon.fake(); - this.controller.recognizer.on('inputstart', fakeHandler) + this.controller.recognizer.on('inputstart', fakeHandler); fireEvent(); - await Promise.resolve(); + await new Promise((resolve) => { + window.setTimeout(resolve, 0); + }); assert.isTrue(fakeHandler.called, "Unit test attempt failed: handler was not called successfully."); }); diff --git a/common/web/gesture-recognizer/src/test/auto/browser/cases/recordedCoordSequences.js b/common/web/gesture-recognizer/src/test/auto/browser/cases/recordedCoordSequences.js index 28d865a8ef3..b1d35549c24 100644 --- a/common/web/gesture-recognizer/src/test/auto/browser/cases/recordedCoordSequences.js +++ b/common/web/gesture-recognizer/src/test/auto/browser/cases/recordedCoordSequences.js @@ -91,12 +91,12 @@ describe("Layer one - DOM -> InputSequence", function() { const sampleResult = resultContactPath[j]; const sampleOriginal = originalContactPath[j]; - assert.isOk(sampleResult, `An expected sample was missing during simulation - failed at path entry ${j}`); - assert.isOk(sampleOriginal, `An extra sample was generated during simulation - failed at path entry ${j}`); + assert.isOk(sampleResult, `An expected sample was missing during simulation - failed at path entry ${j}, path ${i}`); + assert.isOk(sampleOriginal, `An extra sample was generated during simulation - failed at path entry ${j}, path ${i}`); // During test runs against a real Android device, we tend to get almost, but not-quite, integer targetX and targetY values. - expect(sampleResult.targetX).to.be.closeTo(sampleOriginal.targetX, 1e-4, `Mismatch in x-coord at path entry ${j}`); - expect(sampleResult.targetY).to.be.closeTo(sampleOriginal.targetY, 1e-4, `Mismatch in y-coord at path entry ${j}`); + expect(sampleResult.targetX).to.be.closeTo(sampleOriginal.targetX, 1e-4, `Mismatch in x-coord at path entry ${j}, path ${i}`); + expect(sampleResult.targetY).to.be.closeTo(sampleOriginal.targetY, 1e-4, `Mismatch in y-coord at path entry ${j}, path ${i}`); } } diff --git a/common/web/gesture-recognizer/src/test/auto/headless/asyncClosureDispatchQueue.spec.ts b/common/web/gesture-recognizer/src/test/auto/headless/asyncClosureDispatchQueue.spec.ts new file mode 100644 index 00000000000..6b684968aa1 --- /dev/null +++ b/common/web/gesture-recognizer/src/test/auto/headless/asyncClosureDispatchQueue.spec.ts @@ -0,0 +1,236 @@ +import { assert } from 'chai' +import { default as sinon, type SinonSpy } from 'sinon'; +import { AsyncClosureDispatchQueue, type QueueClosure } from '@keymanapp/gesture-recognizer'; +import { ManagedPromise, timedPromise } from '@keymanapp/web-utils'; + +type ClosureSpy = SinonSpy<[], ReturnType>; + +describe('AsyncClosureDispatchQueue', () => { + it('has expected initial state', () => { + const queue = new AsyncClosureDispatchQueue(); + assert.isTrue(queue.ready); + }); + + it('proper handling of simple closure when queue is empty (default config)', async () => { + const queue = new AsyncClosureDispatchQueue(); + + const fake = sinon.fake(); + queue.runAsync(() => { + fake(queue.ready); + }); + + assert.isFalse(queue.ready); + // If the queue was ready, this should be called immediately. + assert.isTrue(fake.called); + + // Default delay between entries: a macroqueue task (i.e., setTimeout(0)) + await timedPromise(0); + + assert.isTrue(queue.ready); + assert.isTrue(fake.called); + + // During the actual closure call, the queue is still awaiting async completion of the closure. + // Default wait: a macroqueue task + assert.isFalse(fake.firstCall.args[0]); + }); + + it('proper handling of Promise-returning closure when queue is empty (default config)', async () => { + const queue = new AsyncClosureDispatchQueue(); + + const lock = new ManagedPromise(); + const fake = sinon.fake(); + queue.runAsync(() => { + fake(queue.ready); + return lock.corePromise; + }); + + assert.isFalse(queue.ready); + // If the queue was ready, this should be called immediately. + assert.isTrue(fake.called); + assert.isFalse(fake.firstCall.args[0]); + + await timedPromise(50); + + assert.isFalse(queue.ready); + + lock.resolve(); + + // Allow the newly-resolved Promise to chain. + // ("White-box" info here, but once is enough.) + await Promise.resolve(); + + assert.isTrue(queue.ready); + assert.isTrue(fake.called); + }); + + it('proper handling of simple closure when queue is not empty (default config)', async () => { + const queue = new AsyncClosureDispatchQueue(); + + const fakeTimers = sinon.useFakeTimers(); + + const lock = new ManagedPromise(); + queue.runAsync(() => lock.corePromise); + + const fake = sinon.fake(); + queue.runAsync(() => { + fake(queue.ready); + }); + + try { + assert.isFalse(queue.ready); + assert.isFalse(fake.called); + + // Doesn't matter how long we wait; there's still a pending entry in front of `fake`. + await fakeTimers.tickAsync(50); + + assert.isFalse(queue.ready); + assert.isFalse(fake.called); + + // Allow that pending entry to resolve; `fake` should be able to resolve afterward with little issue. + lock.resolve(); + await fakeTimers.tickAsync(50); + + assert.isTrue(queue.ready); + assert.isTrue(fake.called); + + // During the actual closure call, the queue is still awaiting async completion of the closure. + // Default wait: a macroqueue task + assert.isFalse(fake.firstCall.args[0]); + } finally { + fakeTimers.restore(); + } + }); + + it('complex case 1 - many tasks, all queued at the same time', async () => { + // Uses the default timeout between events; just making it extra-explicit here. + const queue = new AsyncClosureDispatchQueue(() => { return timedPromise(0) }); + + const buildSet = (n: number) => { + let set: ClosureSpy[] = []; + + // Deliberately using the same one multiple times - the class gives us a call count. + let closure = sinon.spy(() => {}); + for(let i=0; i < n; i++) { + set.push(closure); + } + + return set; + } + + const set0 = buildSet(3); + const lock0 = new ManagedPromise(); + const set1 = buildSet(7); + const lock1 = new ManagedPromise(); + const set2 = buildSet(5); + + const fakeTimers = sinon.useFakeTimers(); + + set0.forEach((entry) => queue.runAsync(entry)); + queue.runAsync(() => { + return lock0.corePromise; + }); + set1.forEach((entry) => queue.runAsync(entry)); + queue.runAsync(() => { + return lock1.corePromise; + }) + set2.forEach((entry) => queue.runAsync(entry)); + + assert.isFalse(queue.ready); + + try { + // Run set0; it'll stop before set1 due to lock0 not being resolved. + await fakeTimers.tickAsync(50); + + assert.equal(set0[0].callCount, 3); + assert.equal(set1[0].callCount, 0); + assert.equal(set2[0].callCount, 0); + + // Now we run set1; it'll stop before set2 due to lock1 not being resolved. + lock0.resolve(); + await fakeTimers.tickAsync(50); + + assert.equal(set0[0].callCount, 3); + assert.equal(set1[0].callCount, 7); + assert.equal(set2[0].callCount, 0); + + // Now we run set2, flushing out the queue. + lock1.resolve(); + await fakeTimers.tickAsync(50); + + assert.equal(set0[0].callCount, 3); + assert.equal(set1[0].callCount, 7); + assert.equal(set2[0].callCount, 5); + + assert.isTrue(queue.ready); + } finally { + fakeTimers.restore(); + } + }); + + it('complex case 2 - queued closure promises "unlocking" out of order', async () => { + // Uses the default timeout between events; just making it extra-explicit here. + const queue = new AsyncClosureDispatchQueue(() => { return timedPromise(0) }); + + const buildSet = (n: number) => { + let set: ClosureSpy[] = []; + + // Deliberately using the same one multiple times - the class gives us a call count. + let closure = sinon.spy(() => {}); + for(let i=0; i < n; i++) { + set.push(closure); + } + + return set; + } + + const fakeTimers = sinon.useFakeTimers(); + + const set0 = buildSet(3); + const lock0 = new ManagedPromise(); + const set1 = buildSet(7); + const lock1 = new ManagedPromise(); + const set2 = buildSet(5); + + set0.forEach((entry) => queue.runAsync(entry)); + queue.runAsync(() => { + return lock0.corePromise; + }); + set1.forEach((entry) => queue.runAsync(entry)); + queue.runAsync(() => { + return lock1.corePromise; + }) + set2.forEach((entry) => queue.runAsync(entry)); + + assert.isFalse(queue.ready); + + try { + // Run set0; it'll stop before set1 due to lock0 not being resolved. + await fakeTimers.tickAsync(50); + + assert.equal(set0[0].callCount, 3); + assert.equal(set1[0].callCount, 0); + assert.equal(set2[0].callCount, 0); + + // Now we resolve lock1 - but this isn't what is currently blocking the queue. + // No new tasks should run. + lock1.resolve(); /* NOTE: is being unlocked before lock0, which is earlier! */ + await fakeTimers.tickAsync(50); + + assert.equal(set0[0].callCount, 3); + assert.equal(set1[0].callCount, 0); + assert.equal(set2[0].callCount, 0); + + // Now we resolve lock0, allowing both set1 and set2 to complete. + lock0.resolve(); + await fakeTimers.tickAsync(50); + + assert.equal(set0[0].callCount, 3); + assert.equal(set1[0].callCount, 7); + assert.equal(set2[0].callCount, 5); + + assert.isTrue(queue.ready); + } finally { + fakeTimers.restore(); + } + }); +}); \ No newline at end of file diff --git a/common/web/gesture-recognizer/src/tools/unit-test-resources/src/headlessInputEngine.ts b/common/web/gesture-recognizer/src/tools/unit-test-resources/src/headlessInputEngine.ts index 7252436929b..85c65c90970 100644 --- a/common/web/gesture-recognizer/src/tools/unit-test-resources/src/headlessInputEngine.ts +++ b/common/web/gesture-recognizer/src/tools/unit-test-resources/src/headlessInputEngine.ts @@ -85,7 +85,7 @@ export class HeadlessInputEngine extends InputEngineBase { const playbackTerminations = recordedObj.inputs.map((recording, index) => this.playbackTerminations(sources[index], recording)); playbackStarts.forEach((promise) => promise.then(() => { - this.maintainTouchpointsWithIds(playbackStartTuples.map((tuple) => tuple.internal_id)); + this.maintainTouchpoints(playbackStartTuples.map((tuple) => tuple.source)); }) ); diff --git a/common/web/gesture-recognizer/src/tools/unit-test-resources/src/inputSequenceSimulator.ts b/common/web/gesture-recognizer/src/tools/unit-test-resources/src/inputSequenceSimulator.ts index dfc65bfaafb..d698ad6b5dd 100644 --- a/common/web/gesture-recognizer/src/tools/unit-test-resources/src/inputSequenceSimulator.ts +++ b/common/web/gesture-recognizer/src/tools/unit-test-resources/src/inputSequenceSimulator.ts @@ -111,7 +111,8 @@ export class InputSequenceSimulator { let touchEventDict: TouchEventInit = { bubbles: true, - touches: changedTouches.concat(otherTouches), + // Ending touchpoints should NOT show up in `touches`. + touches: state == 'end' ? otherTouches : changedTouches.concat(otherTouches), changedTouches: changedTouches, } diff --git a/common/web/keyboard-processor/src/text/codes.ts b/common/web/keyboard-processor/src/text/codes.ts index 11490f32f2d..3f1ebe7afe8 100644 --- a/common/web/keyboard-processor/src/text/codes.ts +++ b/common/web/keyboard-processor/src/text/codes.ts @@ -1,7 +1,7 @@ // TODO: Move to separate folder: 'codes' // We should start splitting off code needed by keyboards even without a KeyboardProcessor active. -// There's an upcoming `/common/web/types` package that 'codes' and 'keyboards' may fit well within. -// In fact, there's a file there (on its branch) that should be merged with this one! + +// see also: common/web/types/src/kmx/kmx.ts const Codes = { // Define Keyman Developer modifier bit-flags (exposed for use by other modules) @@ -25,6 +25,10 @@ const Codes = { "NO_SCROLL_LOCK":0x2000, // NOTSCROLLFLAG "VIRTUAL_KEY":0x4000, // ISVIRTUALKEY "VIRTUAL_CHAR_KEY":0x8000 // VIRTUALCHARKEY // Unused by KMW, but reserved for use by other Keyman engines. + + // Note: keys_mod_other = 0x10000, used by KMX+ for the + // other modifier flag in layers, > 16 bit so not available here. + // See keys_mod_other in keyman_core_ldml.ts }, modifierBitmasks: { @@ -168,4 +172,4 @@ const Codes = { } } -export default Codes; \ No newline at end of file +export default Codes; diff --git a/common/web/keyboard-processor/src/text/outputTarget.ts b/common/web/keyboard-processor/src/text/outputTarget.ts index ab9656b6999..ef3a3d74e33 100644 --- a/common/web/keyboard-processor/src/text/outputTarget.ts +++ b/common/web/keyboard-processor/src/text/outputTarget.ts @@ -13,7 +13,7 @@ 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; } @@ -21,15 +21,17 @@ export function isEmptyTransform(transform: Transform) { 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 { diff --git a/common/web/keyboard-processor/tests/node/transcriptions.js b/common/web/keyboard-processor/tests/node/transcriptions.js index 6d6be7b540f..b071532bf2a 100644 --- a/common/web/keyboard-processor/tests/node/transcriptions.js +++ b/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); diff --git a/common/web/lm-worker/src/main/correction/context-tracker.ts b/common/web/lm-worker/src/main/correction/context-tracker.ts index 16e4d627805..b25f7274401 100644 --- a/common/web/lm-worker/src/main/correction/context-tracker.ts +++ b/common/web/lm-worker/src/main/correction/context-tracker.ts @@ -406,6 +406,22 @@ export class ContextTracker extends CircularArray { * - For languages using whitespace to word-break, said keystroke would have to include said whitespace to break the assumption. */ + function maintainLastToken() { + if(isWhitespace && editPath[tailIndex] == 'match') { + /* + We can land here if there are multiple whitespaces in a row. + There's already an implied whitespace to the left, so we conceptually + merge the new whitespace with that one. + */ + return state; + } else if(isBackspace) { + // Consider backspace entry for this case? + state.replaceTailForBackspace(finalToken, primaryInput.id); + } else { + state.updateTail(primaryInput ? transformDistribution : null, finalToken); + } + } + // If there is/was more than one context token available... if(editPath.length > 1) { // We're removing a context token, but at least one remains. @@ -438,14 +454,11 @@ export class ContextTracker extends CircularArray { } state.pushTail(pushedToken); - } else { // We're editing the final context token. + } else { + // We're editing the final context token. // TODO: Assumption: we didn't 'miss' any inputs somehow. // As is, may be prone to fragility should the lm-layer's tracked context 'desync' from its host's. - if(isBackspace) { - state.replaceTailForBackspace(finalToken, primaryInput.id); - } else { - state.updateTail(primaryInput ? transformDistribution : null, finalToken); - } + maintainLastToken(); } // There is only one word in the context. } else { @@ -458,13 +471,9 @@ export class ContextTracker extends CircularArray { token.raw = tokenizedContext[0]; token.transformDistributions = [transformDistribution]; state.pushTail(token); - } else { // Edit the lone context token. - // Consider backspace entry for this case? - if(isBackspace) { - state.replaceTailForBackspace(finalToken, primaryInput.id); - } else { - state.updateTail(primaryInput ? transformDistribution : null, finalToken); - } + } else { + // Edit the lone context token. + maintainLastToken(); } } return state; diff --git a/common/web/types/src/kmx/element-string.ts b/common/web/types/src/kmx/element-string.ts index 578e0f9e1ae..b26878f459e 100644 --- a/common/web/types/src/kmx/element-string.ts +++ b/common/web/types/src/kmx/element-string.ts @@ -74,6 +74,11 @@ export class ElementString extends Array { typeFlag |= constants.elem_flags_type_uset; // TODO-LDML: err on max buffer size const needRanges = sections.usetparser.sizeUnicodeSet(item.segment); + if (needRanges < 0) { + // Note that sizeUnicodeSet() already will notify via callback if there's an + // error. So we can just exit here. + return null; // UnicodeSet error + } const uset = sections.usetparser.parseUnicodeSet(item.segment, needRanges); if (!uset) { return null; // UnicodeSet error already thrown diff --git a/common/web/types/src/kmx/kmx-plus.ts b/common/web/types/src/kmx/kmx-plus.ts index 6b485a2b2fe..6e3c73d33c8 100644 --- a/common/web/types/src/kmx/kmx-plus.ts +++ b/common/web/types/src/kmx/kmx-plus.ts @@ -2,7 +2,7 @@ import { constants } from '@keymanapp/ldml-keyboard-constants'; import * as r from 'restructure'; import { ElementString } from './element-string.js'; import { ListItem } from './string-list.js'; -import { isOneChar, toOneChar, unescapeString } from '../util/util.js'; +import { isOneChar, toOneChar, unescapeString, escapeStringForRegex } from '../util/util.js'; import { KMXFile } from './kmx.js'; import { UnicodeSetParser, UnicodeSet } from '@keymanapp/common-types'; import { VariableParser } from '../ldml-keyboard/pattern-parser.js'; @@ -272,7 +272,7 @@ export class Vars extends Section { return v.value.value; // string value }); } - substituteStrings(str: string, sections: DependencySections): string { + substituteStrings(str: string, sections: DependencySections, forMatch?: boolean): string { if (!str) return str; return str.replaceAll(VariableParser.STRING_REFERENCE, (_entire, id) => { const val = this.findStringVariableValue(id); @@ -280,6 +280,7 @@ export class Vars extends Section { // Should have been caught during validation. throw Error(`Internal Error: reference to missing string variable ${id}`); } + if (forMatch) return escapeStringForRegex(val); return val; }); } @@ -292,8 +293,9 @@ export class Vars extends Section { const set = Vars.findVariable(this.sets, id); if (set !== null) { const { items } = set; - const inner = items.map(i => i.value.value).join('|'); - return `(?:${inner})`; // TODO-LDML: need to escape here + const escapedStrings = items.map(v => escapeStringForRegex(v.value.value)); + const inner = escapedStrings.join('|'); + return `(?:${inner})`; } // try as unicodeset @@ -372,11 +374,15 @@ export class UnicodeSetItem extends VarsItem { }; export class SetVarItem extends VarsItem { - constructor(id: string, value: string[], sections: DependencySections) { + constructor(id: string, value: string[], sections: DependencySections, rawItems: string[]) { super(id, value.join(' '), sections); this.items = sections.elem.allocElementString(sections, value); + this.rawItems = rawItems; } + // element string array items: ElementString; + // like items, but with unprocessed marker strings + rawItems: string[]; valid() : boolean { return !!this.items; } diff --git a/common/web/types/src/kmx/kmx.ts b/common/web/types/src/kmx/kmx.ts index 6102a69cf55..c5989ec1157 100644 --- a/common/web/types/src/kmx/kmx.ts +++ b/common/web/types/src/kmx/kmx.ts @@ -345,6 +345,10 @@ export class KMXFile { public static readonly ISVIRTUALKEY = 0x4000; // It is a Virtual Key Sequence public static readonly VIRTUALCHARKEY = 0x8000; // Keyman 6.0: Virtual Key Cap Sequence NOT YET + // Note: OTHER_MODIFIER = 0x10000, used by KMX+ for the + // other modifier flag in layers, > 16 bit so not available here. + // See keys_mod_other in keyman_core_ldml.ts + public static readonly MASK_MODIFIER_CHIRAL = KMXFile.LCTRLFLAG | KMXFile.RCTRLFLAG | KMXFile.LALTFLAG | KMXFile.RALTFLAG; public static readonly MASK_MODIFIER_SHIFT = KMXFile.K_SHIFTFLAG; public static readonly MASK_MODIFIER_NONCHIRAL = KMXFile.K_CTRLFLAG | KMXFile.K_ALTFLAG; @@ -457,4 +461,4 @@ export class KMXFile { throw "COMP_KEYBOARD size is "+this.COMP_KEYBOARD.size()+" but should be "+KMXFile.COMP_KEYBOARD_SIZE+" bytes"; } } -} \ No newline at end of file +} diff --git a/common/web/types/src/util/util.ts b/common/web/types/src/util/util.ts index 13057bc3384..c6a6fa55820 100644 --- a/common/web/types/src/util/util.ts +++ b/common/web/types/src/util/util.ts @@ -127,7 +127,7 @@ export function escapeRegexChar(ch: string) { } /** chars that must be escaped: syntax, C0 + C1 controls */ -const REGEX_SYNTAX_CHAR = /^[\u0000-\u001F\u007F-\u009F{}\[\]\\?.^$*()/-]$/; +const REGEX_SYNTAX_CHAR = /^[\u0000-\u001F\u007F-\u009F{}\[\]\\?|.^$*()/-]$/; function escapeRegexCharIfSyntax(ch: string) { // escape if syntax or not valid @@ -148,6 +148,13 @@ function regexOne(hex: string): string { // re-escape as 16 or 32 bit code units return Array.from(unescaped).map(ch => escapeRegexCharIfSyntax(ch)).join(''); } +/** + * Escape a string (\uxxxx form) if there are any problematic codepoints + */ +export function escapeStringForRegex(s: string) : string { + return s.split('').map(ch => escapeRegexCharIfSyntax(ch)).join(''); +} + /** * Unescapes a string according to UTS#18§1.1, see * @param s escaped string diff --git a/common/windows/cpp/include/legacy_kmx_file.h b/common/windows/cpp/include/legacy_kmx_file.h index 9b8f6a59fb1..d3bc3aed0f6 100644 --- a/common/windows/cpp/include/legacy_kmx_file.h +++ b/common/windows/cpp/include/legacy_kmx_file.h @@ -320,6 +320,10 @@ #define K_MODIFIERFLAG 0x007F #define K_NOTMODIFIERFLAG 0xFF00 // I4548 +// Note: OTHER_MODIFIER = 0x10000, used by KMX+ for the +// other modifier flag in layers, > 16 bit so not available here. +// See keys_mod_other in keyman_core_ldml.ts + /* These sanity checks help ensure we don't break on-disk struct sizes when we cross diff --git a/common/windows/delphi/general/Upload_Settings.pas b/common/windows/delphi/general/Upload_Settings.pas index f38cc078903..d5347f4fb9e 100644 --- a/common/windows/delphi/general/Upload_Settings.pas +++ b/common/windows/delphi/general/Upload_Settings.pas @@ -94,6 +94,7 @@ implementation DebugPaths, ErrorControlledRegistry, RegistryKeys, + utilhttp, VersionInfo; const @@ -169,7 +170,7 @@ function URLPath_PackageDownload(const PackageID, BCP47: string; IsUpdate: Boole begin if IsUpdate then IsUpdateInt := 1 else IsUpdateInt := 0; - Result := Format(URLPath_PackageDownload_Format, [PackageID, CKeymanVersionInfo.Tier, BCP47, IsUpdateInt]); + Result := Format(URLPath_PackageDownload_Format, [URLEncode(PackageID), URLEncode(CKeymanVersionInfo.Tier), URLEncode(BCP47), IsUpdateInt]); end; end. diff --git a/common/windows/delphi/keyboards/kmxfileconsts.pas b/common/windows/delphi/keyboards/kmxfileconsts.pas index 966dada8827..2a32184ec04 100644 --- a/common/windows/delphi/keyboards/kmxfileconsts.pas +++ b/common/windows/delphi/keyboards/kmxfileconsts.pas @@ -129,6 +129,10 @@ interface KMX_ISVIRTUALKEY = $4000; // It is a Virtual Key Sequence KMX_VIRTUALCHARKEY = $8000; // It is a virtual character key sequence - mnemonic layouts + // Note: KMX_OTHER_MODIFIER = $10000, used by KMX+ for the + // other modifier flag in layers, > 16 bit so not available here. + // See keys_mod_other in keyman_core_ldml.ts + // Combinations of key masks KMX_MASK_MODIFIER_CHIRAL = KMX_LCTRLFLAG or KMX_RCTRLFLAG or KMX_LALTFLAG or KMX_RALTFLAG; KMX_MASK_MODIFIER_SHIFT = KMX_SHIFTFLAG; diff --git a/core/include/ldml/keyman_core_ldml.h b/core/include/ldml/keyman_core_ldml.h index b498065acaa..1d8aaa2575d 100644 --- a/core/include/ldml/keyman_core_ldml.h +++ b/core/include/ldml/keyman_core_ldml.h @@ -34,7 +34,7 @@ #define LDML_FINL_FLAGS_ERROR 0x1 #define LDML_KEYS_KEY_FLAGS_EXTEND 0x1 #define LDML_KEYS_KEY_FLAGS_GAP 0x2 -#define LDML_KEYS_MOD_ALL 0x17F +#define LDML_KEYS_MOD_ALL 0x1017F #define LDML_KEYS_MOD_ALT 0x40 #define LDML_KEYS_MOD_ALTL 0x4 #define LDML_KEYS_MOD_ALTR 0x8 @@ -43,6 +43,7 @@ #define LDML_KEYS_MOD_CTRLL 0x1 #define LDML_KEYS_MOD_CTRLR 0x2 #define LDML_KEYS_MOD_NONE 0x0 +#define LDML_KEYS_MOD_OTHER 0x10000 #define LDML_KEYS_MOD_SHIFT 0x10 #define LDML_LAYR_LIST_HARDWARE_TOUCH "touch" #define LDML_LENGTH_BKSP 0xC diff --git a/core/include/ldml/keyman_core_ldml.ts b/core/include/ldml/keyman_core_ldml.ts index ca9654587b5..b649d9424e7 100644 --- a/core/include/ldml/keyman_core_ldml.ts +++ b/core/include/ldml/keyman_core_ldml.ts @@ -270,6 +270,11 @@ class Constants { */ readonly keys_mod_shift = 0x0010; + /** + * bitmask for 'other'. + */ + readonly keys_mod_other = 0x10000; + /** * Convenience map for modifiers */ @@ -284,6 +289,7 @@ class Constants { ["ctrlL", this.keys_mod_ctrlL], ["ctrlR", this.keys_mod_ctrlR], ["shift", this.keys_mod_shift], + ["other", this.keys_mod_other], ] ); diff --git a/core/src/actions_normalize.cpp b/core/src/actions_normalize.cpp index a2f3db5433a..3384fda9757 100644 --- a/core/src/actions_normalize.cpp +++ b/core/src/actions_normalize.cpp @@ -119,39 +119,36 @@ bool km::core::actions_normalize( normalization in our output, we now need to look for a normalization boundary prior to the intersection of the cached_context and the output. */ + if(!output.isEmpty()) { + while(n > 0 && !nfd->hasBoundaryBefore(output[0])) { + // The output may interact with the context further in normalization. We + // need to copy characters back further until we reach a normalization + // boundary. - while(n > 0 && output[0] && !nfd->hasBoundaryBefore(output[0])) { - // The output may interact with the context further in normalization. We - // need to copy characters back further until we reach a normalization - // boundary. + // Remove last code point from the context ... - // Remove last code point from the context ... + n = cached_context_string.moveIndex32(n, -1); + UChar32 chr = cached_context_string.char32At(n); + cached_context_string.remove(n); - n = cached_context_string.moveIndex32(n, -1); - UChar32 chr = cached_context_string.char32At(n); - cached_context_string.remove(n); + // And prepend it to the output ... - // And prepend it to the output ... - - output.insert(0, chr); - - // And finally remember that we now need to delete an additional NFD codepoint - - actions.code_points_to_delete++; + output.insert(0, chr); + } } /* At this point, our output and cached_context are coherent and normalization will be complete at the edit boundary. - Now, we need to adjust the delete_back to match the number of characters + Now, we need to adjust the delete_back to match the number of codepoints that must actually be deleted from the applications's NFU context - To adjust, we remove one character at a time from the app_context until + To adjust, we remove one codepoint at a time from the app_context until its normalized form matches the cached_context normalized form. */ - while(app_context_string.length()) { + while(app_context_string.countChar32()) { icu::UnicodeString app_context_nfd; nfd->normalize(app_context_string, app_context_nfd, icu_status); assert(U_SUCCESS(icu_status)); @@ -163,7 +160,13 @@ bool km::core::actions_normalize( if(app_context_nfd.compare(cached_context_string) == 0) { break; } - app_context_string.remove(app_context_string.length()-1); + + // remove the last UChar32 + int32_t lastUChar32 = app_context_string.length()-1; + // adjust pointer to get the entire char (i.e. so we don't slice a non-BMP char) + lastUChar32 = app_context_string.getChar32Start(lastUChar32); + // remove the UChar32 (1 or 2 code units) + app_context_string.remove(lastUChar32); nfu_to_delete++; } @@ -319,4 +322,4 @@ bool km::core::actions_update_app_context_nfu( delete [] items; return status == KM_CORE_STATUS_OK; -} \ No newline at end of file +} diff --git a/core/src/km_core_action_api.cpp b/core/src/km_core_action_api.cpp index 98ee0f11e7b..6026bad0e54 100644 --- a/core/src/km_core_action_api.cpp +++ b/core/src/km_core_action_api.cpp @@ -56,7 +56,9 @@ void km::core::actions_dispose( km_core_usv const *km::core::get_deleted_context(context const &app_context, unsigned int code_points_to_delete) { auto p = app_context.end(); - for(size_t i = code_points_to_delete; i > 0; i--, p--); + for(size_t i = code_points_to_delete; i > 0; i--, p--) { + assert(p != app_context.begin()); + } auto deleted_context = new km_core_usv[code_points_to_delete + 1]; for(size_t i = 0; i < code_points_to_delete; i++) { diff --git a/core/src/km_core_state_context_set_if_needed.cpp b/core/src/km_core_state_context_set_if_needed.cpp index fd6106fa7ae..7107815249f 100644 --- a/core/src/km_core_state_context_set_if_needed.cpp +++ b/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); diff --git a/core/src/kmx/kmx_plus.cpp b/core/src/kmx/kmx_plus.cpp index 295d92a4b24..007a87cdf7d 100644 --- a/core/src/kmx/kmx_plus.cpp +++ b/core/src/kmx/kmx_plus.cpp @@ -42,6 +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 +// LDML_KEYS_MOD_OTHER is not present in kmx_file.h (>16 bit) /** * \def LDML_IS_VALID_MODIFIER_BITS test whether x is a valid modifier bitfield diff --git a/core/src/ldml/C7043_ldml.md b/core/src/ldml/C7043_ldml.md index 8714e01f023..b1d2223d556 100644 --- a/core/src/ldml/C7043_ldml.md +++ b/core/src/ldml/C7043_ldml.md @@ -461,17 +461,18 @@ For each key: by the compiler. - `mod`: 32-bit bitfield defined as below. Little endian values. -| Value | Meaning |`kmx_file.h` | Comment | -|----------|----------|---------------|---------------------------------------------| -| 0x0000 | `none` | | All zeros = no modifiers | -| 0x0001 | `ctrlL` | `LCTRLFLAG` | Left Control | -| 0x0002 | `ctrlR` | `RCTRLFLAG` | Right Control | -| 0x0004 | `altL` | `LALTFLAG` | Left Alt | -| 0x0008 | `altR` | `RALTFLAG` | Right Alt | -| 0x0010 | `shift` | `K_SHIFTFLAG` | Either Shift | -| 0x0020 | `ctrl` | `K_CTRLFLAG` | Either Control | -| 0x0040 | `alt` | `K_ALTFLAG` | Either Alt | -| 0x0100 | `caps` | `CAPITALFLAG` | Caps lock | +| Value | Meaning |`kmx_file.h` | Comment | +|----------|-----------|--------------------|-----------------------------------------------| +| 0x0000 | `none` | | All zeros = no modifiers | +| 0x0001 | `ctrlL` | `LCTRLFLAG` | Left Control | +| 0x0002 | `ctrlR` | `RCTRLFLAG` | Right Control | +| 0x0004 | `altL` | `LALTFLAG` | Left Alt | +| 0x0008 | `altR` | `RALTFLAG` | Right Alt | +| 0x0010 | `shift` | `K_SHIFTFLAG` | Either Shift | +| 0x0020 | `ctrl` | `K_CTRLFLAG` | Either Control | +| 0x0040 | `alt` | `K_ALTFLAG` | Either Alt | +| 0x0100 | `caps` | `CAPITALFLAG` | Caps lock | +| 0x10000 | `other` | n/a | Other (not used in conjunction with others) | TODO-LDML: Note that conforming to other keyman values, left versus right shift cannot be distinguished. diff --git a/core/src/ldml/ldml_processor.cpp b/core/src/ldml/ldml_processor.cpp index 166b8002b86..9667a2275af 100644 --- a/core/src/ldml/ldml_processor.cpp +++ b/core/src/ldml/ldml_processor.cpp @@ -377,6 +377,11 @@ ldml_event_state::emit_difference(const std::u32string &old_ctxt, const std::u32 // So the BBBBB needs to be removed and then CCC added. auto ctxt_prefix = mismatch(old_ctxt.begin(), old_ctxt.end(), new_ctxt.begin(), new_ctxt.end()); + // is the 'mismatch' at the end (i.e., no mismatch)? + if(ctxt_prefix.first == old_ctxt.end() && ctxt_prefix.second == new_ctxt.end()) { + return; // Optimization: We can just exit, there's nothing to do. + } + // handle a special case where we're simply changing from one marker to another. // Example: // 0. old_ctxtstr_changed ends with … U+FFFF U+0008 | U+0001 … @@ -390,14 +395,14 @@ ldml_event_state::emit_difference(const std::u32string &old_ctxt, const std::u32 // marker change. // We can detect this because the unchanged_prefix will end with u+FFFF U+0008 // - // Oh, and yes, test case 'regex-test-8a-0' hits this. + // k_212* and k_213* hit this case. std::u32string common_prefix(old_ctxt.begin(), ctxt_prefix.first); if (common_prefix.length() >= 2) { auto iter = common_prefix.rbegin(); if (*(iter++) == LDML_MARKER_CODE && *(iter++) == UC_SENTINEL) { // adjust the iterator so that the "U+FFFF U+0008" is not a part of the common prefix. - ctxt_prefix.first -= 2; - ctxt_prefix.second += 2; + ctxt_prefix.first -= 2; // backup the 'mismatch' point to before the FFFF + ctxt_prefix.second -= 2; // backup the 'mismatch' point to before the FFFF // Now, old_ctxtstr_changed and new_ctxtstr_changed will start with U+FFFF U+0008 … } } diff --git a/core/src/ldml/ldml_vkeys.cpp b/core/src/ldml/ldml_vkeys.cpp index 156a4c3247e..07a2ff08ad9 100644 --- a/core/src/ldml/ldml_vkeys.cpp +++ b/core/src/ldml/ldml_vkeys.cpp @@ -7,6 +7,7 @@ #include "ldml_vkeys.hpp" #include "kmx_file.h" +#include namespace km { namespace core { @@ -65,6 +66,16 @@ vkeys::lookup(km_core_virtual_key vk, uint16_t modifier_state, bool &found) cons return ret; } } + + // look for a layer with "other" + { + const vkey_id id_default(vk, (LDML_KEYS_MOD_OTHER)); + ret = lookup(id_default, found); + if (found) { + return ret; + } + } + // default: return failure. found=false. return ret; } diff --git a/core/tests/unit/kmnkbd/test_actions_normalize.cpp b/core/tests/unit/kmnkbd/test_actions_normalize.cpp index 8ae556c7e66..02bf604ee35 100644 --- a/core/tests/unit/kmnkbd/test_actions_normalize.cpp +++ b/core/tests/unit/kmnkbd/test_actions_normalize.cpp @@ -152,7 +152,7 @@ void test_actions_normalize( * this is initial_cached_context - * actions_code_points_to_delete + * actions_output) - no markers supported. - * If specified, final_cached_context_items + * If specified, final_cached_context_items * must be nullptr. * @param final_cached_context_items cached context _after_ actions have been * applied -- NFU (essentially, @@ -440,6 +440,23 @@ void run_actions_normalize_tests() { /* app_context: */ u"a\U0001F607bca\U0001F60E" ); + km_core_context_item items_11067[] = { + { KM_CORE_CT_CHAR, {0,}, { U'𐒻' } }, + { KM_CORE_CT_CHAR, {0,}, { U'𐒷' } }, + KM_CORE_CONTEXT_ITEM_END + }; + + // regression #11067 + test_actions_normalize( + "A non-BMP char in context (#11067)", + /* app context pre transform: */ u"𐒻", + /* cached context post transform: */ u"𐒻𐒷", + /* cached context post transform: */ &items_11067[0], + /* action del, output: */ 0, U"𐒻𐒷", + // ---- results ---- + /* action del, output: */ 1, U"𐒻𐒷", + /* app_context: */ u"𐒻𐒷" + ); } void run_actions_update_app_context_nfu_tests() { diff --git a/core/tests/unit/ldml/keyboards/k_007_transform_rgx-test.xml b/core/tests/unit/ldml/keyboards/k_007_transform_rgx-test.xml index 80f3bbafabe..f4024275e8d 100644 --- a/core/tests/unit/ldml/keyboards/k_007_transform_rgx-test.xml +++ b/core/tests/unit/ldml/keyboards/k_007_transform_rgx-test.xml @@ -88,4 +88,56 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/core/tests/unit/ldml/keyboards/k_007_transform_rgx.xml b/core/tests/unit/ldml/keyboards/k_007_transform_rgx.xml index 18356b2c3e3..cc79b826de6 100644 --- a/core/tests/unit/ldml/keyboards/k_007_transform_rgx.xml +++ b/core/tests/unit/ldml/keyboards/k_007_transform_rgx.xml @@ -12,6 +12,9 @@ from https://github.com/unicode-org/cldr/blob/keyboard-preview/docs/ldml/tr35-ke + + + @@ -26,7 +29,7 @@ from https://github.com/unicode-org/cldr/blob/keyboard-preview/docs/ldml/tr35-ke - + @@ -34,6 +37,13 @@ from https://github.com/unicode-org/cldr/blob/keyboard-preview/docs/ldml/tr35-ke + + + + + + + @@ -45,6 +55,11 @@ from https://github.com/unicode-org/cldr/blob/keyboard-preview/docs/ldml/tr35-ke + + + + + diff --git a/core/tests/unit/ldml/keyboards/k_012_other.xml b/core/tests/unit/ldml/keyboards/k_012_other.xml new file mode 100644 index 00000000000..186cfd88699 --- /dev/null +++ b/core/tests/unit/ldml/keyboards/k_012_other.xml @@ -0,0 +1,27 @@ + + + + + + + + + + + + + + + + + + + + diff --git a/core/tests/unit/ldml/keyboards/k_210_marker-test.xml b/core/tests/unit/ldml/keyboards/k_210_marker-test.xml index df6b65422b6..5a36d1941b3 100644 --- a/core/tests/unit/ldml/keyboards/k_210_marker-test.xml +++ b/core/tests/unit/ldml/keyboards/k_210_marker-test.xml @@ -50,4 +50,16 @@ + + + + + + + + + + + + diff --git a/core/tests/unit/ldml/keyboards/k_210_marker.xml b/core/tests/unit/ldml/keyboards/k_210_marker.xml index 4d3d66ce986..0d25de75ac6 100644 --- a/core/tests/unit/ldml/keyboards/k_210_marker.xml +++ b/core/tests/unit/ldml/keyboards/k_210_marker.xml @@ -24,12 +24,20 @@ - + + + + + + + + + diff --git a/core/tests/unit/ldml/keyboards/k_212_marker_11057-test.xml b/core/tests/unit/ldml/keyboards/k_212_marker_11057-test.xml new file mode 100644 index 00000000000..92e2abbf3f0 --- /dev/null +++ b/core/tests/unit/ldml/keyboards/k_212_marker_11057-test.xml @@ -0,0 +1,23 @@ + + + + + + + + + + + + + + + + + + + + + + + diff --git a/core/tests/unit/ldml/keyboards/k_212_marker_11057.xml b/core/tests/unit/ldml/keyboards/k_212_marker_11057.xml new file mode 100644 index 00000000000..7bbd2bb94ac --- /dev/null +++ b/core/tests/unit/ldml/keyboards/k_212_marker_11057.xml @@ -0,0 +1,30 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/core/tests/unit/ldml/keyboards/meson.build b/core/tests/unit/ldml/keyboards/meson.build index ca047a67bc6..e27fb39b3ef 100644 --- a/core/tests/unit/ldml/keyboards/meson.build +++ b/core/tests/unit/ldml/keyboards/meson.build @@ -16,6 +16,7 @@ tests_from_cldr = [ 'bn', ] +# these have 'embedded' (@@) testdata instead of a separate file tests_without_testdata = [ # disabling 000 until we have updates to core or to the keyboard so that it passes # 'k_000_null_keyboard', @@ -25,13 +26,13 @@ tests_without_testdata = [ 'k_005_modbittest', 'k_010_mt', 'k_011_mt_iso', + 'k_012_other', 'k_100_keytest', 'k_101_keytest', 'k_102_keytest', - ] -# These tests have a k_001_tiny-test.xml file as well. +# These tests have a *-test.xml file as well. tests_with_testdata = [ 'ldml_test', 'k_001_tiny', @@ -44,6 +45,7 @@ tests_with_testdata = [ 'k_201_reorder_esk', 'k_210_marker', 'k_211_marker_escape', + 'k_212_marker_11057', ] tests = tests_without_testdata diff --git a/core/tests/unit/ldml/ldml_test_source.cpp b/core/tests/unit/ldml/ldml_test_source.cpp index b638379bb6d..71c5a7ee4e0 100644 --- a/core/tests/unit/ldml/ldml_test_source.cpp +++ b/core/tests/unit/ldml/ldml_test_source.cpp @@ -367,12 +367,19 @@ LdmlEmbeddedTestSource::vkey_to_event(std::string const &vk_event) { modifier_state |= modifier; } else { vk = get_vk(s); - break; + if (vk == 0) { + std::cerr << "Error parsing [" << vk_event << "] - could not find vkey or modifier: " << s << std::endl; + } + assert(vk != 0); + break; // only one vkey allowed } } // The string should be empty at this point - assert(!std::getline(f, s, ' ')); + if (std::getline(f, s, ' ')) { + std::cerr << "Error parsing vkey ["< tag === 'en'); + assert.deepEqual((unitTestEndpoints.langtagsByTag)['en'], en_langtag); + assert.deepEqual((unitTestEndpoints.langtagsByTag)['en-Latn-US'], en_langtag); + assert.deepEqual((unitTestEndpoints.langtagsByTag)['en-Latn'], en_langtag); + assert.deepEqual((unitTestEndpoints.langtagsByTag)['en-US'], en_langtag); + }); + + it('check init initialises KeyboardInfoCompiler correctly', async function() { + const sources = KHMER_ANGKOR_SOURCES; + const compiler = new KeyboardInfoCompiler(); + assert.isTrue(await compiler.init(callbacks, {sources})); + assert.deepEqual(compiler['callbacks'], callbacks); + assert.deepEqual(compiler['options'], {sources}); + }); + + it('check run returns null if KmpCompiler.init fails', async function() { + const kpjFilename = KHMER_ANGKOR_KPJ; + const sources = KHMER_ANGKOR_SOURCES; + const compiler = new KeyboardInfoCompiler(); + assert.isTrue(await compiler.init(callbacks, {sources})); + const origKmpCompilerInit = KmpCompiler.prototype.init; + let result: KeyboardInfoCompilerResult; + try { + KmpCompiler.prototype.init = async (_callbacks: CompilerCallbacks, _options: KmpCompilerOptions): Promise => false; + result = await compiler.run(kpjFilename, null); + } catch(e) { + assert.fail(e); + } finally { + KmpCompiler.prototype.init = origKmpCompilerInit; + } + assert.isNull(result); + }); + + it('check run returns null if KmpCompiler.transformKpsToKmpObject fails', async function() { + const kpjFilename = KHMER_ANGKOR_KPJ; + const sources = KHMER_ANGKOR_SOURCES; + const compiler = new KeyboardInfoCompiler(); + assert.isTrue(await compiler.init(callbacks, {sources})); + const origKmpCompilerTransformKpsToKmpObject = KmpCompiler.prototype.transformKpsToKmpObject; + let result: KeyboardInfoCompilerResult; + try { + KmpCompiler.prototype.transformKpsToKmpObject = (_kpsFilename: string): KmpJsonFile.KmpJsonFile => null; + result = await compiler.run(kpjFilename, null); + } catch(e) { + assert.fail(e); + } finally { + KmpCompiler.prototype.transformKpsToKmpObject = origKmpCompilerTransformKpsToKmpObject; + } + assert.isNull(result); + }); + + it('check run returns null if loadJsFile fails', async function() { + const kpjFilename = KHMER_ANGKOR_KPJ; + const sources = KHMER_ANGKOR_SOURCES + const compiler = new KeyboardInfoCompiler(); + assert.isTrue(await compiler.init(callbacks, {sources})); + compiler['loadJsFile'] = (_filename: string): string => null; + const result = await compiler.run(kpjFilename, null); + assert.isNull(result); + }); + + it('check run returns null if license is not MIT', async function() { + const kpjFilename = KHMER_ANGKOR_KPJ; + const sources = KHMER_ANGKOR_SOURCES; + const compiler = new KeyboardInfoCompiler(); + assert.isTrue(await compiler.init(callbacks, {sources})); + compiler['isLicenseMIT'] = (_filename: string): boolean => false; + const result = await compiler.run(kpjFilename, null); + assert.isNull(result); + }); + + it('check run returns null if fillLanguages fails', async function() { + const kpjFilename = KHMER_ANGKOR_KPJ; + const sources = KHMER_ANGKOR_SOURCES; + const compiler = new KeyboardInfoCompiler(); + assert.isTrue(await compiler.init(callbacks, {sources})); + compiler['fillLanguages'] = async (_kpsFilename: string, _keyboard_info: KeyboardInfoFile, _kmpJsonData: KmpJsonFile.KmpJsonFile): Promise => false; + const result = await compiler.run(kpjFilename, null); + assert.isNull(result); + }); + + it('should write artifacts to disk', async function() { + const kpjFilename = KHMER_ANGKOR_KPJ; + const actualFilename = makePathToFixture('khmer_angkor', 'build', 'actual.keyboard_info'); + const expectedFilename = makePathToFixture('khmer_angkor', 'build', 'khmer_angkor.keyboard_info'); + const sources = KHMER_ANGKOR_SOURCES; + + const compiler = new KeyboardInfoCompiler(); + assert.isTrue(await compiler.init(callbacks, {sources})); + const result = await compiler.run(kpjFilename, null); + assert.isNotNull(result); + + if(fs.existsSync(actualFilename)) { + fs.rmSync(actualFilename); + } + + result.artifacts.keyboard_info.filename = actualFilename; + assert.isTrue(await compiler.write(result.artifacts)); + assert(fs.existsSync(actualFilename)) + + const actual = JSON.parse(fs.readFileSync(actualFilename, 'utf-8')); + const expected = JSON.parse(fs.readFileSync(expectedFilename, 'utf-8')); + + // `lastModifiedDate` is dependent on time of run (not worth mocking) + delete actual['lastModifiedDate']; + delete expected['lastModifiedDate']; + + assert.deepEqual(actual, expected); + + if(fs.existsSync(actualFilename)) { // tidy up + fs.rmSync(actualFilename); + } + }); + + it('check mapKeymanTargetToPlatform returns correct platforms', async function() { + const compiler = new KeyboardInfoCompiler(); + const map: {[index in KeymanTargets.KeymanTarget]: KeyboardInfoFilePlatform[]} = { + any: [], + androidphone: ['android'], + androidtablet: ['android'], + desktop: [], + ipad: ['ios'], + iphone: ['ios'], + linux: ['linux'], + macosx: ['macos'], + mobile: [], + tablet: [], + web: ['desktopWeb'], + windows: ['windows'] + } + for (const [target, platform] of Object.entries(map)) { + assert.deepEqual(compiler['mapKeymanTargetToPlatform'](target), platform); + } + }); + + it('check kmxFileVersionToString returns correct strings', async function() { + const compiler = new KeyboardInfoCompiler(); + const convs = [ + {num: 0x0000, str: '0.0'}, + {num: 0x0001, str: '0.1'}, + {num: 0x0100, str: '1.0'}, + {num: 0x0101, str: '1.1'}, + {num: 0x0A0A, str: '10.10'}, + ]; + convs.forEach((conv) => { + assert.equal(compiler['kmxFileVersionToString'](conv.num), conv.str); + }); + }); + + it('check loadKmxFiles returns empty array if .kmx file is missing from .kmp', async function() { + const kpsFilename = KHMER_ANGKOR_KPS; + const compiler = new KeyboardInfoCompiler(); + const kmpCompiler = new KmpCompiler(); + assert.isTrue(await kmpCompiler.init(callbacks, {})); + const kmpJsonData = kmpCompiler.transformKpsToKmpObject(kpsFilename); + assert.isNotNull(kmpJsonData); + // remove .kmx file + kmpJsonData.files = kmpJsonData.files.filter(file => !KeymanFileTypes.filenameIs(file.name, KeymanFileTypes.Binary.Keyboard)); + const kmxFiles: { + filename: string, + data: KMX.KEYBOARD + }[] = compiler['loadKmxFiles'](kpsFilename, kmpJsonData); + assert.deepEqual(kmxFiles, []); + }); + + it('check loadKmxFiles throws error if .kmx file is missing from disk', async function() { + const kpsFilename = KHMER_ANGKOR_KPS; + const compiler = new KeyboardInfoCompiler(); + const kmpCompiler = new KmpCompiler(); + assert.isTrue(await kmpCompiler.init(callbacks, {})); + const kmpJsonData = kmpCompiler.transformKpsToKmpObject(kpsFilename); + assert.isNotNull(kmpJsonData); + // rename .kmx file in files list so it cannot be loaded from disk + const kmpIndex = kmpJsonData.files.findIndex(file => KeymanFileTypes.filenameIs(file.name, KeymanFileTypes.Binary.Keyboard)); + kmpJsonData.files[kmpIndex].name = '../build/throw_error.kmx'; + assert.throws(() => compiler['loadKmxFiles'](kpsFilename, kmpJsonData)); + }); + + it('check loadKmxFiles can handle two .kmx files', async function() { + const jsFilename = makePathToFixture('two-kmx', 'build', 'two_kmx.js'); + const kpsFilename = makePathToFixture('two-kmx', 'source', 'two_kmx.kps'); + const kmpFilename = makePathToFixture('two-kmx', 'build', 'two_kmx.kmp'); + + const sources = { + kmpFilename, + sourcePath: 'release/k/two-kmx', + kpsFilename, + jsFilename: jsFilename, + forPublishing: true, + }; + + const kmx_filename_001 = 'k_001___basic_input_unicodei.kmx'; + const kmx_filename_002 = 'k_002___basic_input_unicode.kmx'; + + const compiler = new KeyboardInfoCompiler(); + assert.isTrue(await compiler.init(callbacks, {sources})); + const kmpJsonData: KmpJsonFile.KmpJsonFile = { + system: { fileVersion: '', keymanDeveloperVersion: '' }, + options: null, + files: [ + { name: '../build/' + kmx_filename_001, description: 'Keyboard 001' }, + { name: '../build/' + kmx_filename_002, description: 'Keyboard 002' }, + ] + }; + const kmxFiles: { + filename: string, + data: KMX.KEYBOARD + }[] = compiler['loadKmxFiles'](kpsFilename, kmpJsonData); + assert.equal(kmxFiles.length, 2); + assert.deepEqual(kmxFiles[0].filename, kmx_filename_001); + assert.deepEqual(kmxFiles[1].filename, kmx_filename_002); + assert.isNotNull(kmxFiles[0].data); + assert.isNotNull(kmxFiles[1].data); + }); + + it('check loadJsFile throws error if .js file is invalid', async function() { + const jsFilename = makePathToFixture('invalid-js-file', 'build', 'invalid_js_file.js'); + const kpsFilename = makePathToFixture('invalid-js-file', 'source', 'invalid_js_file.kps'); + const kmpFilename = makePathToFixture('invalid-js-file', 'build', 'invalid_js_file.kmp'); + + const sources = { + kmpFilename, + sourcePath: 'release/k/invalid-js-file', + kpsFilename, + jsFilename: jsFilename, + forPublishing: true, + }; + + const compiler = new KeyboardInfoCompiler(); + assert.isTrue(await compiler.init(callbacks, {sources})); + const origTextDecoderDecode = TextDecoder.prototype.decode; + TextDecoder.prototype.decode = () => { throw new TypeError(); } + assert.throws(() => compiler['loadJsFile'](jsFilename)); + TextDecoder.prototype.decode = origTextDecoderDecode; + }); + + it('check fillLanguages constructs keyboard_info.languages correctly', async function() { + const kmpJsonData: KmpJsonFile.KmpJsonFile = { + system: { fileVersion: '', keymanDeveloperVersion: '' }, + options: null, + keyboards: [KHMER_ANGKOR_KEYBOARD], + }; + + const sources = KHMER_ANGKOR_SOURCES; + const compiler = new KeyboardInfoCompiler(); + assert.isTrue(await compiler.init(callbacks, {sources})); + compiler['fontSourceToKeyboardInfoFont'] = async (_kpsFilename: string, _kmpJsonData: KmpJsonFile.KmpJsonFile, _source: string[]) => { + if (_source[0] == KHMER_ANGKOR_DISPLAY_FONT) { + return KHMER_ANGKOR_DISPLAY_FONT_INFO; + } else { // osk font + return KHMER_ANGKOR_OSK_FONT_INFO; + } + } + const keyboard_info: KeyboardInfoFile = {}; + const result = await compiler['fillLanguages'](KHMER_ANGKOR_KPS, keyboard_info, kmpJsonData); + assert.isTrue(result); + assert.deepEqual(keyboard_info.languages, {km: { + examples: [ KHMER_ANGKOR_EXAMPLES_NO_ID ], + font: KHMER_ANGKOR_DISPLAY_FONT_INFO, + oskFont: KHMER_ANGKOR_OSK_FONT_INFO, + languageName: "Khmer", + regionName: undefined, + scriptName: undefined, + displayName: "Khmer", + }}); + }); + + it('check fillLanguages can handle two keyboards correctly', async function() { + const kmpJsonData: KmpJsonFile.KmpJsonFile = { + system: { fileVersion: '', keymanDeveloperVersion: '' }, + options: null, + keyboards: [KHMER_ANGKOR_KEYBOARD, SECOND_KEYBOARD], + }; + + const sources = KHMER_ANGKOR_SOURCES; + const compiler = new KeyboardInfoCompiler(); + assert.isTrue(await compiler.init(callbacks, {sources})); + let callCount = 0; + compiler['fontSourceToKeyboardInfoFont'] = async (_kpsFilename: string, _kmpJsonData: KmpJsonFile.KmpJsonFile, _source: string[]) => { + callCount++; + const info = [KHMER_ANGKOR_DISPLAY_FONT_INFO, KHMER_ANGKOR_OSK_FONT_INFO, SECOND_DISPLAY_FONT_INFO, SECOND_OSK_FONT_INFO]; + return info[callCount-1]; + }; + const keyboard_info: KeyboardInfoFile = {}; + const result = await compiler['fillLanguages'](KHMER_ANGKOR_KPS, keyboard_info, kmpJsonData); + assert.isTrue(result); + assert.deepEqual(keyboard_info.languages, {km: { + examples: [ KHMER_ANGKOR_EXAMPLES_NO_ID ], + font: KHMER_ANGKOR_DISPLAY_FONT_INFO, + oskFont: KHMER_ANGKOR_OSK_FONT_INFO, + languageName: "Khmer", + regionName: undefined, + scriptName: undefined, + displayName: "Khmer", + }, en: { + examples: [ SECOND_EXAMPLES_NO_ID ], + font: SECOND_DISPLAY_FONT_INFO, + oskFont: SECOND_OSK_FONT_INFO, + languageName: "English", + regionName: undefined, + scriptName: undefined, + displayName: "English", + }}); + }); + + it('check fillLanguages returns false if fontSourceToKeyboardInfoFont fails for display font', async function() { + const kmpJsonData: KmpJsonFile.KmpJsonFile = { + system: { fileVersion: '', keymanDeveloperVersion: '' }, + options: null, + keyboards: [KHMER_ANGKOR_KEYBOARD], + }; + + const sources = KHMER_ANGKOR_SOURCES; + const compiler = new KeyboardInfoCompiler(); + assert.isTrue(await compiler.init(callbacks, {sources})); + compiler['fontSourceToKeyboardInfoFont'] = async (_kpsFilename: string, _kmpJsonData: KmpJsonFile.KmpJsonFile, _source: string[]) => { + if (_source[0] == KHMER_ANGKOR_DISPLAY_FONT) { + return null; + } else { // osk font + return KHMER_ANGKOR_OSK_FONT_INFO; + } + } + const result = await compiler['fillLanguages'](KHMER_ANGKOR_KPS, {}, kmpJsonData); + assert.isFalse(result); + }); + + it('check fillLanguages returns false if fontSourceToKeyboardInfoFont fails for osk font', async function() { + const kmpJsonData: KmpJsonFile.KmpJsonFile = { + system: { fileVersion: '', keymanDeveloperVersion: '' }, + options: null, + keyboards: [KHMER_ANGKOR_KEYBOARD], + }; + + const sources = KHMER_ANGKOR_SOURCES; + const compiler = new KeyboardInfoCompiler(); + assert.isTrue(await compiler.init(callbacks, {sources})); + compiler['fontSourceToKeyboardInfoFont'] = async (_kpsFilename: string, _kmpJsonData: KmpJsonFile.KmpJsonFile, _source: string[]) => { + if (_source[0] == KHMER_ANGKOR_DISPLAY_FONT) { + return KHMER_ANGKOR_DISPLAY_FONT_INFO; + } else { // osk font + return null; + } + } + const result = await compiler['fillLanguages'](KHMER_ANGKOR_KPS, {}, kmpJsonData); + assert.isFalse(result); + }); }); diff --git a/developer/src/kmc-kmn/src/compiler/compiler.ts b/developer/src/kmc-kmn/src/compiler/compiler.ts index 109ec0cfa57..a7b9a73c280 100644 --- a/developer/src/kmc-kmn/src/compiler/compiler.ts +++ b/developer/src/kmc-kmn/src/compiler/compiler.ts @@ -551,13 +551,19 @@ export class KmnCompiler implements KeymanCompiler, UnicodeSetParser { return null; } - // TODO-LDML: Catch OOM + if ((rangeCount * 2) < 0) { + throw new RangeError(`Internal error: negative rangeCount * 2 = ${rangeCount * 2}`); + } const buf = this.wasmExports.malloc(rangeCount * 2 * Module.HEAPU32.BYTES_PER_ELEMENT); + if (buf <= 0) { + // out of memory will return zero. + throw new RangeError(`Internal error: wasm malloc() returned ${buf}`); + } // fix \u1234 pattern format pattern = KmnCompiler.fixNewPattern(pattern); - /** If <= 0: return code. If positive: range count */ const rc = Module.kmcmp_parseUnicodeSet(pattern, buf, rangeCount * 2); if (rc >= 0) { + // If >= 0: it's a range count (which could be zero, an empty set). const ranges = []; const startu = (buf / Module.HEAPU32.BYTES_PER_ELEMENT); for (let i = 0; i < rc; i++) { @@ -568,6 +574,7 @@ export class KmnCompiler implements KeymanCompiler, UnicodeSetParser { this.wasmExports.free(buf); return new UnicodeSet(pattern, ranges); } else { + // rc is negative: it's an error code. this.wasmExports.free(buf); // translate error code into callback this.callbacks.reportMessage(getUnicodeSetError(rc)); diff --git a/developer/src/kmc-kmn/src/compiler/kmn-compiler-messages.ts b/developer/src/kmc-kmn/src/compiler/kmn-compiler-messages.ts index 5a5208009af..32dac9fb661 100644 --- a/developer/src/kmc-kmn/src/compiler/kmn-compiler-messages.ts +++ b/developer/src/kmc-kmn/src/compiler/kmn-compiler-messages.ts @@ -559,6 +559,19 @@ export class KmnCompilerMessages { static ERROR_VirtualKeyInContext = SevError | 0x074; static Error_VirtualKeyInContext = () => m(this.ERROR_VirtualKeyInContext, `Virtual keys are not permitted in context`); + static ERROR_OutsTooLong = SevError | 0x075; + static Error_OutsTooLong = () => m(this.ERROR_OutsTooLong, `Store cannot be inserted with outs() as it makes the extended string too long`); + + static ERROR_ExtendedStringTooLong = SevError | 0x076; + static Error_ExtendedStringTooLong = () => m(this.ERROR_ExtendedStringTooLong, `Extended string is too long`); + + static ERROR_VirtualKeyExpansionTooLong = SevError | 0x077; + static Error_VirtualKeyExpansionTooLong = () => m(this.ERROR_VirtualKeyExpansionTooLong, `Virtual key expansion is too large`); + + static ERROR_CharacterRangeTooLong = SevError | 0x078; + static Error_CharacterRangeTooLong = () => m(this.ERROR_CharacterRangeTooLong, `Character range is too large and cannot be expanded`); + + static WARN_TooManyWarnings = SevWarn | 0x080; static Warn_TooManyWarnings = () => m(this.WARN_TooManyWarnings, `Too many warnings or errors`); diff --git a/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_character_range_too_long.kmn b/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_character_range_too_long.kmn new file mode 100644 index 00000000000..e19e3ab9f87 --- /dev/null +++ b/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_character_range_too_long.kmn @@ -0,0 +1,15 @@ +store(&NAME) 'error_character_range_too_long' +store(&VERSION) '9.0' + +begin unicode > use(main) + +group(main) using keys + +c maximum store length is 4096 UTF-16 code units, including U+0000 terminator +c #define GLOBAL_BUFSIZE 4096 // compfile.h +c so we need 0x101E - 0x0020 + 1 = 0x0FFF --> 4095 words +c See #11136 for calculation adjustment TODO + +store(x) U+0020 .. U+101E + +any(x) + 'x' > 'x' context diff --git a/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_extended_string_too_long.kmn b/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_extended_string_too_long.kmn new file mode 100644 index 00000000000..fcae6a5d758 --- /dev/null +++ b/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_extended_string_too_long.kmn @@ -0,0 +1,16 @@ +store(&NAME) 'error_extended_string_too_long' +store(&VERSION) '9.0' + +begin unicode > use(main) + +group(main) using keys + +c +c maximum store length is 4096 UTF-16 code units, including U+0000 terminator +c #define GLOBAL_BUFSIZE 4096 // compfile.h +c so we need 0x101B - 0x0020 + 1 = 0x0FFD --> 4092 words, + 4 = 4096 = too long +c See #11136 for calculation adjustment TODO + +store(x) U+0020 .. U+101B + +outs(x) 'abcd' + 'x' > 'x' context diff --git a/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_outs_too_long.kmn b/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_outs_too_long.kmn new file mode 100644 index 00000000000..750e45d3bad --- /dev/null +++ b/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_outs_too_long.kmn @@ -0,0 +1,18 @@ +store(&NAME) 'error_outs_too_long' +store(&VERSION) '9.0' + +begin unicode > use(main) + +group(main) using keys + +c maximum store length is 4096 UTF-16 code units, including U+0000 terminator +c #define GLOBAL_BUFSIZE 4096 // compfile.h +c so we need 0x101C - 0x0020 + 1 = 0x0FFD --> 4093 words +c + 1, for 'a' in the rule below = 4094, which triggers the buffer boundary check. +c Noting that this is conservative and losing 2 possible chars, but not fixing +c in compiler.cpp at this time. +c See #11136 for calculation adjustment TODO + +store(x) U+0020 .. U+101C + +'a' outs(x) + 'x' > 'x' context diff --git a/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_virtual_key_expansion_too_long.kmn b/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_virtual_key_expansion_too_long.kmn new file mode 100644 index 00000000000..a617f294349 --- /dev/null +++ b/developer/src/kmc-kmn/test/fixtures/invalid-keyboards/error_virtual_key_expansion_too_long.kmn @@ -0,0 +1,17 @@ +store(&NAME) 'error_virtual_key_expansion_too_long' +store(&VERSION) '9.0' + +begin unicode > use(main) + +group(main) using keys + +c maximum store length is 4096 UTF-16 code units, including U+0000 terminator +c #define GLOBAL_BUFSIZE 4096 // compfile.h +c so we need 0x101E - 0x0020 + 1 = 0x0FFF --> 4095 words +c each vk is 5 words long UC_SENTINEL CODE_EXTENDED shift key CODE_EXTENDEDEND (some long history here!) +c we start filling the buffer with 4066 words and then the remaining 30 bytes = 6 VKs A-F +c See #11136 for calculation adjustment TODO + +store(x) U+0020 .. U+1000 [K_A] .. [K_F] + +any(x) + 'x' > 'x' context diff --git a/developer/src/kmc-kmn/test/test-messages.ts b/developer/src/kmc-kmn/test/test-messages.ts index 674dd17e344..e338cbe521f 100644 --- a/developer/src/kmc-kmn/test/test-messages.ts +++ b/developer/src/kmc-kmn/test/test-messages.ts @@ -94,4 +94,32 @@ describe('KmnCompilerMessages', function () { assert.equal(callbacks.messages[0].message, "Virtual keys are not supported in output"); }); + // ERROR_OutsTooLong + + it('should generate ERROR_OutsTooLong if a store referenced in outs() is too long (more than GLOBAL_BUFSIZE elements)', async function() { + await testForMessage(this, ['invalid-keyboards', 'error_outs_too_long.kmn'], KmnCompilerMessages.ERROR_OutsTooLong); + // callbacks.printMessages(); + }); + + // ERROR_ExtendedStringTooLong + + it('should generate ERROR_ExtendedStringTooLong if an extended string is too long (more than GLOBAL_BUFSIZE elements)', async function() { + await testForMessage(this, ['invalid-keyboards', 'error_extended_string_too_long.kmn'], KmnCompilerMessages.ERROR_ExtendedStringTooLong); + // callbacks.printMessages(); + }); + + // ERROR_VirtualKeyExpansionTooLong + + it('should generate ERROR_VirtualKeyExpansionTooLong if a virtual key expansion is too long (more than GLOBAL_BUFSIZE elements)', async function() { + await testForMessage(this, ['invalid-keyboards', 'error_virtual_key_expansion_too_long.kmn'], KmnCompilerMessages.ERROR_VirtualKeyExpansionTooLong); + // callbacks.printMessages(); + }); + + // ERROR_CharacterRangeTooLong + + it('should generate ERROR_CharacterRangeTooLong if a character range would expand to be too long (more than GLOBAL_BUFSIZE elements)', async function() { + await testForMessage(this, ['invalid-keyboards', 'error_character_range_too_long.kmn'], KmnCompilerMessages.ERROR_CharacterRangeTooLong); + // callbacks.printMessages(); + }); + }); diff --git a/developer/src/kmc-ldml/src/compiler/keys.ts b/developer/src/kmc-ldml/src/compiler/keys.ts index 15cbc2cd98e..7d97df41265 100644 --- a/developer/src/kmc-ldml/src/compiler/keys.ts +++ b/developer/src/kmc-ldml/src/compiler/keys.ts @@ -487,7 +487,7 @@ export class KeysCompiler extends SectionCompiler { sect: Keys, hardware: string ): Keys { - const mod = translateLayerAttrToModifier(layer); + const mods = translateLayerAttrToModifier(layer); const keymap = this.getKeymapFromForm(hardware); // Iterate over rows (y) and cols (x) of the scancodes table. @@ -513,11 +513,14 @@ export class KeysCompiler extends SectionCompiler { if (x < keys.length) { key = keys[x]; } - sect.kmap.push({ - vkey, - mod, - key, // key id, to be changed into key index at finalization - }); + // push every combination + for (const mod of mods) { + sect.kmap.push({ + vkey, + mod, + key, // key id, to be changed into key index at finalization + }); + } } } return sect; diff --git a/developer/src/kmc-ldml/src/compiler/layr.ts b/developer/src/kmc-ldml/src/compiler/layr.ts index 6b1aa6d0285..8056144b25b 100644 --- a/developer/src/kmc-ldml/src/compiler/layr.ts +++ b/developer/src/kmc-ldml/src/compiler/layr.ts @@ -7,7 +7,6 @@ import { translateLayerAttrToModifier, validModifier } from '../util/util.js'; import DependencySections = KMXPlus.DependencySections; import Layr = KMXPlus.Layr; -import LayrEntry = KMXPlus.LayrEntry; import LayrList = KMXPlus.LayrList; import LayrRow = KMXPlus.LayrRow; @@ -41,7 +40,7 @@ export class LayrCompiler extends SectionCompiler { const { modifiers, id } = layer; totalLayerCount++; if (!validModifier(modifiers)) { - this.callbacks.reportMessage(CompilerMessages.Error_InvalidModifier({ modifiers, layer: id })); + this.callbacks.reportMessage(CompilerMessages.Error_InvalidModifier({ modifiers, layer: id || '' })); valid = false; } }); @@ -60,22 +59,28 @@ export class LayrCompiler extends SectionCompiler { sect.lists = this.keyboard3.layers.map((layers) => { const hardware = sections.strs.allocString(layers.formId); // Already validated in validate + const layerEntries = []; + for (const layer of layers.layer) { + const rows = layer.row.map((row) => { + const erow: LayrRow = { + keys: row.keys.split(' ').map((id) => sections.strs.allocString(id)), + }; + return erow; + }); + const mods = translateLayerAttrToModifier(layer); + // push a layer entry for each modifier set + for (const mod of mods) { + layerEntries.push({ + id: sections.strs.allocString(layer.id), + mod, + rows, + }); + } + } const list: LayrList = { hardware, minDeviceWidth: layers.minDeviceWidth || 0, - layers: layers.layer.map((layer) => { - const entry: LayrEntry = { - id: sections.strs.allocString(layer.id), - mod: translateLayerAttrToModifier(layer), - rows: layer.row.map((row) => { - const erow: LayrRow = { - keys: row.keys.split(' ').map((id) => sections.strs.allocString(id)), - }; - return erow; - }), - }; - return entry; - }), + layers: layerEntries, }; return list; }); diff --git a/developer/src/kmc-ldml/src/compiler/messages.ts b/developer/src/kmc-ldml/src/compiler/messages.ts index a4f19f170a6..2757960f429 100644 --- a/developer/src/kmc-ldml/src/compiler/messages.ts +++ b/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 */ @@ -171,13 +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.`); + +} diff --git a/developer/src/kmc-ldml/src/compiler/tran.ts b/developer/src/kmc-ldml/src/compiler/tran.ts index bf27fb2fea5..17b2bdc62f3 100644 --- a/developer/src/kmc-ldml/src/compiler/tran.ts +++ b/developer/src/kmc-ldml/src/compiler/tran.ts @@ -139,25 +139,28 @@ export abstract class TransformCompiler st.markers.add(SubstitutionUse.variable, MarkerParser.allReferences(value))); + // get markers mentioned in a set + keyboard?.variables?.set?.forEach(({ value }) => + VariableParser.setSplitter(value).forEach(v => st.markers.add(SubstitutionUse.match, MarkerParser.allReferences(v)))); return true; } @@ -208,8 +211,6 @@ export class VarsCompiler extends SectionCompiler { // first, strings. variables?.string?.forEach((e) => this.addString(result, e, sections)); - variables?.set?.forEach((e) => - this.addSet(result, e, sections)); variables?.uset?.forEach((e) => this.addUnicodeSet(result, e, sections)); @@ -222,6 +223,10 @@ export class VarsCompiler extends SectionCompiler { const allMarkers : string[] = Array.from(mt.all).filter(m => m !== MarkerParser.ANY_MARKER_ID).sort(); result.markers = sections.list.allocList(allMarkers, {}, sections); + // sets need to be added late, because they can refer to markers + variables?.set?.forEach((e) => + this.addSet(result, e, sections)); + return result.valid() ? result : null; } @@ -240,8 +245,13 @@ export class VarsCompiler extends SectionCompiler { value = result.substituteStrings(value, sections); // OK to do this as a substitute, because we've already validated the set above. value = result.substituteSets(value, sections); - const items : string[] = VariableParser.setSplitter(value); - result.sets.push(new SetVarItem(id, items, sections)); + // raw items - without marker substitution + const rawItems: string[] = VariableParser.setSplitter(value); + // cooked items - has substutition of markers + // this is not 'forMatch', all variables are to be assumed as string literals, not regex + // content. + const cookedItems: string[] = rawItems.map(v => result.substituteMarkerString(v, false)); + result.sets.push(new SetVarItem(id, cookedItems, sections, rawItems)); } addUnicodeSet(result: Vars, e: LDMLKeyboard.LKUSet, sections: DependencySections): void { const { id } = e; diff --git a/developer/src/kmc-ldml/src/util/util.ts b/developer/src/kmc-ldml/src/util/util.ts index a3f43b1b6ee..21409e72990 100644 --- a/developer/src/kmc-ldml/src/util/util.ts +++ b/developer/src/kmc-ldml/src/util/util.ts @@ -156,14 +156,19 @@ export function verifyValidAndUnique( /** * Determine modifier from layer info * @param layer layer obj - * @returns modifier + * @returns modifier array */ -export function translateLayerAttrToModifier(layer: LDMLKeyboard.LKLayer) : number { +export function translateLayerAttrToModifier(layer: LDMLKeyboard.LKLayer) : number[] { const { modifiers } = layer; + if (!modifiers) return [constants.keys_mod_none]; + return modifiers.split(',').map(m => translateModifierSubsetToLayer(m)).sort(); +} + +function translateModifierSubsetToLayer(modifiers: string) : number { + // TODO-LDML: Default #11072 if (modifiers) { - // TODO-LDML if (modifiers.indexOf(',') !== -1) { - throw Error(`TODO-LDML #9838: ”,” in modifiers not supported yet.`); + throw Error(`translateModifierSubsetToLayer only takes a single subset of the modifiers`); } let mod = constants.keys_mod_none; for (let str of modifiers.split(' ')) { @@ -183,8 +188,8 @@ export function translateLayerAttrToModifier(layer: LDMLKeyboard.LKLayer) : numb export function validModifier(modifier?: string) : boolean { if (!modifier) return true; // valid to have no modifier, == none // TODO-LDML: enforce illegal combinations per spec. - for (let sub of modifier.split(',')) { - for (let str of sub.split(' ')) { + for (let sub of modifier.trim().split(',')) { + for (let str of sub.trim().split(' ')) { if (!constants.keys_mod_map.has(str)) { return false; } diff --git a/developer/src/kmc-ldml/test/fixtures/sections/keys/many-modifiers.xml b/developer/src/kmc-ldml/test/fixtures/sections/keys/many-modifiers.xml new file mode 100644 index 00000000000..5c380cd7370 --- /dev/null +++ b/developer/src/kmc-ldml/test/fixtures/sections/keys/many-modifiers.xml @@ -0,0 +1,23 @@ + + + + + + + + + + + + + + + + + + diff --git a/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-IllegalTransformDollarsign-1.xml b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-IllegalTransformDollarsign-1.xml new file mode 100644 index 00000000000..6c866d4d7a2 --- /dev/null +++ b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-IllegalTransformDollarsign-1.xml @@ -0,0 +1,20 @@ + + + + + + + + + + + + + + + + + + + + diff --git a/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-IllegalTransformDollarsign-2.xml b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-IllegalTransformDollarsign-2.xml new file mode 100644 index 00000000000..0b962e1f8ce --- /dev/null +++ b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-IllegalTransformDollarsign-2.xml @@ -0,0 +1,12 @@ + + + + + + + + + + + + diff --git a/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-IllegalTransformDollarsign-3.xml b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-IllegalTransformDollarsign-3.xml new file mode 100644 index 00000000000..7aa8ced0bfe --- /dev/null +++ b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-IllegalTransformDollarsign-3.xml @@ -0,0 +1,12 @@ + + + + + + + + + + + + diff --git a/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-matches-nothing-1.xml b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-matches-nothing-1.xml new file mode 100644 index 00000000000..5b5e2bba5bd --- /dev/null +++ b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-matches-nothing-1.xml @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-matches-nothing-2.xml b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-matches-nothing-2.xml new file mode 100644 index 00000000000..8b932fc579e --- /dev/null +++ b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-matches-nothing-2.xml @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-matches-nothing-3.xml b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-matches-nothing-3.xml new file mode 100644 index 00000000000..98ff0dc828f --- /dev/null +++ b/developer/src/kmc-ldml/test/fixtures/sections/tran/fail-matches-nothing-3.xml @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/developer/src/kmc-ldml/test/fixtures/sections/tran/ok-1.xml b/developer/src/kmc-ldml/test/fixtures/sections/tran/ok-1.xml new file mode 100644 index 00000000000..fd9a456c2b0 --- /dev/null +++ b/developer/src/kmc-ldml/test/fixtures/sections/tran/ok-1.xml @@ -0,0 +1,12 @@ + + + + + + + + + + + + diff --git a/developer/src/kmc-ldml/test/test-keys.ts b/developer/src/kmc-ldml/test/test-keys.ts index a2906d0e58d..7d37dcfd362 100644 --- a/developer/src/kmc-ldml/test/test-keys.ts +++ b/developer/src/kmc-ldml/test/test-keys.ts @@ -403,6 +403,25 @@ describe('keys.kmap', function () { CompilerMessages.Error_MissingStringVariable({id: "varsok"}), ], }, + // modifiers test + { + // keep in sync with similar test in test-layr.ts + subpath: 'sections/keys/many-modifiers.xml', + callback(sect) { + const keys = sect; + assert.ok(keys); + const { kmap } = keys; + const aMods = kmap.filter(({ key }) => key === 'a').map(({ mod }) => mod); + assert.sameDeepMembers(aMods, [ + constants.keys_mod_none, + ], 'modifiers for a'); + const cMods = kmap.filter(({ key }) => key === 'c').map(({ mod }) => mod); + assert.sameDeepMembers(cMods, [ + constants.keys_mod_altR, + constants.keys_mod_ctrl | constants.keys_mod_shift, + ], 'modifiers for c'); + }, + }, ], keysDependencies); it('should reject layouts with too many hardware rows', async function() { diff --git a/developer/src/kmc-ldml/test/test-layr.ts b/developer/src/kmc-ldml/test/test-layr.ts index 04bf461aa40..32d212a0096 100644 --- a/developer/src/kmc-ldml/test/test-layr.ts +++ b/developer/src/kmc-ldml/test/test-layr.ts @@ -110,5 +110,24 @@ describe('layr', function () { subpath: 'sections/layr/invalid-missing-layer.xml', errors: [CompilerMessages.Error_MustBeAtLeastOneLayerElement()], }, + { + // keep in sync with similar test in test-keys.ts + subpath: 'sections/keys/many-modifiers.xml', + callback(sect) { + const layr = sect; + assert.ok(layr); + assert.equal(layr.lists.length, 1, 'layr.lists.length'); + const layers = layr.lists[0]; + const bymod = layers.layers.map(({id,mod,rows})=>([ + id.value, mod, rows[0].keys[0].value, + ])); + assert.sameDeepMembers(bymod, [ + // flatten the layers for comparison, assume a single key + ['base', constants.keys_mod_none, 'a'], + ['', constants.keys_mod_altR, 'c'], + ['', constants.keys_mod_ctrl | constants.keys_mod_shift, 'c'], + ]); + }, + }, ]); }); diff --git a/developer/src/kmc-ldml/test/test-tran.ts b/developer/src/kmc-ldml/test/test-tran.ts index 2ee82fd2d20..a6ba8e8025a 100644 --- a/developer/src/kmc-ldml/test/test-tran.ts +++ b/developer/src/kmc-ldml/test/test-tran.ts @@ -334,6 +334,31 @@ describe('tran', function () { CompilerMessages.Error_MissingStringVariable({ id: "missingstr" }), ], }, + // cases that share the same error code + ...[1, 2].map(n => ({ + subpath: `sections/tran/fail-IllegalTransformDollarsign-${n}.xml`, + errors: [ + { + 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`, + errors: [ + { + code: CompilerMessages.ERROR_TransformFromMatchesNothing, + matchMessage: /.*/, + } + ], + })), // escaping { subpath: `sections/tran/tran-escape.xml`, diff --git a/developer/src/kmc-ldml/test/test-utils.ts b/developer/src/kmc-ldml/test/test-utils.ts index 015af09e1d7..fcf1f786839 100644 --- a/developer/src/kmc-ldml/test/test-utils.ts +++ b/developer/src/kmc-ldml/test/test-utils.ts @@ -174,29 +174,33 @@ describe('test of util/util.ts', () => { }); describe('translateLayerAttrToModifier', () => { it('should map from layer info to modifier number', () => { - assert.equal(translateLayerAttrToModifier({ + assert.sameDeepMembers(translateLayerAttrToModifier({ id: 'base', - }), constants.keys_mod_none); - assert.equal(translateLayerAttrToModifier({ + }), [constants.keys_mod_none]); + assert.sameDeepMembers(translateLayerAttrToModifier({ id: 'base', modifiers: '', - }), constants.keys_mod_none); - assert.equal(translateLayerAttrToModifier({ + }), [constants.keys_mod_none]); + assert.sameDeepMembers(translateLayerAttrToModifier({ id: 'base', modifiers: 'none', - }), constants.keys_mod_none); - assert.equal(translateLayerAttrToModifier({ + }), [constants.keys_mod_none]); + assert.sameDeepMembers(translateLayerAttrToModifier({ id: 'shift', modifiers: 'shift', - }), constants.keys_mod_shift); - assert.equal(translateLayerAttrToModifier({ + }), [constants.keys_mod_shift]); + assert.sameDeepMembers(translateLayerAttrToModifier({ id: 'shift', modifiers: 'shift', - }), constants.keys_mod_shift); - assert.equal(translateLayerAttrToModifier({ + }), [constants.keys_mod_shift]); + assert.sameDeepMembers(translateLayerAttrToModifier({ + id: 'shiftOrCtrl', + modifiers: 'shift,ctrlL', + }), [constants.keys_mod_shift,constants.keys_mod_ctrlL]); + assert.sameDeepMembers(translateLayerAttrToModifier({ id: 'altshift', modifiers: 'alt shift', - }), constants.keys_mod_alt | constants.keys_mod_shift); + }), [constants.keys_mod_alt | constants.keys_mod_shift]); }); it('should round trip each possible modifier', () => { for(let str of constants.keys_mod_map.keys()) { @@ -204,8 +208,8 @@ describe('test of util/util.ts', () => { id: str, modifiers: `${str}`, }; - assert.equal(translateLayerAttrToModifier(layer), - constants.keys_mod_map.get(str), str); + assert.sameDeepMembers(translateLayerAttrToModifier(layer), + [constants.keys_mod_map.get(str)], str); } }); it('should round trip each possible modifier with altL', () => { @@ -214,8 +218,8 @@ describe('test of util/util.ts', () => { id: str, modifiers: `${str} altL`, }; - assert.equal(translateLayerAttrToModifier(layer), - constants.keys_mod_map.get(str) | constants.keys_mod_altL, str); + assert.sameDeepMembers(translateLayerAttrToModifier(layer), + [constants.keys_mod_map.get(str) | constants.keys_mod_altL], str); } }); }); diff --git a/developer/src/kmc-model/build.sh b/developer/src/kmc-model/build.sh index a4700d362ef..8fefa3584fa 100755 --- a/developer/src/kmc-model/build.sh +++ b/developer/src/kmc-model/build.sh @@ -15,6 +15,7 @@ cd "$THIS_SCRIPT_PATH" builder_describe "Build Keyman kmc Lexical Model Compiler module" \ "@/common/web/keyman-version" \ "@/developer/src/common/web/test-helpers" \ + "@/common/models/templates test" \ "clean" \ "configure" \ "build" \ diff --git a/developer/src/kmc-model/package.json b/developer/src/kmc-model/package.json index 7d9da682daf..8fbbfc841ea 100644 --- a/developer/src/kmc-model/package.json +++ b/developer/src/kmc-model/package.json @@ -37,6 +37,7 @@ }, "devDependencies": { "@keymanapp/developer-test-helpers": "*", + "@keymanapp/models-templates": "*", "@types/chai": "^4.1.7", "@types/mocha": "^5.2.7", "@types/node": "^20.4.1", diff --git a/developer/src/kmc-model/test/test-compile-trie.ts b/developer/src/kmc-model/test/test-compile-trie.ts index 5c1353102d7..a9829568370 100644 --- a/developer/src/kmc-model/test/test-compile-trie.ts +++ b/developer/src/kmc-model/test/test-compile-trie.ts @@ -1,11 +1,13 @@ import { LexicalModelCompiler } from '../src/lexical-model-compiler.js'; import {assert} from 'chai'; +import fs from 'fs'; import 'mocha'; import {makePathToFixture, compileModelSourceCode} from './helpers/index.js'; import { createTrieDataStructure } from '../src/build-trie.js'; import { ModelCompilerError } from '../src/model-compiler-messages.js'; import { TestCompilerCallbacks } from '@keymanapp/developer-test-helpers'; +import { TrieModel } from '@keymanapp/models-templates'; describe('LexicalModelCompiler', function () { const callbacks = new TestCompilerCallbacks(); @@ -165,8 +167,48 @@ describe('createTrieDataStructure()', function () { it('does not create `null`/"undefined"-keyed children', function () { const WORDLIST_FILENAME = makePathToFixture('example.qaa.wordlist-ordering', 'wordlist.tsv'); // check to ensure the Trie is fully well-formed. + // + // Is pretty much a JSON-encoded Trie spec, stringifying { root: Node, totalWeight: number } + // as used to initialize the Trie. let sourceCode = createTrieDataStructure([WORDLIST_FILENAME], (wf) => wf); - assert.notMatch(sourceCode, /"undefined"/); + + // Simple first-pass check: the signs of #11073 are not present. + assert.notMatch(sourceCode, /undefined/); assert.notMatch(sourceCode, /null/); - }) + + // A more complex check: load and use the resulting TrieModel to check that all + // words are accessible via `.predict`. + + // First, load the model. + const trieSpec = JSON.parse(sourceCode); + const model = new TrieModel(trieSpec); + assert.isOk(model); + + // Gets the list of all words in the wordlist fixture. + const rawWordlist = fs.readFileSync(WORDLIST_FILENAME).toString(); + const words = rawWordlist.split('\n').map((line) => { + const columns = line.split('\t') + if(!columns.length || !columns[0]) { + return undefined; + } else { + return columns[0]; + } + }).filter((entry) => entry !== undefined); + + // We'll track all _observed_ words from the model here. + const set = new Set(); + const contextFromWord = (word: string) => { + return {left: word, startOfBuffer: true, endOfBuffer: true}; + }; + + // Using each word as a prediction prefix, attempt to get a suggestion corresponding to each. + for(let word of words) { + const rawSuggestions = model.predict({insert: '', deleteLeft: 0}, contextFromWord(word)); + const suggestions = rawSuggestions.map((entry) => entry.sample.displayAs); + suggestions.forEach((suggestion) => set.add(suggestion)); + } + + // The actual assertion: did we see each word as a suggestion? + assert.sameMembers([...set], words, "Could not suggest all words in the wordlist"); + }); }); diff --git a/developer/src/kmc/build.sh b/developer/src/kmc/build.sh index a8986d1213b..03884c37566 100755 --- a/developer/src/kmc/build.sh +++ b/developer/src/kmc/build.sh @@ -126,7 +126,7 @@ if builder_start_action bundle; then mkdir -p build/dist node build-bundler.js - ./node_modules/.bin/sentry-cli sourcemaps inject \ + sentry-cli sourcemaps inject \ --org keyman \ --project keyman-developer \ --release "$VERSION_GIT_TAG" \ diff --git a/developer/src/kmc/package.json b/developer/src/kmc/package.json index 23def4adb07..ff425002091 100644 --- a/developer/src/kmc/package.json +++ b/developer/src/kmc/package.json @@ -55,7 +55,7 @@ "build/unicode-license.txt" ], "devDependencies": { - "@sentry/cli": "^2.19.4", + "@sentry/cli": "^2.31.0", "@types/chai": "^4.1.7", "@types/mocha": "^5.2.7", "@types/node": "^20.4.1", diff --git a/developer/src/kmcmplib/src/CompMsg.cpp b/developer/src/kmcmplib/src/CompMsg.cpp index 0c1adfa6e6d..86a9b4d4757 100644 --- a/developer/src/kmcmplib/src/CompMsg.cpp +++ b/developer/src/kmcmplib/src/CompMsg.cpp @@ -111,6 +111,10 @@ const struct CompilerError CompilerErrors[] = { { CERR_DuplicateStore , "A store with this name has already been defined."}, { CERR_RepeatedBegin , "Begin has already been set"}, { CERR_VirtualKeyInContext , "Virtual keys are not permitted in context"}, + { CERR_OutsTooLong , "Store cannot be inserted with outs() as it makes the extended string too long" }, + { CERR_ExtendedStringTooLong , "Extended string is too long" }, + { CERR_VirtualKeyExpansionTooLong , "Virtual key expansion is too large" }, + { CERR_CharacterRangeTooLong , "Character range is too large and cannot be expanded" }, { CHINT_UnreachableRule , "This rule will never be matched as another rule takes precedence"}, { CHINT_NonUnicodeFile , "Keyman Developer has detected that the file has ANSI encoding. Consider converting this file to UTF-8"}, diff --git a/developer/src/kmcmplib/src/Compiler.cpp b/developer/src/kmcmplib/src/Compiler.cpp index 045b1c39ad1..75409f51413 100644 --- a/developer/src/kmcmplib/src/Compiler.cpp +++ b/developer/src/kmcmplib/src/Compiler.cpp @@ -1827,6 +1827,12 @@ KMX_DWORD GetXStringImpl(PKMX_WCHAR tstr, PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX p = str; do { + if (mx >= max) { + // This is an error condition, we want the compiler + // to crash if we reach this + return CERR_BufferOverflow; + } + tokenFound = FALSE; while (iswspace(*p) && !u16chr(token, *p)) p++; if (!*p) break; @@ -1905,7 +1911,7 @@ KMX_DWORD GetXStringImpl(PKMX_WCHAR tstr, PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX case 1: q = (PKMX_WCHAR) u16chr(p + 1, '\"'); if (!q) return CERR_UnterminatedString; - if ((int)(q - p) - 1 + mx > max) return CERR_UnterminatedString; + if ((int)(q - p) - 1 + mx > max) return CERR_ExtendedStringTooLong; if (sFlag) return CERR_StringInVirtualKeySection; u16ncat(tstr, p + 1, (int)(q - p) - 1); // I3481 mx += (int)(q - p) - 1; @@ -1915,7 +1921,7 @@ KMX_DWORD GetXStringImpl(PKMX_WCHAR tstr, PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX case 2: q = (PKMX_WCHAR) u16chr(p + 1, '\''); if (!q) return CERR_UnterminatedString; - if ((int)(q - p) - 1 + mx > max) return CERR_UnterminatedString; + if ((int)(q - p) - 1 + mx > max) return CERR_ExtendedStringTooLong; if (sFlag) return CERR_StringInVirtualKeySection; u16ncat(tstr, p + 1, (int)(q - p) - 1); // I3481 mx += (int)(q - p) - 1; @@ -2031,7 +2037,9 @@ KMX_DWORD GetXStringImpl(PKMX_WCHAR tstr, PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX for (q = fk->dpStoreArray[i].dpString; *q; q++) { tstr[mx++] = *q; - if (mx >= max - 1) return CERR_BufferOverflow; + if (mx >= max - 1) { + return CERR_OutsTooLong; + } } tstr[mx] = 0; continue; @@ -2421,7 +2429,6 @@ KMX_DWORD GetXStringImpl(PKMX_WCHAR tstr, PFILE_KEYBOARD fk, PKMX_WCHAR str, KMX ErrChr = 0; return CERR_None; } - if (mx >= max) return CERR_BufferOverflow; } while (*p); if (!*token) @@ -2630,7 +2637,9 @@ KMX_DWORD process_expansion(PFILE_KEYBOARD fk, PKMX_WCHAR q, PKMX_WCHAR tstr, in return CERR_ExpansionMustBePositive; } // Verify space in buffer - if (*mx + (HighKey - BaseKey) * 5 + 1 >= max) return CERR_BufferOverflow; + if (*mx + (HighKey - BaseKey) * 5 + 1 >= max) { + return CERR_VirtualKeyExpansionTooLong; + } // Inject an expansion. for (BaseKey++; BaseKey < HighKey; BaseKey++) { // < HighKey because caller will add HighKey to output @@ -2657,12 +2666,16 @@ KMX_DWORD process_expansion(PFILE_KEYBOARD fk, PKMX_WCHAR q, PKMX_WCHAR tstr, in // < HighChar because caller will add HighChar to output if (Uni_IsSMP(BaseChar)) { // We'll test on each char to avoid complex calculations crossing SMP boundary - if (*mx + 3 >= max) return CERR_BufferOverflow; + if (*mx + 3 >= max) { + return CERR_CharacterRangeTooLong; + } tstr[(*mx)++] = (KMX_WCHAR) Uni_UTF32ToSurrogate1(BaseChar); tstr[(*mx)++] = (KMX_WCHAR) Uni_UTF32ToSurrogate2(BaseChar); } else { - if (*mx + 2 >= max) return CERR_BufferOverflow; + if (*mx + 2 >= max) { + return CERR_CharacterRangeTooLong; + } tstr[(*mx)++] = (KMX_WCHAR) BaseChar; } } diff --git a/developer/src/kmconvert/kmconvert.dpr b/developer/src/kmconvert/kmconvert.dpr index d8fffd53c58..ee650baf2fa 100644 --- a/developer/src/kmconvert/kmconvert.dpr +++ b/developer/src/kmconvert/kmconvert.dpr @@ -88,7 +88,8 @@ uses Keyman.System.LexicalModelUtils in '..\common\delphi\lexicalmodels\Keyman.System.LexicalModelUtils.pas', KeymanDeveloperOptions in '..\tike\main\KeymanDeveloperOptions.pas', Keyman.Developer.System.KeymanDeveloperPaths in '..\tike\main\Keyman.Developer.System.KeymanDeveloperPaths.pas', - Keyman.Developer.System.LdmlKeyboardProjectTemplate in 'Keyman.Developer.System.LdmlKeyboardProjectTemplate.pas'; + Keyman.Developer.System.LdmlKeyboardProjectTemplate in 'Keyman.Developer.System.LdmlKeyboardProjectTemplate.pas', + utilhttp in '..\..\..\common\windows\delphi\general\utilhttp.pas'; {$R icons.RES} {$R version.res} diff --git a/developer/src/kmconvert/kmconvert.dproj b/developer/src/kmconvert/kmconvert.dproj index 5314ceda6b4..0fed3d1be3f 100644 --- a/developer/src/kmconvert/kmconvert.dproj +++ b/developer/src/kmconvert/kmconvert.dproj @@ -195,6 +195,7 @@ + Cfg_2 Base diff --git a/developer/src/setup/setup.dpr b/developer/src/setup/setup.dpr index 16231f553ad..867a42fd1e3 100644 --- a/developer/src/setup/setup.dpr +++ b/developer/src/setup/setup.dpr @@ -25,7 +25,8 @@ uses utilexecute in '..\..\..\common\windows\delphi\general\utilexecute.pas', KeymanVersion in '..\..\..\common\windows\delphi\general\KeymanVersion.pas', SFX in '..\..\..\common\windows\delphi\setup\SFX.pas', - Keyman.System.UpdateCheckResponse in '..\..\..\common\windows\delphi\general\Keyman.System.UpdateCheckResponse.pas'; + Keyman.System.UpdateCheckResponse in '..\..\..\common\windows\delphi\general\Keyman.System.UpdateCheckResponse.pas', + utilhttp in '..\..\..\common\windows\delphi\general\utilhttp.pas'; {$R icons.res} {$R version.res} diff --git a/developer/src/setup/setup.dproj b/developer/src/setup/setup.dproj index 3c0ac53ec08..c7bf52ca25b 100644 --- a/developer/src/setup/setup.dproj +++ b/developer/src/setup/setup.dproj @@ -124,6 +124,7 @@ + Cfg_2 Base diff --git a/developer/src/tike/xml/layoutbuilder/builder.js b/developer/src/tike/xml/layoutbuilder/builder.js index 652baacdf8b..3a8caef1f98 100644 --- a/developer/src/tike/xml/layoutbuilder/builder.js +++ b/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]); } }); diff --git a/developer/src/tools/sentry-upload-difs.sh b/developer/src/tools/sentry-upload-difs.sh index a7ae63933d5..e2ba41123ee 100755 --- a/developer/src/tools/sentry-upload-difs.sh +++ b/developer/src/tools/sentry-upload-difs.sh @@ -65,7 +65,7 @@ sourcemap_paths=( ) echo "Uploading symbols for developer/" -./src/kmc/node_modules/.bin/sentry-cli upload-dif \ +sentry-cli upload-dif \ --project keyman-developer \ --include-sources \ --no-zips \ @@ -73,7 +73,7 @@ echo "Uploading symbols for developer/" upload_sourcemap() { local smpath="$1" - "$KEYMAN_ROOT/developer/src/kmc/node_modules/.bin/sentry-cli" sourcemaps upload \ + sentry-cli sourcemaps upload \ --no-dedupe \ --org keyman \ --project keyman-developer \ diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Constants.swift b/ios/engine/KMEI/KeymanEngine/Classes/Constants.swift index c2831845957..3b4ed5a1d62 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Constants.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Constants.swift @@ -61,6 +61,7 @@ public enum Key { // Settings-related keys static let optShouldReportErrors = "ShouldReportErrors" + // Deprecated - no longer used static let optShouldShowBanner = "ShouldShowBanner" static let optSpacebarText = "SpacebarText" // This one SHOULD be app-only, but is needed by the currently diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Extension/String+Helpers.swift b/ios/engine/KMEI/KeymanEngine/Classes/Extension/String+Helpers.swift index c14b7a508c0..d09cd35561e 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Extension/String+Helpers.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Extension/String+Helpers.swift @@ -53,10 +53,9 @@ func trimDirectionalMarkPrefix(_ str: String?) -> String { // If the first intended char in context is a diacritic (such as U+0300), Swift's // standard string-handling will treat it and a preceding directional character // as a single unit. This code block exists to avoid the issue. - if text.utf16.count > 0 { - let head = UnicodeScalar(text.utf16[text.utf16.startIndex])! - let tail = text.utf16.count > 1 ? String(text.utf16[text.utf16.index(text.utf16.startIndex, offsetBy: 1).. 0 { + let head = text.unicodeScalars.first! + let tail = String(text.unicodeScalars.dropFirst(1)) // // "%02X" - to hex-format the integer for the string conversion. // let headEncoding = String(format:"%02X", text.utf16[text.utf16.startIndex]) diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift index 20f912c96eb..172385c6fcf 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/InputViewController.swift @@ -73,7 +73,6 @@ private class CustomInputView: UIInputView, UIInputViewAudioFeedback { func setConstraints() { let innerView = keymanWeb.view! - let guide = self.safeAreaLayoutGuide // Fallback on earlier versions @@ -89,11 +88,13 @@ private class CustomInputView: UIInputView, UIInputViewAudioFeedback { kbdWidthConstraint.priority = .defaultHigh kbdWidthConstraint.isActive = true + let bannerHeight = InputViewController.topBarHeight + // Cannot be met by the in-app keyboard, but helps to 'force' height for the system keyboard. - let portraitHeight = innerView.heightAnchor.constraint(equalToConstant: keymanWeb.constraintTargetHeight(isPortrait: true)) + let portraitHeight = innerView.heightAnchor.constraint(equalToConstant: bannerHeight + keymanWeb.constraintTargetHeight(isPortrait: true)) portraitHeight.identifier = "Height constraint for portrait mode" portraitHeight.priority = .defaultHigh - let landscapeHeight = innerView.heightAnchor.constraint(equalToConstant: keymanWeb.constraintTargetHeight(isPortrait: false)) + let landscapeHeight = innerView.heightAnchor.constraint(equalToConstant: bannerHeight + keymanWeb.constraintTargetHeight(isPortrait: false)) landscapeHeight.identifier = "Height constraint for landscape mode" landscapeHeight.priority = .defaultHigh @@ -105,22 +106,6 @@ private class CustomInputView: UIInputView, UIInputViewAudioFeedback { override func updateConstraints() { super.updateConstraints() - // Keep the constraints up-to-date! They should vary based upon the selected keyboard. - let userData = Storage.active.userDefaults - let alwaysShow = userData.bool(forKey: Key.optShouldShowBanner) - - var hideBanner = true - if alwaysShow || Manager.shared.isSystemKeyboard || keymanWeb.activeModel { - hideBanner = false - } - let topBarDelta = hideBanner ? 0 : InputViewController.topBarHeight - - // Sets height before the constraints, as it's the height constraint that triggers OSK resizing. - keymanWeb.setBannerHeight(to: Int(InputViewController.topBarHeight)) - - portraitConstraint?.constant = topBarDelta + keymanWeb.constraintTargetHeight(isPortrait: true) - landscapeConstraint?.constant = topBarDelta + keymanWeb.constraintTargetHeight(isPortrait: false) - // Activate / deactivate layout-specific constraints. if InputViewController.isPortrait { landscapeConstraint?.isActive = false @@ -168,7 +153,7 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { } var expandedHeight: CGFloat { - return keymanWeb.keyboardHeight + activeTopBarHeight + return keymanWeb.keyboardHeight + InputViewController.topBarHeight } public convenience init() { @@ -442,10 +427,6 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { case .doNothing: break } - - // If we allow the system keyboard to show no banners, this line is needed - // for variable system keyboard height. - updateShowBannerSetting() } else { // Use in-app keyboard behavior instead. if !(Manager.shared.currentResponder?.showKeyboardPicker() ?? false) { _ = Manager.shared.switchToNextKeyboard @@ -453,11 +434,6 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { } } - // Needed due to protection level on the `keymanWeb` property - func updateShowBannerSetting() { - keymanWeb.updateShowBannerSetting() - } - func updateSpacebarText() { keymanWeb.updateSpacebarText() } @@ -483,22 +459,6 @@ open class InputViewController: UIInputViewController, KeymanWebDelegate { baseWidthConstraint.isActive = true } - public var isTopBarActive: Bool { - let userData = Storage.active.userDefaults - let alwaysShow = userData.bool(forKey: Key.optShouldShowBanner) - - if alwaysShow || Manager.shared.isSystemKeyboard || keymanWeb.activeModel { - return true - } - - return false - } - - public var activeTopBarHeight: CGFloat { - // If 'isSystemKeyboard' is true, always show the top bar. - return isTopBarActive ? CGFloat(InputViewController.topBarHeight) : 0 - } - public var kmwHeight: CGFloat { return keymanWeb.keyboardHeight } diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeyboardMenuView.swift b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeyboardMenuView.swift index 9ae5434174a..397697222cc 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeyboardMenuView.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeyboardMenuView.swift @@ -339,7 +339,6 @@ class KeyboardMenuView: UIView, UITableViewDelegate, UITableViewDataSource, UIGe // keyboard loaded does not provide suggestions. Parking it here as it's not // worth pursuing further at this stage. inputViewController?.updateViewConstraints() - inputViewController?.updateShowBannerSetting() return } } diff --git a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift index 9ed3643d819..1bf54832f08 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/Keyboard/KeymanWebViewController.swift @@ -131,6 +131,14 @@ class KeymanWebViewController: UIViewController { webView!.navigationDelegate = self webView!.scrollView.isScrollEnabled = false + // Disable WKWebView default layout-constraint manipulations. We ensure + // safe-area boundaries are respected via InputView / InputViewController + // constraints. + // + // Fixes #10859. + // Ref: https://stackoverflow.com/a/63741514 + webView!.scrollView.contentInsetAdjustmentBehavior = .never + view = webView // Set UILongPressGestureRecognizer to show sub keys @@ -415,15 +423,6 @@ extension KeymanWebViewController { } else { // We're registering a model in the background - don't change settings. webView!.evaluateJavaScript("keyman.addModel(\(stubString));", completionHandler: nil) } - - setBannerHeight(to: Int(InputViewController.topBarHeight)) - } - - func showBanner(_ display: Bool) { - let message = "Changing banner's alwaysShow property to \(display)" - os_log("%{public}s", log: KeymanEngineLogger.settings, type: .debug, message) - SentryManager.breadcrumb(message, category: "engine", sentryLevel: .debug) - webView?.evaluateJavaScript("showBanner(\(display ? "true" : "false"))", completionHandler: nil) } func setBannerImage(to path: String) { @@ -731,11 +730,12 @@ extension KeymanWebViewController: KeymanWebDelegate { } updateSpacebarText() - updateShowBannerSetting() setBannerImage(to: bannerImgPath) // Reset the keyboard's size. keyboardSize = kbSize + setBannerHeight(to: Int(InputViewController.topBarHeight)) + fixLayout() // Will trigger Manager's `keyboardLoaded` method. @@ -749,16 +749,6 @@ extension KeymanWebViewController: KeymanWebDelegate { } } - func updateShowBannerSetting() { - let userData = Storage.active.userDefaults - let alwaysShow = userData.bool(forKey: Key.optShouldShowBanner) - if !Manager.shared.isSystemKeyboard { - showBanner(false) - } else { - showBanner(alwaysShow) - } - } - func insertText(_ view: KeymanWebViewController, numCharsToDelete: Int, newText: String) { dismissHelpBubble() Manager.shared.isKeymanHelpOn = false @@ -1120,8 +1110,6 @@ extension KeymanWebViewController { isLoading = true updateSpacebarText() - // Check for a change of "always show banner" state - updateShowBannerSetting() } /* diff --git a/ios/engine/KMEI/KeymanEngine/Classes/SettingsViewController.swift b/ios/engine/KMEI/KeymanEngine/Classes/SettingsViewController.swift index ad65e491b4a..543453bb0c1 100644 --- a/ios/engine/KMEI/KeymanEngine/Classes/SettingsViewController.swift +++ b/ios/engine/KMEI/KeymanEngine/Classes/SettingsViewController.swift @@ -61,12 +61,6 @@ open class SettingsViewController: UITableViewController { "reuseid" : "languages" ]) - itemsArray.append([ - "title": NSLocalizedString("menu-settings-show-banner", bundle: engineBundle, comment: ""), - "subtitle": "", - "reuseid" : "showbanner" - ]) - itemsArray.append([ "title": NSLocalizedString("menu-settings-startup-get-started", bundle: engineBundle, comment: ""), "subtitle": "", @@ -151,21 +145,6 @@ open class SettingsViewController: UITableViewController { switch(cellIdentifier) { case "languages": break - case "showbanner": - let showBannerSwitch = UISwitch() - showBannerSwitch.translatesAutoresizingMaskIntoConstraints = false - - let switchFrame = frameAtRightOfCell(cell: cell.frame, controlSize: showBannerSwitch.frame.size) - showBannerSwitch.frame = switchFrame - - showBannerSwitch.isOn = showBanner - showBannerSwitch.addTarget(self, action: #selector(self.bannerSwitchValueChanged), - for: .valueChanged) - cell.addSubview(showBannerSwitch) - cell.contentView.isUserInteractionEnabled = false - - showBannerSwitch.rightAnchor.constraint(equalTo: cell.layoutMarginsGuide.rightAnchor).isActive = true - showBannerSwitch.centerYAnchor.constraint(equalTo: cell.layoutMarginsGuide.centerYAnchor).isActive = true case "showgetstarted": let showAgainSwitch = UISwitch() showAgainSwitch.translatesAutoresizingMaskIntoConstraints = false @@ -216,19 +195,6 @@ open class SettingsViewController: UITableViewController { } } - @objc func bannerSwitchValueChanged(_ sender: Any) { - let userData = Storage.active.userDefaults - if let toggle = sender as? UISwitch { - // actually this should call into KMW, which controls the banner - userData.set(toggle.isOn, forKey: Key.optShouldShowBanner) - userData.synchronize() - } - - // Necessary for the keyboard to visually update to match - // the new setting. - Manager.shared.inputViewController.setShouldReload() - } - @objc func showGetStartedSwitchValueChanged(_ sender: Any) { let userData = Storage.active.userDefaults if let toggle = sender as? UISwitch { @@ -236,11 +202,6 @@ open class SettingsViewController: UITableViewController { userData.synchronize() } } - - private var showBanner: Bool { - let userData = Storage.active.userDefaults - return userData.bool(forKey: Key.optShouldShowBanner) - } private var showGetStarted: Bool { let userData = Storage.active.userDefaults diff --git a/ios/engine/KMEI/KeymanEngine/resources/Keyman.bundle/Contents/Resources/ios-host.js b/ios/engine/KMEI/KeymanEngine/resources/Keyman.bundle/Contents/Resources/ios-host.js index d5eb798b67d..bd323b970b4 100644 --- a/ios/engine/KMEI/KeymanEngine/resources/Keyman.bundle/Contents/Resources/ios-host.js +++ b/ios/engine/KMEI/KeymanEngine/resources/Keyman.bundle/Contents/Resources/ios-host.js @@ -60,6 +60,11 @@ function init() { kmw.osk.bannerView.activeBannerHeight = bannerHeight; keyman.refreshOskLayout(); } + + var bc = keyman.osk.bannerController; + if(bannerImgPath) { + bc.inactiveBanner = new bc.ImageBanner(bannerImgPath); + } }); } @@ -72,13 +77,6 @@ function verifyLoaded() { } } -function showBanner(flag) { - console.log("Setting banner display for dictionaryless keyboards to " + flag); - - var bc = keyman.osk.bannerController; - bc.inactiveBanner = flag ? new bc.ImageBanner(bannerImgPath) : null; -} - function setBannerImage(path) { bannerImgPath = path; @@ -88,7 +86,7 @@ function setBannerImage(path) { } // If an inactive banner is set, update its image. - bc.inactiveBanner = bc.inactiveBanner ? new bc.ImageBanner(bannerImgPath) : null; + bc.inactiveBanner = new bc.ImageBanner(bannerImgPath); } function setBannerHeight(h) { diff --git a/ios/help/basic/config/index.md b/ios/help/basic/config/index.md index 22468a4021c..64ea47eac59 100644 --- a/ios/help/basic/config/index.md +++ b/ios/help/basic/config/index.md @@ -26,17 +26,6 @@ screen where you may view the languages you have installed and, by clicking any Click on this to [search for a keyboard or language](../../start/searching-for-keyboards). -## Show Banner -This adds a banner to the top of the keyboard to allow space for pop-up keys to render above the top row of keys. - -![](../../ios_images/banner.png) - -When this is off, - -![](../../ios_images/no-banner.png) - -pop-up keys in the top row render horizontally. - ## Show "Get Started" on startup When enabled, the Keyman app will display the 'Get Started' screen on app startup. diff --git a/ios/help/basic/index.md b/ios/help/basic/index.md index e2c9e82d6d2..b67688e03ce 100644 --- a/ios/help/basic/index.md +++ b/ios/help/basic/index.md @@ -1,6 +1,7 @@ --- title: Basic Help --- +* [Using the Banner on the Keyboard](using-the-banner) * [Switching Between Keyboards](switching-between-keyboards) diff --git a/ios/help/basic/keyboard-usage.md b/ios/help/basic/keyboard-usage.md index e56b56ae4b5..f7834a4e7a1 100644 --- a/ios/help/basic/keyboard-usage.md +++ b/ios/help/basic/keyboard-usage.md @@ -22,5 +22,7 @@ The Gestures: * **Flick:** hold and slide in various directions on a key to reach alternate outputs. The key will animate to show you the expected output. For example, a keyboard could support sliding down on keys in the top row to output numerals. +![](../ios_images/down-flick-i.gif) + * **Multitap:** some keys produce alternate outputs with rapid, repeated taps. For example, tapping rapidly twice on shift will activate caps lock on many keyboards. diff --git a/ios/help/basic/using-the-banner.md b/ios/help/basic/using-the-banner.md new file mode 100644 index 00000000000..dc1edf2479c --- /dev/null +++ b/ios/help/basic/using-the-banner.md @@ -0,0 +1,29 @@ +--- +title: Using the Banner on the Keyboard - Keyman for iPhone and iPad Help +--- + +## About the Keyboard Banner + +Keyman keyboards now always display a banner above the keyboard for one of the following functionalities: + +* Display suggestions (See "Using the Suggestion Banner" below) +* Display a Keyman-themed banner so popups and gestures for the top row of keys are visible +* Reserved for future functionality + +## Using the Suggestion Banner + +If a [dictionary is installed](installing-custom-keyboards-dictionaries) and enabled for the active Keyman +keyboard, the banner will display suggestions that can be selected. + +![](../ios_images/settings-suggestions-i.png) + +* Drag the banner horizontally to see more suggestions +* Overly-long suggestions are partially hidden, but expand when a finger is held on them +* The banner will display up to 8 suggestions at a time. + +## The Keyman-Themed Banner + +When suggestions are disabled or unavailable, the Keyman-themed banner will display instead. +This is displayed so that key-popups and gestures for the top row of keys are visible. + +![](../ios_images/themed-banner.png) \ No newline at end of file diff --git a/ios/help/index.md b/ios/help/index.md index 20601382f9f..c71c703e728 100644 --- a/ios/help/index.md +++ b/ios/help/index.md @@ -15,6 +15,7 @@ title: Keyman for iPhone and iPad 18.0 Help ## [Using Keyman for iPhone and iPad](basic/) +* [Using the Banner on the Keyboard](basic/using-the-banner) * [Switching Between Keyboards](basic/switching-between-keyboards) * [Using a Keyboard](basic/keyboard-usage) * [Using the Menu](context/) diff --git a/ios/help/ios_images/banner.png b/ios/help/ios_images/banner.png deleted file mode 100644 index e7d38e8a2ee..00000000000 Binary files a/ios/help/ios_images/banner.png and /dev/null differ diff --git a/ios/help/ios_images/dist-install1-i.png b/ios/help/ios_images/dist-install1-i.png index e4632eafc93..4871c99ea5d 100644 Binary files a/ios/help/ios_images/dist-install1-i.png and b/ios/help/ios_images/dist-install1-i.png differ diff --git a/ios/help/ios_images/down-flick-i.gif b/ios/help/ios_images/down-flick-i.gif new file mode 100644 index 00000000000..35734c3d159 Binary files /dev/null and b/ios/help/ios_images/down-flick-i.gif differ diff --git a/ios/help/ios_images/key-submenu.png b/ios/help/ios_images/key-submenu.png index faef6f5e8f0..58b029ca92a 100644 Binary files a/ios/help/ios_images/key-submenu.png and b/ios/help/ios_images/key-submenu.png differ diff --git a/ios/help/ios_images/lang-list.png b/ios/help/ios_images/lang-list.png deleted file mode 100644 index a4f966467ee..00000000000 Binary files a/ios/help/ios_images/lang-list.png and /dev/null differ diff --git a/ios/help/ios_images/language-settings.png b/ios/help/ios_images/language-settings.png index 1cea1ffcc11..80f4a5ea63f 100644 Binary files a/ios/help/ios_images/language-settings.png and b/ios/help/ios_images/language-settings.png differ diff --git a/ios/help/ios_images/no-banner.png b/ios/help/ios_images/no-banner.png deleted file mode 100644 index 68a19f67f6c..00000000000 Binary files a/ios/help/ios_images/no-banner.png and /dev/null differ diff --git a/ios/help/ios_images/settings-suggestions-i.png b/ios/help/ios_images/settings-suggestions-i.png new file mode 100644 index 00000000000..7018a16bbd9 Binary files /dev/null and b/ios/help/ios_images/settings-suggestions-i.png differ diff --git a/ios/help/ios_images/system-keyboard-cal.png b/ios/help/ios_images/system-keyboard-cal.png index 6c60f41a74c..49259f7938c 100644 Binary files a/ios/help/ios_images/system-keyboard-cal.png and b/ios/help/ios_images/system-keyboard-cal.png differ diff --git a/ios/help/ios_images/system-picker-i.png b/ios/help/ios_images/system-picker-i.png index b7e1d7696f2..f9d3c1fac7a 100644 Binary files a/ios/help/ios_images/system-picker-i.png and b/ios/help/ios_images/system-picker-i.png differ diff --git a/ios/help/ios_images/themed-banner.png b/ios/help/ios_images/themed-banner.png new file mode 100644 index 00000000000..a8fd5e2daa0 Binary files /dev/null and b/ios/help/ios_images/themed-banner.png differ diff --git a/mac/Keyman4MacIM/Keyman4MacIM/KMInputMethodEventHandler.m b/mac/Keyman4MacIM/Keyman4MacIM/KMInputMethodEventHandler.m index 2ebda0470a6..0cb52f77972 100644 --- a/mac/Keyman4MacIM/Keyman4MacIM/KMInputMethodEventHandler.m +++ b/mac/Keyman4MacIM/Keyman4MacIM/KMInputMethodEventHandler.m @@ -306,21 +306,33 @@ -(void)reportContext:(NSEvent *)event forClient:(id) client { } -(NSString*)readContext:(NSEvent *)event forClient:(id) client { - NSString *contextString = nil; + NSString *contextString = @""; NSAttributedString *attributedString = nil; - // if we can read the text, then get the context + // if we can read the text, then get the context for up to kMaxContext characters if (self.apiCompliance.canReadText) { NSRange selectionRange = [client selectedRange]; - NSRange contextRange = NSMakeRange(0, selectionRange.location); - attributedString = [client attributedSubstringFromRange:contextRange]; + NSUInteger contextLength = MIN(kMaxContext, selectionRange.location); + NSUInteger contextStart = selectionRange.location - contextLength; + + if (contextLength > 0) { + [self.appDelegate logDebugMessage:@" *** InputMethodEventHandler readContext, %d characters", contextLength]; + NSRange contextRange = NSMakeRange(contextStart, contextLength); + attributedString = [client attributedSubstringFromRange:contextRange]; + + // adjust string in case that we receive half of a surrogate pair at context start + // the API appears to always return a full code point, but this could vary by app + if (attributedString.length > 0) { + if (CFStringIsSurrogateLowCharacter([attributedString.string characterAtIndex:0])) { + [self.appDelegate logDebugMessage:@" *** InputMethodEventHandler readContext, first char is low surrogate, reducing context by one character"]; + contextString = [attributedString.string substringFromIndex:1]; + } else { + contextString = attributedString.string; + } + } + } } - if(attributedString == nil) { - contextString = @""; - } else { - contextString = attributedString.string; - } return contextString; } @@ -427,7 +439,7 @@ -(void) applyCapsLock:(CapsLockState)capsLockState { } -(void)insertAndReplaceTextForOutput:(CoreKeyOutput*)output client:(id) client { - [self insertAndReplaceText:output.textToInsert deleteCount:(int)output.codePointsToDeleteBeforeInsert client:client]; + [self insertAndReplaceText:output.textToInsert deleteCount:(int)output.codePointsToDeleteBeforeInsert toReplace:output.textToDelete client:client]; } /** @@ -435,35 +447,10 @@ -(void)insertAndReplaceTextForOutput:(CoreKeyOutput*)output client:(id) client { * Because this method depends on the selectedRange API which is not implemented correctly for some client applications, * this method can only be used if approved by TextApiCompliance */ --(void)insertAndReplaceText:(NSString *)text deleteCount:(int) replacementCount client:(id) client { - +-(void)insertAndReplaceText:(NSString *)text deleteCount:(int) replacementCount toReplace:(NSString*)textToDelete client:(id) client { [self.appDelegate logDebugMessage:@"KXMInputMethodHandler insertAndReplaceText: %@ replacementCount: %d", text, replacementCount]; - int actualReplacementCount = 0; - NSRange replacementRange; - NSRange notFoundRange = NSMakeRange(NSNotFound, NSNotFound); - - if (replacementCount > 0) { - NSRange selectionRange = [client selectedRange]; - - // make sure we have room to apply backspaces from current location - actualReplacementCount = MIN(replacementCount, (int)selectionRange.location); - - // log this: it is a sign that we are out of sync - if (actualReplacementCount < replacementCount) { - [self.appDelegate logDebugMessage:@"KXMInputMethodHandler insertAndReplaceText, replacement of %d characters limited by start of context to actual count: %d", replacementCount, actualReplacementCount]; - } - - if (actualReplacementCount > 0) { - [self.appDelegate logDebugMessage:@"KXMInputMethodHandler insertAndReplaceText, actualReplacementCount=%d, selectionRange.location=%lu", actualReplacementCount, selectionRange.location]; - - replacementRange = NSMakeRange(selectionRange.location-actualReplacementCount, replacementCount); - [self.appDelegate logDebugMessage:@"KXMInputMethodHandler insertAndReplaceText, insertText %@ in replacementRange.start=%lu, replacementRange.length=%lu", text, replacementRange.location, replacementRange.length]; - } else { - replacementRange = notFoundRange; - } - } else { - replacementRange = notFoundRange; - } + NSRange selectionRange = [client selectedRange]; + NSRange replacementRange = [self calculateInsertRangeForDeletedText:textToDelete selectionRange:selectionRange]; [client insertText:text replacementRange:replacementRange]; if (self.apiCompliance.isComplianceUncertain) { @@ -471,6 +458,28 @@ -(void)insertAndReplaceText:(NSString *)text deleteCount:(int) replacementCount } } +/* + * Calculates the range where text will be inserted and replace existing text. + * Returning {NSNotFound, NSNotFound} for range signifies to insert at current location without replacement. + */ +-(NSRange) calculateInsertRangeForDeletedText:(NSString*)textToDelete selectionRange:(NSRange) selection { + NSRange notFoundRange = NSMakeRange(NSNotFound, NSNotFound); + NSRange resultingReplacementRange = NSMakeRange(0, 0); + + // range = current location if either + // 1. no text has been designated to be deleted, or + // 2. the length of the text to delete is impossible (greater than the location) + if ((textToDelete.length == 0) || (textToDelete.length > selection.location)) { + // magic value of {NSNotFound, NSNotFound} passed to insertText means "do not replace, insert in current location" + return notFoundRange; + } else { + resultingReplacementRange.length = textToDelete.length + selection.length; + resultingReplacementRange.location = selection.location - textToDelete.length; + } + + return resultingReplacementRange; +} + -(void)sendEvents:(NSEvent *)event forOutput:(CoreKeyOutput*)output { [self.appDelegate logDebugMessage:@"sendEvents called, output = %@", output]; @@ -496,7 +505,7 @@ -(void)sendEvents:(NSEvent *)event forOutput:(CoreKeyOutput*)output { -(void)insertQueuedText: (NSEvent *)event client:(id) client { if (self.queuedText.length> 0) { [self.appDelegate logDebugMessage:@"insertQueuedText, inserting %@", self.queuedText]; - [self insertAndReplaceText:self.queuedText deleteCount:0 client:client]; + [self insertAndReplaceText:self.queuedText deleteCount:0 toReplace:nil client:client]; self.queuedText = nil; } else { [self.appDelegate logDebugMessage:@"insertQueuedText called but no text to insert"]; diff --git a/mac/Keyman4MacIM/KeymanTests/InputMethodTests.m b/mac/Keyman4MacIM/KeymanTests/InputMethodTests.m index 1acd05384d0..ab19175bb46 100644 --- a/mac/Keyman4MacIM/KeymanTests/InputMethodTests.m +++ b/mac/Keyman4MacIM/KeymanTests/InputMethodTests.m @@ -11,12 +11,13 @@ #import #import "KMInputMethodEventHandler.h" -#import "LegacyTestClient.h" #import "AppleCompliantTestClient.h" #import "TextApiCompliance.h" +#import "AppleCompliantTestClient.h" -@interface InputMethodTests : XCTestCase +KMInputMethodEventHandler *testEventHandler = nil; +@interface InputMethodTests : XCTestCase @end // included following interface that we can see and test private methods of TextApiCompliance @@ -26,10 +27,19 @@ - (BOOL)arrayContainsApplicationId:(NSString *)clientAppId fromArray:(NSArray *) @end +@interface KMInputMethodEventHandler (Testing) + +- (instancetype)initWithClient:(NSString *)clientAppId client:(id) sender; +- (NSRange) calculateInsertRangeForDeletedText:(NSString*)textToDelete selectionRange:(NSRange) selection; + +@end + @implementation InputMethodTests - (void)setUp { - // Put setup code here. This method is called before the invocation of each test method in the class. + id client = [[AppleCompliantTestClient alloc] init]; + NSString *clientAppId = @"com.compliant.app"; + testEventHandler = [[KMInputMethodEventHandler alloc]initWithClient:clientAppId client:client]; } - (void)tearDown { @@ -45,7 +55,7 @@ - (void)testIsClientAppLegacy_unlistedClientAppId_returnsNo { BOOL isLegacy = [apiCompliance arrayContainsApplicationId:clientAppId fromArray:legacyAppsArray]; NSLog(@"isLegacy = %@", isLegacy?@"yes":@"no"); - XCTAssert(isLegacy == NO, @"App not expected to be in legacy list"); + XCTAssertFalse(isLegacy, @"App not expected to be in legacy list"); } - (void)testIsClientAppLegacy_listedClientAppId_returnsYes { @@ -56,8 +66,54 @@ - (void)testIsClientAppLegacy_listedClientAppId_returnsYes { NSArray *legacyAppsArray = [NSArray arrayWithObjects:@"com.adobe.Photoshop",@"com.microsoft.VSCode",nil]; BOOL isLegacy = [apiCompliance arrayContainsApplicationId:clientAppId fromArray:legacyAppsArray]; - NSLog(@"isLegacy = %@", isLegacy?@"yes":@"no"); - XCTAssert(isLegacy == YES, @"App expected to be in legacy list"); + XCTAssertTrue(isLegacy, @"App expected to be in legacy list"); +} + +- (void)testCalculateInsertRange_noDelete_returnsCurrentLocation { + NSRange selectionRange = NSMakeRange(1, 0); + NSRange insertRange = [testEventHandler calculateInsertRangeForDeletedText:@"" selectionRange:selectionRange]; + BOOL notFound = (insertRange.length == NSNotFound) && (insertRange.location == NSNotFound); + XCTAssertTrue(notFound, @"insert or replacement range expected to be NSNotFound "); +} + +- (void)testCalculateInsertRange_noDeleteWithOneSelected_returnsCurrentLocation { + NSRange selectionRange = NSMakeRange(1, 1); + NSRange insertRange = [testEventHandler calculateInsertRangeForDeletedText:@"" selectionRange:selectionRange]; + BOOL notFound = (insertRange.length == NSNotFound) && (insertRange.location == NSNotFound); + XCTAssertTrue(notFound, @"insert or replacement range expected to be NSNotFound "); +} + +- (void)testCalculateInsertRange_deleteNonexistentCharacters_returnsCurrentLocation { + NSRange selectionRange = NSMakeRange(0, 0); + NSRange insertRange = [testEventHandler calculateInsertRangeForDeletedText:@"a" selectionRange:selectionRange]; + BOOL notFound = (insertRange.length == NSNotFound) && (insertRange.location == NSNotFound); + XCTAssertTrue(notFound, @"insert or replacement range expected to be NSNotFound "); +} + +- (void)testCalculateInsertRange_deleteOneSurrogatePair_returnsRangeLengthTwo { + // Brahmi 𑀓 D804 DC13 surrogate pair + NSRange selectionRange = NSMakeRange(2, 0); + NSRange insertRange = [testEventHandler calculateInsertRangeForDeletedText:@"𑀓" selectionRange:selectionRange]; + BOOL correctResult = (insertRange.location == 0) && (insertRange.length == 2); + XCTAssertTrue(correctResult, @"insert or replacement range expected to be {0,2}"); +} + +- (void)testCalculateInsertRange_deleteTwoBMPCharacters_returnsRangeLengthTwo { + NSRange selectionRange = NSMakeRange(3, 0); + // deletedText = KHMER VOWEL SIGN OE, KHMER SIGN COENG + NSRange insertRange = [testEventHandler calculateInsertRangeForDeletedText:@"\u17BE\u17D2" selectionRange:selectionRange]; + BOOL correctResult = (insertRange.location == 1) && (insertRange.length == 2); + XCTAssertTrue(correctResult, @"insert or replacement range expected to be {1,2}"); +} + +// equivalent to Kenya BTL, type "b^x", select 'x' and type 'u' +// selected x should be deleted, as should "^" and replaced with 'û' + +- (void)testCalculateInsertRange_deleteOneBMPCharacterWithOneSelected_returnsRangeLengthTwo { + NSRange selectionRange = NSMakeRange(2, 1); // location = 2, length = 1 + NSRange insertRange = [testEventHandler calculateInsertRangeForDeletedText:@"'" selectionRange:selectionRange]; + BOOL correctResult = (insertRange.location == 1) && (insertRange.length == 2); + XCTAssertTrue(correctResult, @"insert or replacement range expected to be {1,2}"); } @end diff --git a/mac/KeymanEngine4Mac/KeymanEngine4Mac.xcodeproj/xcshareddata/xcschemes/KeymanEngine4Mac.xcscheme b/mac/KeymanEngine4Mac/KeymanEngine4Mac.xcodeproj/xcshareddata/xcschemes/KeymanEngine4Mac.xcscheme index 3668b806f5e..8a40efd6e32 100644 --- a/mac/KeymanEngine4Mac/KeymanEngine4Mac.xcodeproj/xcshareddata/xcschemes/KeymanEngine4Mac.xcscheme +++ b/mac/KeymanEngine4Mac/KeymanEngine4Mac.xcodeproj/xcshareddata/xcschemes/KeymanEngine4Mac.xcscheme @@ -106,6 +106,16 @@ + + + + _codePointsToDeleteBeforeInsert = codePointsToDelete; + self->_textToDelete = deletedText; self->_textToInsert = text; self->_optionsToPersist = options; self->_alert = alert; diff --git a/mac/KeymanEngine4Mac/KeymanEngine4Mac/CoreWrapper/CoreWrapper.m b/mac/KeymanEngine4Mac/KeymanEngine4Mac/CoreWrapper/CoreWrapper.m index b8533b18e2a..0073ce35b91 100644 --- a/mac/KeymanEngine4Mac/KeymanEngine4Mac/CoreWrapper/CoreWrapper.m +++ b/mac/KeymanEngine4Mac/KeymanEngine4Mac/CoreWrapper/CoreWrapper.m @@ -181,9 +181,10 @@ -(CoreKeyOutput*) createCoreKeyOutputForActionsStruct:(km_core_actions*)actions NSString* text = [self.coreHelper utf32CStringToString:actions->output]; NSDictionary* options = [self convertOptionsArray:actions->persist_options]; CapsLockState capsLock = [self convertCapsLockState:actions->new_caps_lock_state]; + NSString* deletedText = [self.coreHelper utf32CStringToString:actions->deleted_context]; - CoreKeyOutput* coreKeyOutput = [[CoreKeyOutput alloc] init: actions->code_points_to_delete textToInsert:text optionsToPersist:options alert:actions->do_alert emitKeystroke:actions->emit_keystroke capsLockState:capsLock]; - + CoreKeyOutput* coreKeyOutput = [[CoreKeyOutput alloc] init: actions->code_points_to_delete textToDelete:deletedText textToInsert:text optionsToPersist:options alert:actions->do_alert emitKeystroke:actions->emit_keystroke capsLockState:capsLock]; + return coreKeyOutput; } diff --git a/mac/KeymanEngine4Mac/KeymanEngine4MacTests/CoreHelperTests.m b/mac/KeymanEngine4Mac/KeymanEngine4MacTests/CoreHelperTests.m index 8a1086fed98..baa4190f1e7 100644 --- a/mac/KeymanEngine4Mac/KeymanEngine4MacTests/CoreHelperTests.m +++ b/mac/KeymanEngine4Mac/KeymanEngine4MacTests/CoreHelperTests.m @@ -115,6 +115,18 @@ - (void)testConversionToUnicharString_number_matchesLiteral { XCTAssertTrue([dataFromLiteral isEqualToData:dataFromConversion], @"Converted unichar string is not equal to literal unichar string."); } +- (void)testConversionToUnicharString_brahmiAndLatinString_matchesLiteral { + NSString *mixedString = @"𑀧a𑀫"; + unichar const * mixedUnicharString = u"𑀧a𑀫"; + + NSUInteger stringLength = [mixedString lengthOfBytesUsingEncoding:NSUTF16StringEncoding]; + + unichar const * convertedString = [[CoreTestStaticHelperMethods helper] createUnicharStringFromNSString: mixedString]; + NSData *dataFromConversion = [NSData dataWithBytes:convertedString length:stringLength]; + NSData *dataFromLiteral = [NSData dataWithBytes:mixedUnicharString length:stringLength]; + XCTAssertTrue([dataFromLiteral isEqualToData:dataFromConversion], @"Converted unichar string is not equal to literal unichar string."); +} + - (void)testConversionFromUnicharString_optionName_matchesLiteral { unichar const * unicharString = u"option_ligature_ew"; NSString *optionNameString = @"option_ligature_ew"; diff --git a/oem/firstvoices/keyboards.csv b/oem/firstvoices/keyboards.csv index e11dfab50b2..1bc76fd58ed 100644 --- a/oem/firstvoices/keyboards.csv +++ b/oem/firstvoices/keyboards.csv @@ -11,11 +11,11 @@ fv,fv_uwikala,'Uwik̓ala,BC Coast,fv_uwikala_kmw-9.0.js,9.3,hei,Heiltsuk fv,fv_dexwlesucid,Dəxʷləšucid,BC Coast,fv_dexwlesucid_kmw-9.0.js,9.2.1,lut-Latn,Lushootseed (Latin) fv,fv_diitiidatx,Diidiitidq,BC Coast,fv_diitiidatx_kmw-9.0.js,9.1.3,nuk-Latn,Nuu-chah-nulth (Latin) fv,fv_gitsenimx,Gitsenimx̱,BC Coast,fv_gitsenimx_kmw-9.0.js,10.0.1,git,Gitxsan (Latin) -fv,fv_hailzaqvla,Haiɫzaqvla,BC Coast,fv_hailzaqvla_kmw-9.0.js,9.5,hei,Heiltsuk (Latin) -fv,fv_haisla,Haisla,BC Coast,fv_haisla.js,1.0,has-Latn,Haisla (Latin) +fv,fv_hailzaqvla,Haiɫzaqvla,BC Coast,fv_hailzaqvla_kmw-9.0.js,9.5.1,hei,Heiltsuk (Latin) +fv,fv_haisla,Haisla,BC Coast,fv_haisla.js,2.0.1,has-Latn,Haisla (Latin) fv,fv_halqemeylem,Halq'eméylem,BC Coast,fv_halqemeylem_kmw-9.0.js,9.1.3,hur-Latn,Halkomelem (Latin) -fv,fv_henqeminem,Hǝn̓q̓ǝmin̓ǝm,BC Coast,fv_henqeminem_kmw-9.0.js,9.1.3,hur-Latn,Halkomelem (Latin) -fv,fv_klahoose,Homalco-Klahoose-Sliammon,BC Coast,fv_klahoose_kmw-9.0.js,10.0,coo,Comox +fv,fv_henqeminem,Hǝn̓q̓ǝmin̓ǝm,BC Coast,fv_henqeminem_kmw-9.0.js,10.0.1,hur-Latn,Halkomelem (Latin) +fv,fv_klahoose,Homalco-Klahoose-Sliammon,BC Coast,fv_klahoose_kmw-9.0.js,10.1,coo,Comox fv,fv_hulquminum,Hul’q’umi’num’,BC Coast,fv_hulquminum_kmw-9.0.js,9.1,hur-Latn,Halkomelem (Latin) fv,fv_hulquminum_combine,Hul̓q̓umin̓um̓,BC Coast,fv_hulquminum_combine_kmw-9.0.js,1.0,hur-Latn,Halkomelem (Latin) fv,fv_kwakwala_liqwala,Kʷak̓ʷala,BC Coast,fv_kwakwala_liqwala_kmw-9.0.js,9.2.5,kwk-Latn,Kwakiutl (Latin) @@ -25,8 +25,9 @@ fv,fv_nisgaa,Nisg̱a'a,BC Coast,fv_nisgaa_kmw-9.0.js,9.1.2,ncg-Latn,Nisga'a (Lat fv,fv_nuucaanul,Nuučaan̓uł,BC Coast,fv_nuucaanul_kmw-9.0.js,9.1.4,nuk-Latn,Nuu-chah-nulth (Latin) fv,fv_nuxalk,Nuxalk,BC Coast,fv_nuxalk_kmw-9.0.js,10.0,blc-Latn,Bella Coola (Latin) fv,fv_sencoten,SENĆOŦEN,BC Coast,fv_sencoten_kmw-9.0.js,9.2.1,str,Straits Salish +fv,fv_sguuxs,Sgüüx̱s,BC Coast,fv_sguuxs.js,1.0,tsi,Tsimshian fv,fv_shashishalhem,Shashishalhem,BC Coast,fv_shashishalhem_kmw-9.0.js,9.1.3,sec-Latn,Sechelt (Latin) -fv,fv_skwxwumesh_snichim,Sḵwx̱wúmesh sníchim,BC Coast,fv_skwxwumesh_snichim_kmw-9.0.js,9.2.1,squ-Latn,Squamish (Latin) +fv,fv_skwxwumesh_snichim,Sḵwx̱wúmesh sníchim,BC Coast,fv_skwxwumesh_snichim_kmw-9.0.js,9.3,squ-Latn,Squamish (Latin) fv,fv_smalgyax,Sm'algya̱x,BC Coast,fv_smalgyax_kmw-9.0.js,9.1.3,tsi-Latn,Tsimshian (Latin) fv,fv_xaislakala,X̄a'ʼislak̓ala,BC Coast,fv_xaislakala_kmw-9.0.js,9.1.1,has-Latn,Haisla (Latin) fv,fv_hlgaagilda_xaayda_kil,X̱aayda-X̱aad Kil,BC Coast,fv_hlgaagilda_xaayda_kil_kmw-9.0.js,9.3,hax,Southern Haida @@ -37,13 +38,13 @@ fv,fv_natwits,Nedut’en-Witsuwit'en,BC Interior,fv_natwits_kmw-9.0.js,9.1.3,caf fv,fv_nlekepmxcin,Nłeʔkepmxcin,BC Interior,fv_nlekepmxcin_kmw-9.0.js,9.2.3,thp-Latn,Thompson (Latin) fv,fv_nlha7kapmxtsin,Nlha7kapmxtsin,BC Interior,fv_nlha7kapmxtsin_kmw-9.0.js,9.1.1,thp-Latn,Thompson (Latin) fv,fv_nsilxcen,Nsilxcən,BC Interior,fv_nsilxcen_kmw-9.0.js,9.3,oka,Okanagan -fv,fv_secwepemctsin,Secwepemctsín,BC Interior,fv_secwepemctsin_kmw-9.0.js,9.1.2,shs-Latn,Shuswap (Latin) +fv,fv_secwepemctsin,Secwepemctsín,BC Interior,fv_secwepemctsin_kmw-9.0.js,9.2,shs-Latn,Shuswap (Latin) fv,fv_stlatlimxec,Sƛ̓aƛ̓imxəc,BC Interior,fv_stlatlimxec_kmw-9.0.js,9.2.3,lil-Latn,Lillooet (Latin) -fv,fv_statimcets,St̓át̓imcets,BC Interior,fv_statimcets_kmw-9.0.js,9.1.3,lil-Latn,Lillooet (Latin) +fv,fv_statimcets,St̓át̓imcets,BC Interior,fv_statimcets_kmw-9.0.js,9.1.4,lil-Latn,Lillooet (Latin) fv,fv_taltan,Tāłtān,BC Interior,fv_taltan_kmw-9.0.js,9.1.5,tht-Latn,Tahltan (Latin) fv,fv_tsekehne,Tsek'ehne,BC Interior,fv_tsekehne_kmw-9.0.js,9.1.2,sek-Latn,Sekani (Latin) fv,fv_tsilhqotin,Tŝilhqot'in,BC Interior,fv_tsilhqotin_kmw-9.0.js,9.1.3,clc-Latn,Chilcotin (Latin) -fv,fv_southern_carrier,ᑐᑊᘁᗕᑋᗸ (Southern Carrier),BC Interior,fv_southern_carrier_kmw-9.0.js,9.2.1,caf-Cans,Southern Carrier (Unified Canadian Aboriginal Syllabics) +fv,fv_southern_carrier,ᑐᑊᘁᗕᑋᗸ (Southern Carrier),BC Interior,fv_southern_carrier_kmw-9.0.js,10.0,caf-Cans,Southern Carrier (Unified Canadian Aboriginal Syllabics) fv,fv_anicinapemi8in,Anicinapemi8in/Anishinàbemiwin,Eastern Subarctic,fv_anicinapemi8in_kmw-9.0.js,9.1.1,alq-Latn,Algonquin (Latin) fv,fv_atikamekw,Atikamekw,Eastern Subarctic,fv_atikamekw_kmw-9.0.js,9.1.1,atj-Latn,Atikamekw (Latin) fv,fv_ilnu_innu_aimun,Ilnu-Innu Aimun,Eastern Subarctic,fv_ilnu_innu_aimun_kmw-9.0.js,9.1.1,moe-Latn,Montagnais (Latin) @@ -69,14 +70,14 @@ fv,fv_wobanakiodwawogan,Wôbanakiôdwawôgan,Great Lakes - St. Lawrence,fv_woban fv,fv_australian,Australian,Pacific,fv_australian_kmw-9.0.js,9.3.1,pjt-Latn,Pitjantjatjara (Latin) fv,fv_maori,Māori,Pacific,fv_maori_kmw-9.0.js,9.1.1,mi-Latn,Maori (Latin) fv,fv_blackfoot,Blackfoot,Prairies,fv_blackfoot_kmw-9.0.js,9.2.1,bla-Latn,Siksika (Latin) -fv,fv_cree_latin,Cree - Roman Orthography,Prairies,fv_cree_latin_kmw-9.0.js,10.0,cr-Latn,Cree (Latin) +fv,fv_cree_latin,Cree - Roman Orthography,Prairies,fv_cree_latin_kmw-9.0.js,10.0.1,cr-Latn,Cree (Latin) fv,fv_dakota_mb,Dakota,Prairies,fv_dakota_mb_kmw-9.0.js,9.1.1,dak-Latn,Dakota (Latin) fv,fv_dakota_sk,Dakot̄a,Prairies,fV_dakota_sk_kmw-9.0.js,9.1.1,dak-Latn,Dakota (Latin) fv,fv_isga_iabi,Isga Iʔabi,Prairies,fv_isga_iabi_kmw-9.0.js,9.1.1,sto-Latn,Stoney (Latin) fv,fv_lakota,Lak̇ot̄a,Prairies,fv_lakota-9.0.js,9.1.1,lkt-Latn,Lakota (Latin) fv,fv_nakoda,Nakoda,Prairies,fv_nakoda_kmw-9.0.js,9.1.1,asb-Latn,Assiniboine (Latin) fv,fv_tsuutina,Tsúùt'ínà,Prairies,fv_tsuutina_kmw-9.0.js,9.1.1,srs-Latn,Sarsi (Latin) -fv,fv_plains_cree,ᓀᐦᐃᔭᐍᐏᐣ (Plains Cree),Prairies,fv_plains_cree_kmw-9.0.js,10.0.2,crk-Cans,ᓀᐦᐃᔭᐍᐏᐣ (Cree syllabics) +fv,fv_plains_cree,ᓀᐦᐃᔭᐍᐏᐣ (Plains Cree),Prairies,fv_plains_cree_kmw-9.0.js,11.0,crk-Cans,ᓀᐦᐃᔭᐍᐏᐣ (Cree syllabics) fv,fv_dine_bizaad,Diné Bizaad,South West,fv_dine_bizaad_kmw-9.0.js,9.1.1,nv-Latn,Navajo (Latin) fv,fv_dane_zaa_zaage,Dane-Z̲aa Z̲áágéʔ,Western Subarctic,fv_dane_zaa_zaage_kmw-9.0.js,9.3,bea,Beaver fv,fv_dene_dzage,Dene Dzage,Western Subarctic,fv_dene_dzage_kmw-9.0.js,9.1.1,kkz-Latn,Kaska (Latin) diff --git a/package-lock.json b/package-lock.json index 3a19da0233d..234746d352d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -995,7 +995,7 @@ "kmlmp": "build/src/kmlmp.js" }, "devDependencies": { - "@sentry/cli": "^2.19.4", + "@sentry/cli": "^2.31.0", "@types/chai": "^4.1.7", "@types/mocha": "^5.2.7", "@types/node": "^20.4.1", @@ -2032,6 +2032,7 @@ }, "devDependencies": { "@keymanapp/developer-test-helpers": "*", + "@keymanapp/models-templates": "*", "@types/chai": "^4.1.7", "@types/mocha": "^5.2.7", "@types/node": "^20.4.1", @@ -2546,26 +2547,6 @@ "node": ">=0.3.1" } }, - "developer/src/kmc/node_modules/@sentry/cli": { - "version": "2.19.4", - "resolved": "https://registry.npmjs.org/@sentry/cli/-/cli-2.19.4.tgz", - "integrity": "sha512-wsSr2O/GVgr/i+DYtie+DNhODyI+HL7F5/0t1HwWMjHJWm4+5XTEauznYgbh2mewkzfUk9+t0CPecA82lEgspg==", - "dev": true, - "hasInstallScript": true, - "dependencies": { - "https-proxy-agent": "^5.0.0", - "node-fetch": "^2.6.7", - "progress": "^2.0.3", - "proxy-from-env": "^1.1.0", - "which": "^2.0.2" - }, - "bin": { - "sentry-cli": "bin/sentry-cli" - }, - "engines": { - "node": ">= 10" - } - }, "developer/src/kmc/node_modules/@types/mocha": { "version": "5.2.7", "dev": true, @@ -3791,14 +3772,14 @@ "integrity": "sha512-Xni35NKzjgMrwevysHTCArtLDpPvye8zV/0E4EyYn43P7/7qvQwPh9BGkHewbMulVntbigmcT7rdX3BNo9wRJg==" }, "node_modules/@sentry/cli": { - "version": "2.2.0", + "version": "2.31.0", + "resolved": "https://registry.npmjs.org/@sentry/cli/-/cli-2.31.0.tgz", + "integrity": "sha512-nCESoXAG3kRUO5n3QbDYAqX6RU3z1ORjnd7a3sqijYsCGHfOpcjGdS7JYLVg5if+tXMEF5529BPXFe5Kg/J9tw==", "dev": true, "hasInstallScript": true, - "license": "BSD-3-Clause", "dependencies": { "https-proxy-agent": "^5.0.0", "node-fetch": "^2.6.7", - "npmlog": "^6.0.1", "progress": "^2.0.3", "proxy-from-env": "^1.1.0", "which": "^2.0.2" @@ -3807,7 +3788,131 @@ "sentry-cli": "bin/sentry-cli" }, "engines": { - "node": ">= 12" + "node": ">= 10" + }, + "optionalDependencies": { + "@sentry/cli-darwin": "2.31.0", + "@sentry/cli-linux-arm": "2.31.0", + "@sentry/cli-linux-arm64": "2.31.0", + "@sentry/cli-linux-i686": "2.31.0", + "@sentry/cli-linux-x64": "2.31.0", + "@sentry/cli-win32-i686": "2.31.0", + "@sentry/cli-win32-x64": "2.31.0" + } + }, + "node_modules/@sentry/cli-darwin": { + "version": "2.31.0", + "resolved": "https://registry.npmjs.org/@sentry/cli-darwin/-/cli-darwin-2.31.0.tgz", + "integrity": "sha512-VM5liyxMnm4K2g0WsrRPXRCMLhaT09C7gK5Fz/CxKYh9sbMZB7KA4hV/3klkyuyw1+ECF1J66cefhNkFZepUig==", + "dev": true, + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">=10" + } + }, + "node_modules/@sentry/cli-linux-arm": { + "version": "2.31.0", + "resolved": "https://registry.npmjs.org/@sentry/cli-linux-arm/-/cli-linux-arm-2.31.0.tgz", + "integrity": "sha512-AZoCN3waXEfXGCd3YSrikcX/y63oQe0Tiyapkeoifq/0QhI+2MOOrAQb60gthsXwb0UDK/XeFi3PaxyUCphzxA==", + "cpu": [ + "arm" + ], + "dev": true, + "optional": true, + "os": [ + "linux", + "freebsd" + ], + "engines": { + "node": ">=10" + } + }, + "node_modules/@sentry/cli-linux-arm64": { + "version": "2.31.0", + "resolved": "https://registry.npmjs.org/@sentry/cli-linux-arm64/-/cli-linux-arm64-2.31.0.tgz", + "integrity": "sha512-eENJTmXoFX3uNr8xRW7Bua2Sw3V1tylQfdtS85pNjZPdbm3U8wYQSWu2VoZkK2ASOoC+17YC8jTQxq62KWnSeQ==", + "cpu": [ + "arm64" + ], + "dev": true, + "optional": true, + "os": [ + "linux", + "freebsd" + ], + "engines": { + "node": ">=10" + } + }, + "node_modules/@sentry/cli-linux-i686": { + "version": "2.31.0", + "resolved": "https://registry.npmjs.org/@sentry/cli-linux-i686/-/cli-linux-i686-2.31.0.tgz", + "integrity": "sha512-cQUFb3brhLaNSIoNzjU/YASnTM1I3TDJP9XXzH0eLK9sSopCcDcc6OrYEYvdjJXZKzFv5sbc9UNMsIDbh4+rYg==", + "cpu": [ + "x86", + "ia32" + ], + "dev": true, + "optional": true, + "os": [ + "linux", + "freebsd" + ], + "engines": { + "node": ">=10" + } + }, + "node_modules/@sentry/cli-linux-x64": { + "version": "2.31.0", + "resolved": "https://registry.npmjs.org/@sentry/cli-linux-x64/-/cli-linux-x64-2.31.0.tgz", + "integrity": "sha512-z1zTNg91nZJRdcGHC/bCU1KwIaifV0MLJteip9KrFDprzhJk1HtMxFOS0+OZ5/UH21CjAFmg9Pj6IAGqm3BYjA==", + "cpu": [ + "x64" + ], + "dev": true, + "optional": true, + "os": [ + "linux", + "freebsd" + ], + "engines": { + "node": ">=10" + } + }, + "node_modules/@sentry/cli-win32-i686": { + "version": "2.31.0", + "resolved": "https://registry.npmjs.org/@sentry/cli-win32-i686/-/cli-win32-i686-2.31.0.tgz", + "integrity": "sha512-+K7fdk57aUd4CmYrQfDGYPzVyxsTnVro6IPb5QSSLpP03dL7ko5208epu4m2SyN/MkFvscy9Di3n3DTvIfDU2w==", + "cpu": [ + "x86", + "ia32" + ], + "dev": true, + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=10" + } + }, + "node_modules/@sentry/cli-win32-x64": { + "version": "2.31.0", + "resolved": "https://registry.npmjs.org/@sentry/cli-win32-x64/-/cli-win32-x64-2.31.0.tgz", + "integrity": "sha512-w5cvpZ6VVlhlyleY8TYHmrP7g48vKHnoVt5xFccfxT+HqQI/AxodvzgVvBTM2kB/sh/kHwexp6bJGWCdkGftww==", + "cpu": [ + "x64" + ], + "dev": true, + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=10" } }, "node_modules/@sentry/core": { @@ -4774,13 +4879,13 @@ }, "node_modules/aproba": { "version": "2.0.0", - "devOptional": true, - "license": "ISC" + "license": "ISC", + "optional": true }, "node_modules/are-we-there-yet": { "version": "3.0.1", - "devOptional": true, "license": "ISC", + "optional": true, "dependencies": { "delegates": "^1.0.0", "readable-stream": "^3.6.0" @@ -4791,8 +4896,8 @@ }, "node_modules/are-we-there-yet/node_modules/readable-stream": { "version": "3.6.0", - "devOptional": true, "license": "MIT", + "optional": true, "dependencies": { "inherits": "^2.0.3", "string_decoder": "^1.1.1", @@ -5476,8 +5581,8 @@ }, "node_modules/color-support": { "version": "1.1.3", - "devOptional": true, "license": "ISC", + "optional": true, "bin": { "color-support": "bin.js" } @@ -5592,8 +5697,8 @@ }, "node_modules/console-control-strings": { "version": "1.1.0", - "devOptional": true, - "license": "ISC" + "license": "ISC", + "optional": true }, "node_modules/content-disposition": { "version": "0.5.4", @@ -5885,8 +5990,8 @@ }, "node_modules/delegates": { "version": "1.0.0", - "devOptional": true, - "license": "MIT" + "license": "MIT", + "optional": true }, "node_modules/depd": { "version": "1.1.2", @@ -7669,8 +7774,8 @@ }, "node_modules/gauge": { "version": "4.0.4", - "devOptional": true, "license": "ISC", + "optional": true, "dependencies": { "aproba": "^1.0.3 || ^2.0.0", "color-support": "^1.1.3", @@ -7687,24 +7792,24 @@ }, "node_modules/gauge/node_modules/ansi-regex": { "version": "5.0.1", - "devOptional": true, "license": "MIT", + "optional": true, "engines": { "node": ">=8" } }, "node_modules/gauge/node_modules/is-fullwidth-code-point": { "version": "3.0.0", - "devOptional": true, "license": "MIT", + "optional": true, "engines": { "node": ">=8" } }, "node_modules/gauge/node_modules/string-width": { "version": "4.2.3", - "devOptional": true, "license": "MIT", + "optional": true, "dependencies": { "emoji-regex": "^8.0.0", "is-fullwidth-code-point": "^3.0.0", @@ -7716,8 +7821,8 @@ }, "node_modules/gauge/node_modules/strip-ansi": { "version": "6.0.1", - "devOptional": true, "license": "MIT", + "optional": true, "dependencies": { "ansi-regex": "^5.0.1" }, @@ -7727,8 +7832,8 @@ }, "node_modules/gauge/node_modules/wide-align": { "version": "1.1.5", - "devOptional": true, "license": "ISC", + "optional": true, "dependencies": { "string-width": "^1.0.2 || 2 || 3 || 4" } @@ -8081,8 +8186,8 @@ }, "node_modules/has-unicode": { "version": "2.0.1", - "devOptional": true, - "license": "ISC" + "license": "ISC", + "optional": true }, "node_modules/he": { "version": "1.2.0", @@ -10355,8 +10460,8 @@ }, "node_modules/npmlog": { "version": "6.0.2", - "devOptional": true, "license": "ISC", + "optional": true, "dependencies": { "are-we-there-yet": "^3.0.0", "console-control-strings": "^1.1.0", @@ -12861,7 +12966,7 @@ "devDependencies": { "@keymanapp/resources-gosh": "*", "@keymanapp/web-sentry-manager": "*", - "@sentry/cli": "2.2.0", + "@sentry/cli": "^2.31.0", "c8": "^7.12.0", "chai": "^4.3.4", "jsdom": "^23.0.1", diff --git a/resources/build/ci/pull-requests.inc.sh b/resources/build/ci/pull-requests.inc.sh index 65fe0efee66..09c0903fe5f 100644 --- a/resources/build/ci/pull-requests.inc.sh +++ b/resources/build/ci/pull-requests.inc.sh @@ -107,7 +107,7 @@ function ci_open_pull_request { git push origin "$branch" builder_echo "Push complete" - hub pull-request -f --no-edit -l auto + hub pull-request --force --message "$commit_message" --labels auto builder_echo "Pull request created" git switch "$current_branch" diff --git a/web/package.json b/web/package.json index 5c2f6dcc401..683cd5c9d42 100644 --- a/web/package.json +++ b/web/package.json @@ -82,7 +82,7 @@ "devDependencies": { "@keymanapp/resources-gosh": "*", "@keymanapp/web-sentry-manager": "*", - "@sentry/cli": "2.2.0", + "@sentry/cli": "^2.31.0", "c8": "^7.12.0", "chai": "^4.3.4", "jsdom": "^23.0.1", diff --git a/web/src/app/webview/src/contextManager.ts b/web/src/app/webview/src/contextManager.ts index d9ef2c6e52b..a2450ad58c8 100644 --- a/web/src/app/webview/src/contextManager.ts +++ b/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); + } } } diff --git a/web/src/engine/main/src/contextManagerBase.ts b/web/src/engine/main/src/contextManagerBase.ts index 54cfc7e8dd2..df6542aebcc 100644 --- a/web/src/engine/main/src/contextManagerBase.ts +++ b/web/src/engine/main/src/contextManagerBase.ts @@ -274,8 +274,6 @@ export abstract class ContextManagerBase // (!wasNull || !!keyboard) - blocks events for `null` -> `null` transitions. // (keyman/keymanweb.com#96) if(this.currentKeyboardSrcTarget() == originalKeyboardTarget && (!wasNull || !!keyboard)) { - // Perform standard context-reset ops, including the processing of new-context events. - this.resetContext(); // Will trigger KeymanEngine handler that passes keyboard to the OSK, displays it. this.emit('keyboardchange', this.activeKeyboard); } diff --git a/web/src/engine/main/src/keymanEngine.ts b/web/src/engine/main/src/keymanEngine.ts index 1400c82e9cd..e6dfb063148 100644 --- a/web/src/engine/main/src/keymanEngine.ts +++ b/web/src/engine/main/src/keymanEngine.ts @@ -2,7 +2,7 @@ import { type Keyboard, KeyboardKeymanGlobal, ProcessorInitOptions } from "@keym import { DOMKeyboardLoader as KeyboardLoader } from "@keymanapp/keyboard-processor/dom-keyboard-loader"; import { InputProcessor, PredictionContext } from "@keymanapp/input-processor"; import { OSKView } from "keyman/engine/osk"; -import { KeyboardRequisitioner, ModelCache, ModelSpec, toUnprefixedKeyboardId as unprefixed } from "keyman/engine/package-cache"; +import { KeyboardRequisitioner, KeyboardStub, ModelCache, ModelSpec, toUnprefixedKeyboardId as unprefixed } from "keyman/engine/package-cache"; import { EngineConfiguration, InitOptionSpec } from "./engineConfiguration.js"; import KeyboardInterface from "./keyboardInterface.js"; @@ -137,14 +137,6 @@ export default class KeymanEngine< }); this.contextManager.on('keyboardchange', (kbd) => { - this.refreshModel(); - this.core.activeKeyboard = kbd?.keyboard; - - this.legacyAPIEvents.callEvent('keyboardchange', { - internalName: kbd?.metadata.id ?? '', - languageCode: kbd?.metadata.langId ?? '' - }); - // Hide OSK and do not update keyboard list if using internal keyboard (desktops). // Condition will not be met for touch form-factors; they force selection of a // default keyboard. @@ -152,16 +144,50 @@ export default class KeymanEngine< this.osk.startHide(false); } - if(this.osk) { - this.osk.setNeedsLayout(); - this.osk.activeKeyboard = kbd; - this.osk.present(); + const earlyBatchClosure = () => { + this.refreshModel(); + // Triggers context resets that can trigger layout stuff. + // It's not the final such context-reset, though. + this.core.activeKeyboard = kbd?.keyboard; + + this.legacyAPIEvents.callEvent('keyboardchange', { + internalName: kbd?.metadata.id ?? '', + languageCode: kbd?.metadata.langId ?? '' + }); + } + + /* + Needed to ensure the correct layer is displayed AND that deadkeys from + the old keyboard have been wiped. + + Needs to be after the OSK has loaded for the keyboard in case the default + layer should be something other than "default" for the current context. + */ + const doContextReset = () => { + this.contextManager.resetContext(); } - // Needed to ensure the correct layer is displayed. - // Needs to be after the OSK has loaded for the keyboard in case the default - // layer should be something other than "default" for the current context. - this.core.resetContext(this.contextManager.activeTarget); + /* + This pattern is designed to minimize layout reflow during the keyboard-swap process. + The 'default' layer is loaded by default, but some keyboards will start on different + layers depending on the current state of the context. + + If possible, we want to only perform layout operations once the correct layer is + set to active. + */ + if(this.osk) { + this.osk.batchLayoutAfter(() => { + earlyBatchClosure(); + this.osk.activeKeyboard = kbd; + // Note: when embedded within the mobile apps, the keyboard will still be visible + // at this time. + doContextReset(); + this.osk.present(); + }); + } else { + earlyBatchClosure(); + doContextReset(); + } }); this.contextManager.on('keyboardasyncload', (metadata) => { @@ -216,7 +242,15 @@ export default class KeymanEngine< resetContext: (target) => { // Could reset the target's deadkeys here, but it's really more of a 'core' task. // So we delegate that to keyboard-processor. - this.core.resetContext(target); + const doReset = () => this.core.resetContext(target); + + if(this.osk) { + this.osk.batchLayoutAfter(() => { + doReset(); + }) + } else { + doReset(); + } }, predictionContext: new PredictionContext(this.core.languageProcessor, this.core.keyboardProcessor), keyboardCache: this.keyboardRequisitioner.cache diff --git a/web/src/engine/osk/src/input/gestures/browser/keytip.ts b/web/src/engine/osk/src/input/gestures/browser/keytip.ts index a679093db02..5ebac04e1e2 100644 --- a/web/src/engine/osk/src/input/gestures/browser/keytip.ts +++ b/web/src/engine/osk/src/input/gestures/browser/keytip.ts @@ -3,6 +3,7 @@ import { KeyElement } from '../../../keyElement.js'; import KeyTipInterface from '../../../keytip.interface.js'; import VisualKeyboard from '../../../visualKeyboard.js'; import { GesturePreviewHost } from '../../../keyboard-layout/gesturePreviewHost.js'; +import { ParsedLengthStyle } from '../../../lengthStyle.js'; const CSS_PREFIX = 'kmw-'; const DEFAULT_TIP_ORIENTATION: PhoneKeyTipOrientation = 'top'; @@ -111,16 +112,15 @@ export default class KeyTip implements KeyTipInterface { let canvasWidth = xWidth + Math.ceil(xWidth * 0.3) * 2; let canvasHeight = Math.ceil(2.3 * xHeight) + (ySubPixelPadding); // - kts.top = 'auto'; + if(orientation == 'bottom') { + y += canvasHeight - xHeight; + } + kts.top = 'auto'; const unselectedOrientation = orientation == 'top' ? 'bottom' : 'top'; this.tip.classList.remove(`${CSS_PREFIX}${unselectedOrientation}`); this.tip.classList.add(`${CSS_PREFIX}${orientation}`); - if(orientation == 'bottom') { - y += canvasHeight - xHeight; - } - kts.bottom = Math.floor(_BoxRect.height - y) + 'px'; kts.textAlign = 'center'; kts.overflow = 'visible'; @@ -141,14 +141,14 @@ export default class KeyTip implements KeyTipInterface { } if(px != 0) { - let popupFS = previewFontScale * px; let scaleStyle = { - fontFamily: kts.fontFamily, - fontSize: popupFS + 'px', - height: 1.6 * xHeight + 'px' // as opposed to the canvas height of 2.3 * xHeight. + keyWidth: 1.6 * xWidth, + keyHeight: 1.6 * xHeight, // as opposed to the canvas height of 2.3 * xHeight. + baseEmFontSize: vkbd.getKeyEmFontSize(), + layoutFontSize: new ParsedLengthStyle(vkbd.kbdDiv.style.fontSize) }; - kts.fontSize = key.key.getIdealFontSize(vkbd, key.key.keyText, scaleStyle, true); + kts.fontSize = key.key.getIdealFontSize(key.key.keyText, scaleStyle, previewFontScale).styleString; } // Adjust shape if at edges diff --git a/web/src/engine/osk/src/keyboard-layout/oskBaseKey.ts b/web/src/engine/osk/src/keyboard-layout/oskBaseKey.ts index 16522a9ad39..2a273c17aba 100644 --- a/web/src/engine/osk/src/keyboard-layout/oskBaseKey.ts +++ b/web/src/engine/osk/src/keyboard-layout/oskBaseKey.ts @@ -1,14 +1,13 @@ import { ActiveKey, Codes, DeviceSpec } from '@keymanapp/keyboard-processor'; import { landscapeView } from 'keyman/engine/dom-utils'; -import OSKKey, { renameSpecialKey } from './oskKey.js'; +import OSKKey, { KeyLayoutParams, renameSpecialKey } from './oskKey.js'; import { KeyData, KeyElement, link } from '../keyElement.js'; import OSKRow from './oskRow.js'; import VisualKeyboard from '../visualKeyboard.js'; import { ParsedLengthStyle } from '../lengthStyle.js'; import { GesturePreviewHost } from './gesturePreviewHost.js'; - export default class OSKBaseKey extends OSKKey { private capLabel: HTMLDivElement; private previewHost: GesturePreviewHost; @@ -197,29 +196,23 @@ export default class OSKBaseKey extends OSKKey { this.btn.replaceChild(this.preview, oldPreview); } - public refreshLayout(vkbd: VisualKeyboard) { - let key = this.spec as ActiveKey; - this.square.style.width = vkbd.layoutWidth.scaledBy(key.proportionalWidth).styleString; - this.square.style.marginLeft = vkbd.layoutWidth.scaledBy(key.proportionalPad).styleString; - this.btn.style.width = vkbd.usesFixedWidthScaling ? this.square.style.width : '100%'; + public refreshLayout(layoutParams: KeyLayoutParams) { + const keyTextClosure = super.refreshLayout(layoutParams); // key labels in particular. - if(vkbd.usesFixedHeightScaling) { - // Matches its row's height. - this.square.style.height = vkbd.internalHeight.scaledBy(this.row.heightFraction).styleString; - } else { - this.square.style.height = '100%'; // use the full row height - } + return () => { + // part 2: key internals - these do depend on recalculating internal layout. - super.refreshLayout(vkbd); + // Ideally, the rest would be in yet another calculation layer... need to figure out a good design for this. + keyTextClosure(); // we're already in that phase, so go ahead and run it. - const device = vkbd.device; - const resizeLabels = (device.OS == DeviceSpec.OperatingSystem.iOS && - device.formFactor == DeviceSpec.FormFactor.Phone - && landscapeView()); - - // Rescale keycap labels on iPhone (iOS 7) - if(resizeLabels && this.capLabel) { - this.capLabel.style.fontSize = '6px'; + const emFont = layoutParams.baseEmFontSize; + // Rescale keycap labels on small phones + if(emFont.val < 12) { + this.capLabel.style.fontSize = '6px'; + } else { + // The default value set within kmwosk.css. + this.capLabel.style.fontSize = ParsedLengthStyle.forScalar(0.5).styleString; + } } } diff --git a/web/src/engine/osk/src/keyboard-layout/oskKey.ts b/web/src/engine/osk/src/keyboard-layout/oskKey.ts index cbf06eaf735..34ee93e300e 100644 --- a/web/src/engine/osk/src/keyboard-layout/oskKey.ts +++ b/web/src/engine/osk/src/keyboard-layout/oskKey.ts @@ -1,17 +1,20 @@ -import { ActiveKey, ActiveSubKey, ButtonClass, DeviceSpec, LayoutKey } from '@keymanapp/keyboard-processor'; -import { TouchLayout } from '@keymanapp/common-types'; -import TouchLayoutFlick = TouchLayout.TouchLayoutFlick; +import { ActiveKey, ActiveSubKey, ButtonClass, ButtonClasses, DeviceSpec } from '@keymanapp/keyboard-processor'; // At present, we don't use @keymanapp/keyman. Just `keyman`. (Refer to /web/package.json.) -import { getAbsoluteX, getAbsoluteY } from 'keyman/engine/dom-utils'; - -import { getFontSizeStyle } from '../fontSizeUtils.js'; import specialChars from '../specialCharacters.js'; import buttonClassNames from '../buttonClassNames.js'; import { KeyElement } from '../keyElement.js'; import VisualKeyboard from '../visualKeyboard.js'; import { getTextMetrics } from './getTextMetrics.js'; +import { ParsedLengthStyle } from '../lengthStyle.js'; + +export interface KeyLayoutParams { + keyWidth: number; + keyHeight: number; + baseEmFontSize: ParsedLengthStyle; + layoutFontSize: ParsedLengthStyle; +} /** * Replace default key names by special font codes for modifier keys @@ -59,6 +62,9 @@ export default abstract class OSKKey { label: HTMLSpanElement; square: HTMLDivElement; + private _fontSize: ParsedLengthStyle; + private _fontFamily: string; + /** * The layer of the OSK on which the key is displayed. */ @@ -169,47 +175,47 @@ export default abstract class OSKKey { /** * Calculate the font size required for a key cap, scaling to fit longer text - * @param vkbd * @param text - * @param style specification for the desired base font size - * @param override if true, don't use the font spec from the button, just use the passed in spec + * @param layoutParams specification for the key + * @param scale additional scaling to apply for the font-size calculation (used by keytips) * @returns font size as a style string */ - getIdealFontSize(vkbd: VisualKeyboard, text: string, style: {height?: string, fontFamily?: string, fontSize: string}, override?: boolean): string { - let buttonStyle: typeof style & {width?: string} = getComputedStyle(this.btn); - let keyWidth = parseFloat(buttonStyle.width); - let emScale = vkbd.getKeyEmFontSize(); + getIdealFontSize(text: string, layoutParams: KeyLayoutParams, scale?: number): ParsedLengthStyle { + // Fallback in case not all style info is currently ready. + if(!this._fontFamily) { + return new ParsedLengthStyle('1em'); + } + + scale ??= 1; + + const keyWidth = layoutParams.keyWidth; + const keyHeight = layoutParams.keyHeight; + const emScale = layoutParams.baseEmFontSize.scaledBy(layoutParams.layoutFontSize.val); // Among other things, ensures we use SpecialOSK styling for special key text. // It's set on the key-span, not on the button. // // Also helps ensure that the stub's font-family name is used for keys, should // that mismatch the font-family name specified within the keyboard's touch layout. - const capFont = !this.label ? undefined: getComputedStyle(this.label).fontFamily; - if(capFont) { - buttonStyle = { - fontFamily: capFont, - fontSize: buttonStyle.fontSize, - height: buttonStyle.height - } - } - const originalSize = getFontSizeStyle(style.fontSize || '1em'); + let originalSize = this._fontSize; + if(!originalSize.absolute) { + originalSize = emScale.scaledBy(originalSize.val); + } - // Not yet available; it'll be handled in a later layout pass. - if(!override) { - // When available, just use computedStyle instead. - style = buttonStyle; + const style = { + fontFamily: this._fontFamily, + fontSize: originalSize.styleString, + height: layoutParams.keyHeight } - let fontSpec = getFontSizeStyle(style.fontSize || '1em'); - let metrics = getTextMetrics(text, emScale, style); + let metrics = getTextMetrics(text, emScale.scaledBy(scale).val, style); const MAX_X_PROPORTION = 0.90; const MAX_Y_PROPORTION = 0.90; const X_PADDING = 2; - var fontHeight: number, keyHeight: number; + var fontHeight: number; if(metrics.fontBoundingBoxAscent) { fontHeight = metrics.fontBoundingBoxAscent + metrics.fontBoundingBoxDescent; } @@ -217,10 +223,6 @@ export default abstract class OSKKey { // Don't add extra padding to height - multiplying with MAX_Y_PROPORTION already gives // padding let textHeight = fontHeight ?? 0; - if(style.height && style.height.indexOf('px') != -1) { - keyHeight = Number.parseFloat(style.height.substring(0, style.height.indexOf('px'))); - } - let xProportion = (keyWidth * MAX_X_PROPORTION) / (metrics.width + X_PADDING); // How much of the key does the text want to take? let yProportion = textHeight && keyHeight ? (keyHeight * MAX_Y_PROPORTION) / textHeight : undefined; @@ -229,28 +231,10 @@ export default abstract class OSKKey { proportion = yProportion; } - // Never upscale keys past the default - only downscale them. + // Never upscale keys past the default * the specified scale - only downscale them. // Proportion < 1: ratio of key width to (padded [loosely speaking]) text width // maxProportion determines the 'padding' involved. - // - if(proportion < 1) { - if(originalSize.absolute) { - return proportion * fontSpec.val + 'px'; - } else { - return proportion * originalSize.val + 'em'; - } - } else { - if(originalSize.absolute) { - return fontSpec.val + 'px'; - } else { - return originalSize.val + 'em'; - } - } - } - - getKeyWidth(vkbd: VisualKeyboard): number { - let key = this.spec as ActiveKey; - return key.proportionalWidth * vkbd.width; + return ParsedLengthStyle.forScalar(scale * Math.min(proportion, 1)); } public get keyText(): string { @@ -310,47 +294,73 @@ export default abstract class OSKKey { } // Check the key's display width - does the key visualize well? - let emScale = vkbd.getKeyEmFontSize(); - var width: number = getTextMetrics(keyText, emScale, styleSpec).width; - if(width == 0 && keyText != '' && keyText != '\xa0') { - // Add the Unicode 'empty circle' as a base support for needy diacritics. - - // Disabled by mcdurdin 2020-10-19; dotted circle display is inconsistent on iOS/Safari - // at least and doesn't combine with diacritic marks. For consistent display, it may be - // necessary to build a custom font that does not depend on renderer choices for base - // mark display -- e.g. create marks with custom base included, potentially even on PUA - // code points and use those in rendering the OSK. See #3039 for more details. - // keyText = '\u25cc' + keyText; - - if(vkbd.isRTL) { - // Add the RTL marker to ensure it displays properly. - keyText = '\u200f' + keyText; - } + if(vkbd.isRTL) { + // Add the RTL marker to ensure it displays properly. + keyText = '\u200f' + keyText; } - ts.fontSize = this.getIdealFontSize(vkbd, keyText, styleSpec); - // Finalize the key's text. t.innerText = keyText; return t; } - public refreshLayout(vkbd: VisualKeyboard) { + public resetFontPrecalc() { + this._fontFamily = undefined; + this._fontSize = undefined; + this.label.style.fontSize = ''; + } + + public refreshLayout(layoutParams: KeyLayoutParams) { + // Avoid doing any font-size related calculations if there's no text to display. + if(this.spec.sp == ButtonClasses.spacer || this.spec.sp == ButtonClasses.blank) { + return () => {}; + } + + // Attempt to detect static but key-specific style properties if they haven't yet + // been detected. + if(this._fontFamily === undefined) { + const lblStyle = getComputedStyle(this.label); + + // Abort if the element is not currently in the DOM; we can't get any info this way. + if(!lblStyle.fontFamily) { + return () => {}; + } + this._fontFamily = lblStyle.fontFamily; + + // Detect any difference in base (em) font size and that which is computed for the key itself. + const computedFontSize = new ParsedLengthStyle(lblStyle.fontSize); + const layoutFontSize = layoutParams.layoutFontSize; + if(layoutFontSize.absolute) { + // rather than just straight-up taking .layoutFontSize + this._fontSize = computedFontSize; + } else { + const baseEmFontSize = layoutParams.baseEmFontSize; + const baseFontSize = layoutFontSize.scaledBy(baseEmFontSize.val); + const localFontScaling = computedFontSize.val / baseFontSize.val; + this._fontSize = ParsedLengthStyle.forScalar(localFontScaling); + } + } + // space bar may not define the text span! if(this.label) { if(!this.label.classList.contains('kmw-spacebar-caption')) { // Do not use `this.keyText` - it holds *___* codes for special keys, not the actual glyph! const keyCapText = this.label.textContent; - this.label.style.fontSize = this.getIdealFontSize(vkbd, keyCapText, this.btn.style); + const fontSize = this.getIdealFontSize(keyCapText, layoutParams); + return () => { + this.label.style.fontSize = fontSize.styleString; + }; } else { - // Remove any custom setting placed on it before recomputing its inherited style info. - this.label.style.fontSize = ''; - const fontSize = this.getIdealFontSize(vkbd, this.label.textContent, this.btn.style); - - // Since the kmw-spacebar-caption version uses !important, we must specify - // it directly on the element too; otherwise, scaling gets ignored. - this.label.style.setProperty("font-size", fontSize, "important"); + // Spacebar text, on the other hand, is available via this.keyText. + // Using this field helps prevent layout reflow during updates. + const fontSize = this.getIdealFontSize(this.keyText, layoutParams); + + return () => { + // Since the kmw-spacebar-caption version uses !important, we must specify + // it directly on the element too; otherwise, scaling gets ignored. + this.label.style.setProperty("font-size", fontSize.styleString, "important"); + }; } } } diff --git a/web/src/engine/osk/src/keyboard-layout/oskLayer.ts b/web/src/engine/osk/src/keyboard-layout/oskLayer.ts index 9f9486e28e1..ee84679a217 100644 --- a/web/src/engine/osk/src/keyboard-layout/oskLayer.ts +++ b/web/src/engine/osk/src/keyboard-layout/oskLayer.ts @@ -3,7 +3,17 @@ import { ActiveLayer, ActiveLayout } from '@keymanapp/keyboard-processor'; import OSKRow from './oskRow.js'; import OSKBaseKey from './oskBaseKey.js'; import VisualKeyboard from '../visualKeyboard.js'; - +import { ParsedLengthStyle } from '../lengthStyle.js'; + +export interface LayerLayoutParams { + keyboardWidth: number; + keyboardHeight: number; + widthStyle: ParsedLengthStyle; + heightStyle: ParsedLengthStyle; + baseEmFontSize: ParsedLengthStyle; + layoutFontSize: ParsedLengthStyle; + spacebarText: string; +} export default class OSKLayer { public readonly element: HTMLDivElement; public readonly rows: OSKRow[]; @@ -76,6 +86,21 @@ export default class OSKLayer { this.capsKey = this.findKey('K_CAPS'); this.numKey = this.findKey('K_NUMLOCK'); this.scrollKey = this.findKey('K_SCROLL'); + + if(this.spaceBarKey) { + const spacebarLabel = this.spaceBarKey.label; + let tButton = this.spaceBarKey.btn; + + 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') { + spacebarLabel.className = 'kmw-spacebar-caption'; + } + } } /** @@ -96,29 +121,78 @@ export default class OSKLayer { return null; } - public refreshLayout(vkbd: VisualKeyboard, layerHeight: number) { + /** + * Indicate the current language and keyboard on the space bar + **/ + showLanguage(displayName: string) { + if(!this.spaceBarKey) { + return () => {}; + } + + try { + const spacebarLabel = this.spaceBarKey.label; + + // The key can read the text from here during the display update without us + // needing to trigger a reflow by running the closure below early. + this.spaceBarKey.spec.text = displayName; + + return () => { + // It sounds redundant, but this dramatically cuts down on browser DOM processing; + // but sometimes innerText is reported empty when it actually isn't, so set it + // anyway in that case (Safari, iOS 14.4) + if (spacebarLabel.innerText != displayName || displayName == '') { + spacebarLabel.innerText = displayName; + } + } + } + catch (ex) { } + } + + public refreshLayout(layoutParams: LayerLayoutParams) { // Check the heights of each row, in case different layers have different row counts. + const layerHeight = layoutParams.keyboardHeight; const nRows = this.rows.length; const rowHeight = this._rowHeight = Math.floor(layerHeight/(nRows == 0 ? 1 : nRows)); - if(vkbd.usesFixedHeightScaling) { + const usesFixedWidthScaling = layoutParams.widthStyle.absolute; + if(usesFixedWidthScaling) { this.element.style.height=(layerHeight)+'px'; } + const spacebarTextClosure = this.showLanguage(layoutParams.spacebarText); + + // Update row layout properties + const rowClosures: (() => void)[] = []; for(let nRow=0; nRow { + oldRowClosure(); oskRow.element.style.bottom = '1px'; - } + }; } + rowClosures.push(rowClosure); + } - oskRow.refreshLayout(vkbd); + const rowKeyClosures: (() => void)[] = []; + for(const row of this.rows) { + const batchedUpdates = row.refreshKeyLayouts(layoutParams); + rowKeyClosures.push(batchedUpdates); } + + // After row layout properties have been updated, _then_ update key internals. + // Doing this separately like this helps to reduce layout reflow. + spacebarTextClosure(); + rowClosures.forEach((closure) => closure()); + rowKeyClosures.forEach((closure) => closure()); } } diff --git a/web/src/engine/osk/src/keyboard-layout/oskLayerGroup.ts b/web/src/engine/osk/src/keyboard-layout/oskLayerGroup.ts index 044af641fb7..6d7065e6b79 100644 --- a/web/src/engine/osk/src/keyboard-layout/oskLayerGroup.ts +++ b/web/src/engine/osk/src/keyboard-layout/oskLayerGroup.ts @@ -1,19 +1,26 @@ -import { ActiveLayer, type DeviceSpec, Keyboard, LayoutLayer, ActiveLayout } from '@keymanapp/keyboard-processor'; -import { ManagedPromise } from '@keymanapp/web-utils'; +import { ActiveLayer, type DeviceSpec, Keyboard, LayoutLayer, ActiveLayout, ButtonClasses } from '@keymanapp/keyboard-processor'; import { InputSample } from '@keymanapp/gesture-recognizer'; import { KeyElement } from '../keyElement.js'; -import OSKLayer from './oskLayer.js'; +import OSKLayer, { LayerLayoutParams } from './oskLayer.js'; import VisualKeyboard from '../visualKeyboard.js'; -import OSKRow from './oskRow.js'; +import OSKBaseKey from './oskBaseKey.js'; +import { ParsedLengthStyle } from '../lengthStyle.js'; + +const NEAREST_KEY_TOUCH_MARGIN_PERCENT = 0.06; export default class OSKLayerGroup { public readonly element: HTMLDivElement; public readonly layers: {[layerID: string]: OSKLayer} = {}; public readonly spec: ActiveLayout; + // Exist as local copies of the VisualKeyboard values, updated via refreshLayout. + private computedWidth: number; + private computedHeight: number; + private _activeLayerId: string = 'default'; + private _heightPadding: number; public constructor(vkbd: VisualKeyboard, keyboard: Keyboard, formFactor: DeviceSpec.FormFactor) { let layout = keyboard.layout(formFactor); @@ -64,6 +71,14 @@ export default class OSKLayerGroup { } } + public get activeLayer(): OSKLayer { + if(!this.activeLayerId) { + return null; + } + + return this.layers[this.activeLayerId]; + } + public get activeLayerId(): string { return this._activeLayerId; } @@ -107,8 +122,6 @@ export default class OSKLayerGroup { throw new Error(`Layer id ${layerId} could not be found`); } - this.blinkLayer(layer); - return this.nearestKey(coord, layer); } @@ -165,86 +178,127 @@ export default class OSKLayerGroup { } private nearestKey(coord: Omit, 'item'>, layer: OSKLayer): KeyElement { - const baseRect = this.element.getBoundingClientRect(); - - let row: OSKRow = null; - let bestMatchDistance = Number.MAX_VALUE; + // If there are no rows, there are no keys; return instantly. + if(layer.rows.length == 0) { + return null; + } - // Find the row that the touch-coordinate lies within. - for(const r of layer.rows) { - const rowRect = r.element.getBoundingClientRect(); - if(rowRect.top <= coord.clientY && coord.clientY < rowRect.bottom) { - row = r; - break; - } else { - const distance = rowRect.top > coord.clientY ? rowRect.top - coord.clientY : coord.clientY - rowRect.bottom; + // Our pre-processed layout info maps whatever shape the keyboard is in into a unit square. + // So, we map our coord to find its location within that square. + const proportionalCoords = { + x: coord.targetX / this.computedWidth, + y: coord.targetY / this.computedHeight + }; - if(distance < bestMatchDistance) { - bestMatchDistance = distance; - row = r; - } - } + // If our computed width and/or height are 0, it's best to abort; key distance + // calculations are not viable. + if(!isFinite(proportionalCoords.x) || !isFinite(proportionalCoords.y)) { + return null; } - // Assertion: row no longer `null`. + // Step 1: find the nearest row. + // Rows aren't variable-height - this value is "one size fits all." - // Warning: am not 100% sure that what follows is actually fully correct. + /* + If 4 rows, y = .2 x 4 = .8 - still within the row with index 0 (spanning from 0 to .25) + y = .6 x 4 = 2.4 - within row with index 2 (third row, spanning .5 to .75) - // Find minimum distance from any key - let closestKeyIndex = 0; - let dx: number; - let dxMax = 24; - let dxMin = 100000; + 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.max(0, Math.min(layer.rows.length-1, Math.floor(proportionalCoords.y * layer.rows.length))); + const row = layer.rows[rowIndex]; - const x = coord.clientX; + // Assertion: row no longer `null`. + // (We already prevented the no-rows available scenario, anyway.) - for (let k = 0; k < row.keys.length; k++) { - // Second-biggest, though documentation suggests this is probably right. - const keySquare = row.keys[k].square as HTMLElement; // gets the .kmw-key-square containing a key - const squareRect = keySquare.getBoundingClientRect(); + // Step 2: Find minimum distance from any key + // - If the coord is within a key's square, go ahead and return it. + let closestKey: OSKBaseKey = null; + // Is percentage-based! + let minDistance = Number.MAX_VALUE; - // Find the actual key element. - let childNode = keySquare.firstChild ? keySquare.firstChild as HTMLElement : keySquare; - - if (childNode.className !== undefined - && (childNode.className.indexOf('key-hidden') >= 0 - || childNode.className.indexOf('key-blank') >= 0)) { + for (let key of row.keys) { + const keySpec = key.spec; + if(keySpec.sp == ButtonClasses.blank || keySpec.sp == ButtonClasses.spacer) { continue; } - const x1 = squareRect.left; - const x2 = squareRect.right; - if (x >= x1 && x <= x2) { - // Within the key square - return childNode; - } - dx = x1 - x; - if (dx >= 0 && dx < dxMin) { - // To right of key - closestKeyIndex = k; dxMin = dx; - } - dx = x - x2; - if (dx >= 0 && dx < dxMin) { - // To left of key - closestKeyIndex = k; dxMin = dx; + + // Max distance from the key's center to consider, horizontally. + const keyRadius = keySpec.proportionalWidth / 2; + const distanceFromCenter = Math.abs(proportionalCoords.x - keySpec.proportionalX); + + // Find the actual key element. + if(distanceFromCenter - keyRadius <= 0) { + // As noted above: if we land within a key's square, match instantly! + return key.btn; + } else { + const distance = distanceFromCenter - keyRadius; + if(distance < minDistance) { + minDistance = distance; + closestKey = key; + } } } - if (dxMin < 100000) { - const t = row.keys[closestKeyIndex].square; - const squareRect = t.getBoundingClientRect(); + /* + Step 3: If the input coordinate wasn't within any valid key's "square", + determine if the nearest valid key is acceptable - if it's within 60% of + a standard key's width from the touch location. - const x1 = squareRect.left; - const x2 = squareRect.right; + If the condition is not met, there are no valid keys within this row. + */ + if (minDistance /* %age-based! */ <= NEAREST_KEY_TOUCH_MARGIN_PERCENT) { + return closestKey.btn; + } - // Limit extended touch area to the larger of 0.6 of key width and 24 px - if (squareRect.width > 40) { - dxMax = 0.6 * squareRect.width; - } + // Step 4: no matches => return null. The caller should be able to handle such cases, + // anyway. + return null; + } - if (((x1 - x) >= 0 && (x1 - x) < dxMax) || ((x - x2) >= 0 && (x - x2) < dxMax)) { - return t.firstChild; + public resetPrecalcFontSizes() { + for(const layer of Object.values(this.layers)) { + for(const row of layer.rows) { + for(const key of row.keys) { + key.resetFontPrecalc(); + } } } - return null; + + // This method is called whenever all related stylesheets are fully loaded and applied. + // The actual padding data may not have been available until now. + this._heightPadding = undefined; + } + + public refreshLayout(layoutParams: LayerLayoutParams) { + // Set layer-group copies of relevant computed-size values; they are used by nearest-key + // detection. + this.computedWidth = layoutParams.keyboardWidth; + this.computedHeight = layoutParams.keyboardHeight; + + // Assumption: this styling value will not change once the keyboard and + // related stylesheets are loaded and applied. + if(this._heightPadding === undefined) { + // Should not trigger a new layout reflow; VisualKeyboard should have made no further DOM + // style changes since the last one. + + // For touch-based OSK layouts, kmwosk.css may include top & bottom + // padding on the layer-group element. + const computedGroupStyle = getComputedStyle(this.element); + + // parseInt('') => NaN, which is falsy; we want to fallback to zero. + let pt = parseInt(computedGroupStyle.paddingTop, 10) || 0; + let pb = parseInt(computedGroupStyle.paddingBottom, 10) || 0; + this._heightPadding = pt + pb; + } + + if(this.activeLayer) { + this.activeLayer.refreshLayout(layoutParams); + } + } + + public get verticalPadding() { + return this._heightPadding ?? 0; } } \ No newline at end of file diff --git a/web/src/engine/osk/src/keyboard-layout/oskRow.ts b/web/src/engine/osk/src/keyboard-layout/oskRow.ts index 75494f5921c..777b8e76583 100644 --- a/web/src/engine/osk/src/keyboard-layout/oskRow.ts +++ b/web/src/engine/osk/src/keyboard-layout/oskRow.ts @@ -3,6 +3,15 @@ import { ActiveKey, ActiveLayer, ActiveRow } from '@keymanapp/keyboard-processor import OSKBaseKey from './oskBaseKey.js'; import { ParsedLengthStyle } from '../lengthStyle.js'; import VisualKeyboard from '../visualKeyboard.js'; +import { KeyLayoutParams } from './oskKey.js'; +import { LayerLayoutParams } from './oskLayer.js'; + +/* + The total proportion of key-square height used as key-button padding. + The 'padding' is visible to users as the vertical space between keys + and exists both in "fixed" and "absolute" sizing modes. +*/ +export const KEY_BTN_Y_PAD_RATIO = 0.15; /** * Models one row of one layer of the OSK (`VisualKeyboard`) for a keyboard. @@ -11,6 +20,7 @@ export default class OSKRow { public readonly element: HTMLDivElement; public readonly keys: OSKBaseKey[]; public readonly heightFraction: number; + public readonly spec: ActiveRow; public constructor(vkbd: VisualKeyboard, layerSpec: ActiveLayer, @@ -23,6 +33,7 @@ export default class OSKRow { // Apply defaults, setting the width and other undefined properties for each key const keys=rowSpec.key; + this.spec = rowSpec; this.keys = []; // Calculate actual key widths by multiplying by the OSK's width and rounding appropriately, @@ -55,34 +66,78 @@ export default class OSKRow { } } - public refreshLayout(vkbd: VisualKeyboard) { + public refreshLayout(layoutParams: LayerLayoutParams) { const rs = this.element.style; - const rowHeight = vkbd.internalHeight.scaledBy(this.heightFraction); - rs.maxHeight=rs.lineHeight=rs.height=rowHeight.styleString; + const rowHeight = layoutParams.heightStyle.scaledBy(this.heightFraction); + const executeRowStyleUpdates = () => { + rs.maxHeight=rs.lineHeight=rs.height=rowHeight.styleString; + } + + const keyHeightBase = layoutParams.heightStyle.absolute ? rowHeight : ParsedLengthStyle.forScalar(1); + const padTop = keyHeightBase.scaledBy(KEY_BTN_Y_PAD_RATIO / 2); + const keyHeight = keyHeightBase.scaledBy(1 - KEY_BTN_Y_PAD_RATIO); - // Only used for fixed-height scales at present. - const padRatio = 0.15; + // Update all key-square layouts. + const keyStyleUpdates = this.keys.map((key) => { + return () => { + const keySquare = key.square; + const keyElement = key.btn; - const keyHeightBase = vkbd.usesFixedHeightScaling ? rowHeight : ParsedLengthStyle.forScalar(1); - const padTop = keyHeightBase.scaledBy(padRatio / 2); - const keyHeight = keyHeightBase.scaledBy(1 - padRatio); + // Set the kmw-key-square position + const kss = keySquare.style; + kss.height=kss.minHeight=keyHeightBase.styleString; - for(const key of this.keys) { - const keySquare = key.btn.parentElement; + const kes = keyElement.style; + kes.top = padTop.styleString; + kes.height=kes.lineHeight=kes.minHeight=keyHeight.styleString; + } + }) + + return () => { + executeRowStyleUpdates(); + keyStyleUpdates.forEach((closure) => closure()); + } + } + + public refreshKeyLayouts(layoutParams: LayerLayoutParams) { + const updateClosures = this.keys.map((key) => { + // Calculate changes to be made... const keyElement = key.btn; - // Set the kmw-key-square position - const kss = keySquare.style; - kss.height=kss.minHeight=keyHeightBase.styleString; + const widthStyle = layoutParams.widthStyle; + const heightStyle = layoutParams.heightStyle; + + const keyWidth = widthStyle.scaledBy(key.spec.proportionalWidth); + const keyPad = widthStyle.scaledBy(key.spec.proportionalPad); + + // We maintain key-btn padding within the key-square - the latter `scaledBy` + // adjusts for that, providing the final key-btn height. + const keyHeight = heightStyle.scaledBy(this.heightFraction).scaledBy(1 - KEY_BTN_Y_PAD_RATIO); + + // Match the row height (if fixed-height) or use full row height (if percent-based) + const styleHeight = heightStyle.absolute ? keyHeight.styleString : '100%'; - const kes = keyElement.style; - kes.top = padTop.styleString; - kes.height=kes.lineHeight=kes.minHeight=keyHeight.styleString; + const keyStyle: KeyLayoutParams = { + keyWidth: keyWidth.val * (keyWidth.absolute ? 1 : layoutParams.keyboardWidth), + keyHeight: keyHeight.val * (heightStyle.absolute ? 1 : layoutParams.keyboardHeight), + baseEmFontSize: layoutParams.baseEmFontSize, + layoutFontSize: layoutParams.layoutFontSize + }; + //return keyElement.key ? keyElement.key.refreshLayout(keyStyle) : () => {}; + const keyFontClosure = keyElement.key ? keyElement.key.refreshLayout(keyStyle) : () => {}; - if(keyElement.key) { - keyElement.key.refreshLayout(vkbd); + // And queue them to be run in a single batch later. This helps us avoid layout reflow thrashing. + return () => { + key.square.style.width = keyWidth.styleString; + key.square.style.marginLeft = keyPad.styleString; + + key.btn.style.width = widthStyle.absolute ? keyWidth.styleString : '100%'; + key.square.style.height = styleHeight; + keyFontClosure(); } - } + }); + + return () => updateClosures.forEach((closure) => closure()); } } \ No newline at end of file diff --git a/web/src/engine/osk/src/views/floatingOskView.ts b/web/src/engine/osk/src/views/floatingOskView.ts index 2399eefe258..7748d463b83 100644 --- a/web/src/engine/osk/src/views/floatingOskView.ts +++ b/web/src/engine/osk/src/views/floatingOskView.ts @@ -64,6 +64,7 @@ export default class FloatingOSKView extends OSKView { this.resizeBar.on('showbuild', () => this.emit('showbuild')); this.headerView = this.titleBar; + this._Box.insertBefore(this.headerView.element, this._Box.firstChild); const onListenedEvent = (eventName: keyof EventMap | keyof LegacyOSKEventMap) => { // As the following title bar buttons (for desktop / FloatingOSKView) do nothing unless a site diff --git a/web/src/engine/osk/src/views/oskView.ts b/web/src/engine/osk/src/views/oskView.ts index 4f0d6c75044..984ba63d856 100644 --- a/web/src/engine/osk/src/views/oskView.ts +++ b/web/src/engine/osk/src/views/oskView.ts @@ -159,6 +159,7 @@ export default abstract class OSKView private uiStyleSheetManager: StylesheetManager; private config: Configuration; + private deferLayout: boolean; private _boxBaseMouseDown: (e: MouseEvent) => boolean; private _boxBaseTouchStart: (e: TouchEvent) => boolean; @@ -233,10 +234,22 @@ export default abstract class OSKView // Initializes the two constant OSKComponentView fields. this.bannerView = new BannerView(); this.bannerView.events.on('bannerchange', () => this.refreshLayout()); + this._Box.appendChild(this.bannerView.element); this._bannerController = new BannerController(this.bannerView, this.hostDevice, this.config.predictionContextManager); - this.keyboardView = null; + this.keyboardView = this._GenerateKeyboardView(null, null); + this._Box.appendChild(this.keyboardView.element); + + // Install the default OSK stylesheets - but don't have it managed by the keyboard-specific stylesheet manager. + // We wish to maintain kmwosk.css whenever keyboard-specific styles are reset/removed. + // Temp-hack: embedded products prefer their stylesheet, etc linkages without the /osk path component. + const resourcePath = getResourcePath(this.config); + + for(let sheetFile of OSKView.STYLESHEET_FILES) { + const sheetHref = `${resourcePath}${sheetFile}`; + this.uiStyleSheetManager.linkExternalSheet(sheetHref); + } this.setBaseMouseEventListeners(); if(this.hostDevice.touchable) { @@ -583,8 +596,33 @@ export default abstract class OSKView this.needsLayout = true; } + public batchLayoutAfter(closure: () => void) { + /* + Is there already an ongoing batch? If so, just run the closure and don't + adjust the tracking variables. The outermost call will finalize layout. + */ + if(this.deferLayout) { + closure(); + return; + } + + try { + this.deferLayout = true; + if(this.vkbd) { + this.vkbd.deferLayout = true; + } + closure(); + } finally { + this.deferLayout = false; + if(this.vkbd) { + this.vkbd.deferLayout = false; + } + this.refreshLayout(); + } + } + public refreshLayout(pending?: boolean): void { - if(!this.keyboardView) { + if(!this.keyboardView || this.deferLayout) { return; } @@ -646,6 +684,7 @@ export default abstract class OSKView if(this.bannerView.height > 0) { availableHeight -= this.bannerView.height + 5; } + // Triggers the VisualKeyboard.refreshLayout() method, which includes a showLanguage() call. this.vkbd.setSize(this.computedWidth, availableHeight, pending); const bs = this._Box.style; @@ -653,9 +692,6 @@ export default abstract class OSKView // visualizations, not to help text or empty views. bs.width = bs.maxWidth = this.computedWidth + 'px'; bs.height = bs.maxHeight = this.computedHeight + 'px'; - - // Ensure that the layer's spacebar is properly captioned. - this.vkbd.showLanguage(); } else { const bs = this._Box.style; bs.width = 'auto'; @@ -700,52 +736,28 @@ export default abstract class OSKView private loadActiveKeyboard() { this.setBoxStyling(); - - // Do not erase / 'shutdown' the banner-controller; we simply re-use its elements. - if(this.vkbd) { - this.vkbd.shutdown(); - } - this.keyboardView = null; this.needsLayout = true; + // Save references to the old kbd & its styles for shutdown after replacement. + const oldKbd = this.keyboardView; + const oldKbdStyleManager = this.kbdStyleSheetManager; - // Instantly resets the OSK container, erasing / delinking the previously-loaded keyboard. - this._Box.innerHTML = ''; - - // Since we cleared all inner HTML, that means we cleared the stylesheets, too. - this.uiStyleSheetManager.unlinkAll(); - this.kbdStyleSheetManager.unlinkAll(); - - // Install the default OSK stylesheets - but don't have it managed by the keyboard-specific stylesheet manager. - // We wish to maintain kmwosk.css whenever keyboard-specific styles are reset/removed. - // Temp-hack: embedded products prefer their stylesheet, etc linkages without the /osk path component. - const resourcePath = getResourcePath(this.config); - - for(let sheetFile of OSKView.STYLESHEET_FILES) { - const sheetHref = `${resourcePath}${sheetFile}`; - this.uiStyleSheetManager.linkExternalSheet(sheetHref); - } - - // Any event-cancelers would go here, after the innerHTML reset. - - // Add header element to OSK only for desktop browsers - if(this.headerView) { - this._Box.appendChild(this.headerView.element); - } - - // Add suggestion banner bar to OSK - this._Box.appendChild(this.banner.element); - - this.bannerController?.configureForKeyboard(this.keyboardData?.keyboard, this.keyboardData?.metadata); + // Create new ones for the new, incoming kbd. + this.kbdStyleSheetManager = new StylesheetManager(this._Box, this.config.doCacheBusting || false); + const kbdView = this.keyboardView = this._GenerateKeyboardView(this.keyboardData?.keyboard, this.keyboardData?.metadata); - let kbdView: KeyboardView = this.keyboardView = this._GenerateKeyboardView(this.keyboardData?.keyboard, this.keyboardData?.metadata); - this._Box.appendChild(kbdView.element); + // Perform the replacement. + this._Box.replaceChild(kbdView.element, oldKbd.element); kbdView.postInsert(); + this.bannerController?.configureForKeyboard(this.keyboardData?.keyboard, this.keyboardData?.metadata); - // Add footer element to OSK only for desktop browsers - if(this.footerView) { - this._Box.appendChild(this.footerView.element); + // Now that the swap has occurred, it's safe to shutdown the old VisualKeyboard and any related stylesheets. + if(oldKbd instanceof VisualKeyboard) { + oldKbd.shutdown(); } + oldKbdStyleManager.unlinkAll(); + // END: construction of the actual internal layout for the overall OSK + // Footer element management is handled within FloatingOSKView. this.banner.appendStyles(); @@ -770,10 +782,6 @@ export default abstract class OSKView private _GenerateKeyboardView(keyboard: Keyboard, keyboardMetadata: KeyboardProperties): KeyboardView { let device = this.targetDevice; - if(this.vkbd) { - this.vkbd.shutdown(); - } - this._Box.className = ""; // Case 1: since we hide the system keyboard on touch devices, we need @@ -871,16 +879,9 @@ export default abstract class OSKView // Also, only change the layer ID itself if there is an actual corresponding layer // in the OSK. if(this.vkbd?.layerGroup.layers[newValue] && !this.vkbd?.layerLocked) { + // triggers state-update + layer refresh automatically. this.vkbd.layerId = newValue; - // Ensure that the layer's spacebar is properly captioned. - this.vkbd.showLanguage(); } - - // Ensure the keyboard view is modeling the correct state. (Correct layer, etc.) - this.keyboardView.updateState(); // will also need the stateKeys. - // We need to recalc the font size here because the layer did not have - // calculated dimensions available before it was visible - this.refreshLayout(); } return false; @@ -908,10 +909,6 @@ export default abstract class OSKView // First thing after it's made visible. this.refreshLayoutIfNeeded(); - if(this.keyboardView instanceof VisualKeyboard) { - this.keyboardView.showLanguage(); - } - this._Visible=true; /* In case it's still '0' from a hide() operation. diff --git a/web/src/engine/osk/src/visualKeyboard.ts b/web/src/engine/osk/src/visualKeyboard.ts index 95a15ae4d73..67ae9f48fc0 100644 --- a/web/src/engine/osk/src/visualKeyboard.ts +++ b/web/src/engine/osk/src/visualKeyboard.ts @@ -40,11 +40,11 @@ import KeyboardView from './components/keyboardView.interface.js'; import { type KeyElement, getKeyFrom } from './keyElement.js'; import KeyTip from './keytip.interface.js'; import OSKKey from './keyboard-layout/oskKey.js'; -import OSKLayer from './keyboard-layout/oskLayer.js'; +import OSKLayer, { LayerLayoutParams } from './keyboard-layout/oskLayer.js'; import OSKLayerGroup from './keyboard-layout/oskLayerGroup.js'; import OSKView from './views/oskView.js'; import { LengthStyle, ParsedLengthStyle } from './lengthStyle.js'; -import { defaultFontSize, getFontSizeStyle } from './fontSizeUtils.js'; +import { defaultFontSize } from './fontSizeUtils.js'; import PhoneKeyTip from './input/gestures/browser/keytip.js'; import { TabletKeyTip } from './input/gestures/browser/tabletPreview.js'; import CommonConfiguration from './config/commonConfiguration.js'; @@ -158,7 +158,6 @@ export default class VisualKeyboard extends EventEmitter implements Ke readonly config: VisualKeyboardConfiguration; - private _layerId: string = "default"; layerLocked: boolean = false; layerIndex: number = 0; // the index of the default layer readonly isRTL: boolean; @@ -183,6 +182,13 @@ export default class VisualKeyboard extends EventEmitter implements Ke */ private _height: number; + /** + * The main VisualKeyboard element's border-width styling. + * + * Assumption: is a fixed, uniform length that doesn't vary between refreshLayout() calls. + */ + private _borderWidth: number = 0; + /** * The computed width for this VisualKeyboard. May be null if auto sizing * is allowed and the VisualKeyboard is not currently in the DOM hierarchy. @@ -225,21 +231,29 @@ export default class VisualKeyboard extends EventEmitter implements Ke activeGestures: GestureHandler[] = []; activeModipress: Modipress = null; + private _deferLayout: boolean; // The keyboard object corresponding to this VisualKeyboard. public readonly layoutKeyboard: Keyboard; public readonly layoutKeyboardProperties: KeyboardProperties; + get deferLayout(): boolean { + return this._deferLayout; + } + + set deferLayout(value: boolean) { + this._deferLayout = value; + } + get layerId(): string { - return this._layerId; + return this.layerGroup?.activeLayerId ?? 'default'; } set layerId(value: string) { - const changedLayer = value != this._layerId; + const changedLayer = value != this.layerId; if(!this.layerGroup.layers[value]) { throw new Error(`Keyboard ${this.layoutKeyboard.id} does not have a layer with id ${value}`); } else { - this._layerId = value; this.layerGroup.activeLayerId = value; // Does not exist for documentation keyboards! @@ -248,14 +262,15 @@ export default class VisualKeyboard extends EventEmitter implements Ke } } - if(changedLayer) { + if(changedLayer && !this._deferLayout) { this.updateState(); - this.refreshLayout(); + // We changed the active layer, but not any layout property of the keyboard as a whole. + this.layerGroup.refreshLayout(this.constructLayoutParams()); } } get currentLayer(): OSKLayer { - return this.layerId ? this.layerGroup?.layers[this.layerId] : null; + return this.layerGroup?.activeLayer; } // Special keys (for the currently-visible layer) @@ -484,6 +499,7 @@ export default class VisualKeyboard extends EventEmitter implements Ke this.highlightKey(oldKey, false); this.gesturePreviewHost?.cancel(); this.gesturePreviewHost = null; + trackingEntry.previewHost = null; const previewHost = this.highlightKey(key, true); if(previewHost) { @@ -531,6 +547,13 @@ export default class VisualKeyboard extends EventEmitter implements Ke return sourceTrackingMap[id]?.previewHost; }).find((obj) => !!obj); + const clearPreviewHost = () => { + if(existingPreviewHost) { + existingPreviewHost.cancel(); + this.gesturePreviewHost = null; + } + } + let handlers: GestureHandler[] = gestureHandlerMap.get(gestureSequence); if(!handlers && existingPreviewHost && !gestureStage.matchedId.includes('flick')) { existingPreviewHost.clearFlick(); @@ -625,7 +648,7 @@ export default class VisualKeyboard extends EventEmitter implements Ke if(gestureStage.matchedId == 'special-key-start') { if(gestureKey.key.spec.baseKeyID == 'K_BKSP') { // There shouldn't be a preview host for special keys... but it doesn't hurt to add the check. - existingPreviewHost?.cancel(); + clearPreviewHost(); // Possible enhancement: maybe update the held location for the backspace if there's movement? // But... that seems pretty low-priority. @@ -647,7 +670,7 @@ export default class VisualKeyboard extends EventEmitter implements Ke gestureKey.key.highlight(true); } } else if(gestureStage.matchedId.indexOf('longpress') > -1) { - existingPreviewHost?.cancel(); + clearPreviewHost(); // Matches: 'longpress', 'longpress-reset'. // Likewise. @@ -666,8 +689,7 @@ export default class VisualKeyboard extends EventEmitter implements Ke trackingEntry.previewHost = null; gestureSequence.on('complete', () => { - existingPreviewHost?.cancel(); - this.gesturePreviewHost = null; + clearPreviewHost(); }) // Past that, mere construction of the class for delegation is enough. @@ -683,7 +705,7 @@ export default class VisualKeyboard extends EventEmitter implements Ke )]; } else if(gestureStage.matchedId.includes('modipress') && gestureStage.matchedId.includes('-start')) { // There shouldn't be a preview host for modipress keys... but it doesn't hurt to add the check. - existingPreviewHost?.cancel(); + clearPreviewHost(); if(this.layerLocked) { console.warn("Unexpected state: modipress start attempt during an active modipress"); @@ -703,7 +725,7 @@ export default class VisualKeyboard extends EventEmitter implements Ke } } else { // Probably an initial-tap or a simple-tap. - existingPreviewHost?.cancel(); + clearPreviewHost(); } if(handlers) { @@ -776,11 +798,7 @@ export default class VisualKeyboard extends EventEmitter implements Ke get layoutWidth(): ParsedLengthStyle { if (this.usesFixedWidthScaling) { let baseWidth = this.width; - let cs = getComputedStyle(this.element); - if (cs.border) { - let borderWidth = new ParsedLengthStyle(cs.borderWidth).val; - baseWidth -= borderWidth * 2; - } + baseWidth -= this._borderWidth * 2; return ParsedLengthStyle.inPixels(baseWidth); } else { return ParsedLengthStyle.forScalar(1); @@ -790,11 +808,7 @@ export default class VisualKeyboard extends EventEmitter implements Ke get layoutHeight(): ParsedLengthStyle { if (this.usesFixedHeightScaling) { let baseHeight = this.height; - let cs = getComputedStyle(this.element); - if (cs.border) { - let borderHeight = new ParsedLengthStyle(cs.borderWidth).val; - baseHeight -= borderHeight * 2; - } + baseHeight -= this._borderWidth * 2; return ParsedLengthStyle.inPixels(baseHeight); } else { return ParsedLengthStyle.forScalar(1); @@ -804,7 +818,9 @@ export default class VisualKeyboard extends EventEmitter implements Ke get internalHeight(): ParsedLengthStyle { if (this.usesFixedHeightScaling) { // Touch OSKs may apply internal padding to prevent row cropping at the edges. - return ParsedLengthStyle.inPixels(this.layoutHeight.val - this.getVerticalLayerGroupPadding()); + // ... why not precompute both, rather than recalculate each time? + // - appears to contribute to layout reflow costs on layer swaps! + return ParsedLengthStyle.inPixels(this.layoutHeight.val - this._borderWidth * 2 - this.layerGroup.verticalPadding); } else { return ParsedLengthStyle.forScalar(1); } @@ -1108,38 +1124,6 @@ export default class VisualKeyboard extends EventEmitter implements Ke //#endregion - /** - * Indicate the current language and keyboard on the space bar - **/ - showLanguage() { - let activeStub = this.layoutKeyboardProperties; - let displayName: string = activeStub?.displayName ?? '(System keyboard)'; - - try { - var t = this.spaceBar.key.label; - let tParent = t.parentNode; - if (typeof (tParent.className) == 'undefined' || tParent.className == '') { - tParent.className = 'kmw-spacebar'; - } else if (tParent.className.indexOf('kmw-spacebar') == -1) { - tParent.className += ' kmw-spacebar'; - } - - if (t.className != 'kmw-spacebar-caption') { - t.className = 'kmw-spacebar-caption'; - } - - // It sounds redundant, but this dramatically cuts down on browser DOM processing; - // but sometimes innerText is reported empty when it actually isn't, so set it - // anyway in that case (Safari, iOS 14.4) - if (t.innerText != displayName || displayName == '') { - t.innerText = displayName; - } - - this.spaceBar.key.refreshLayout(this); - } - catch (ex) { } - } - /** * Add or remove a class from a keyboard key (when touched or clicked) * or add a key preview for phone devices @@ -1179,28 +1163,28 @@ export default class VisualKeyboard extends EventEmitter implements Ke * Use of `getComputedStyle` is ideal, but in many of our use cases its preconditions are not met. * This function allows us to calculate the font size in those situations. */ - getKeyEmFontSize(): number { + getKeyEmFontSize(): ParsedLengthStyle { if (!this.fontSize) { - return 0; + return new ParsedLengthStyle('0px'); } if (this.device.formFactor == 'desktop') { let keySquareScale = 0.8; // Set in kmwosk.css, is relative. - return this.fontSize.scaledBy(keySquareScale).val; + return this.fontSize.scaledBy(keySquareScale); } else { - let emSizeStr = getComputedStyle(document.body).fontSize; - let emSize = getFontSizeStyle(emSizeStr).val; + const emSizeStr = getComputedStyle(document.body).fontSize; + const emSize = new ParsedLengthStyle(emSizeStr); - var emScale = 1; + let emScale = 1; if (!this.isStatic) { // Double-check against the font scaling applied to the _Box element. if (this.fontSize.absolute) { - return this.fontSize.val; + return this.fontSize; } else { emScale = this.fontSize.val; } } - return emSize * emScale; + return emSize.scaledBy(emScale); } } @@ -1212,7 +1196,6 @@ export default class VisualKeyboard extends EventEmitter implements Ke return; } - var n, b = this.kbdDiv.childNodes[0].childNodes; this.nextLayer = this.layerId; if (this.currentLayer.nextlayer) { @@ -1232,7 +1215,15 @@ export default class VisualKeyboard extends EventEmitter implements Ke * when needed. */ refreshLayout() { - //let keyman = com.keyman.singleton; + if(this._deferLayout) { + return; + } + + /* + Phase 1: calculations possible at the start without triggering _any_ additional layout reflow. + (A single, initial reflow may happen depending on DOM manipulations before this method..., + but no extras until phase 2.) + */ let device = this.device; var fs = 1.0; @@ -1241,11 +1232,9 @@ export default class VisualKeyboard extends EventEmitter implements Ke fs = fs / getViewportScale(this.device.formFactor); } - let paddedHeight: number; - if (this.height) { - paddedHeight = this.computedAdjustedOskHeight(this.height); - } - + /* + Phase 2: first self-triggered reflow - locking in the keyboard's base property styling. + */ let gs = this.kbdDiv.style; if (this.usesFixedHeightScaling && this.height) { // Sets the layer group to the correct height. @@ -1256,13 +1245,20 @@ export default class VisualKeyboard extends EventEmitter implements Ke // Layer-group font-scaling is applied separately. gs.fontSize = this.fontSize.scaledBy(fs).styleString; + // Phase 3: reflow from top-level getComputedStyle calls + // Step 1: have the necessary conditions been met? const fixedSize = this.width && this.height; const computedStyle = getComputedStyle(this.kbdDiv); + const groupStyle = getComputedStyle(this.kbdDiv.firstElementChild); + const isInDOM = computedStyle.height != '' && computedStyle.height != 'auto'; + if (computedStyle.border) { + this._borderWidth = new ParsedLengthStyle(computedStyle.borderWidth).val; + } + // Step 2: determine basic layout geometry, refresh things that might update. - this.showLanguage(); // In case the spacebar-text mode setting has changed. if (fixedSize) { this._computedWidth = this.width; @@ -1270,9 +1266,6 @@ export default class VisualKeyboard extends EventEmitter implements Ke } else if (isInDOM) { this._computedWidth = parseInt(computedStyle.width, 10); if (!this._computedWidth) { - // For touch keyboards, the width _was_ specified on the layer group, - // not the root element (`kbdDiv`). - const groupStyle = getComputedStyle(this.kbdDiv.firstElementChild); this._computedWidth = parseInt(groupStyle.width, 10); } this._computedHeight = parseInt(computedStyle.height, 10); @@ -1293,23 +1286,23 @@ export default class VisualKeyboard extends EventEmitter implements Ke this.gestureParams.flick.triggerDist = 0.75 * this.currentLayer.rowHeight; } - // Step 4: perform layout operations. - // Needs the refreshed layout info to work correctly. - if(this.currentLayer) { - this.currentLayer.refreshLayout(this, this._computedHeight - this.getVerticalLayerGroupPadding()); - } + // Phase 4: Refresh the layout of the layer-group and active layer. + this.layerGroup.refreshLayout(this.constructLayoutParams()); } - private getVerticalLayerGroupPadding(): number { - // For touch-based OSK layouts, kmwosk.css may include top & bottom padding on the layer-group element. - const computedGroupStyle = getComputedStyle(this.layerGroup.element); - - // parseInt('') => NaN, which is falsy; we want to fallback to zero. - let pt = parseInt(computedGroupStyle.paddingTop, 10) || 0; - let pb = parseInt(computedGroupStyle.paddingBottom, 10) || 0; - return pt + pb; + private constructLayoutParams(): LayerLayoutParams { + return { + keyboardWidth: this._computedWidth - 2 * this._borderWidth, + keyboardHeight: this._computedHeight - 2 * this._borderWidth - this.layerGroup.verticalPadding, + widthStyle: this.layoutWidth, + heightStyle: this.internalHeight, + baseEmFontSize: this.getKeyEmFontSize(), + layoutFontSize: new ParsedLengthStyle(this.layerGroup.element.style.fontSize), + spacebarText: this.layoutKeyboardProperties?.displayName ?? '(System keyboard)' + }; } + // Appears to be abandoned now - candidate for removal in future. /*private*/ computedAdjustedOskHeight(allottedHeight: number): number { if (!this.layerGroup) { return allottedHeight; @@ -1382,7 +1375,12 @@ export default class VisualKeyboard extends EventEmitter implements Ke } // Once any related fonts are loaded, we can re-adjust key-cap scaling. - this.styleSheetManager.allLoadedPromise().then(() => this.refreshLayout()); + this.styleSheetManager.allLoadedPromise().then(() => { + // All existing font-precalculations will need to be reset, as the font + // was previously unavailable. + this.layerGroup.resetPrecalcFontSizes(); + this.refreshLayout() + }); } /** @@ -1619,10 +1617,10 @@ export default class VisualKeyboard extends EventEmitter implements Ke showGesturePreview(key: KeyElement) { const tip = this.keytip; - const keyCS = getComputedStyle(key); - const parsedHeight = Number.parseInt(keyCS.height, 10); - const parsedWidth = Number.parseInt(keyCS.width, 10); - const previewHost = new GesturePreviewHost(key, this.device.formFactor == 'phone', parsedWidth, parsedHeight); + const layoutParams = this.constructLayoutParams(); + const keyWidth = layoutParams.keyboardWidth * key.key.spec.proportionalWidth; + const keyHeight = layoutParams.keyboardHeight / this.currentLayer.rows.length; + const previewHost = new GesturePreviewHost(key, this.device.formFactor == 'phone', keyWidth, keyHeight); if (tip == null) { const baseKey = key.key as OSKBaseKey; diff --git a/web/src/tools/testing/recorder/browserDriver.ts b/web/src/tools/testing/recorder/browserDriver.ts index 8467681fde1..e13d17a7827 100644 --- a/web/src/tools/testing/recorder/browserDriver.ts +++ b/web/src/tools/testing/recorder/browserDriver.ts @@ -100,8 +100,10 @@ export class BrowserDriver { downEvent = new Event(BrowserDriver.oskDownTouchType); upEvent = new Event(BrowserDriver.oskUpTouchType); downEvent['touches'] = asTouchList([{"target": oskKeyElement, ...center}]); - upEvent['touches'] = asTouchList([{"target": oskKeyElement, ...center}]); + // The touch should NOT show up in event.touches when a touch ends. + upEvent['touches'] = asTouchList([]); downEvent['changedTouches'] = asTouchList([{"target": oskKeyElement, ...center}]); + // It should still show up in .changedTouches, though. upEvent['changedTouches'] = asTouchList([{"target": oskKeyElement, ...center}]); } else { downEvent = new Event(BrowserDriver.oskDownMouseType); diff --git a/windows/src/desktop/kmshell/install/Keyman.Configuration.UI.KeymanProtocolHandler.pas b/windows/src/desktop/kmshell/install/Keyman.Configuration.UI.KeymanProtocolHandler.pas index 8b78fa7601d..6f13f6777bd 100644 --- a/windows/src/desktop/kmshell/install/Keyman.Configuration.UI.KeymanProtocolHandler.pas +++ b/windows/src/desktop/kmshell/install/Keyman.Configuration.UI.KeymanProtocolHandler.pas @@ -39,6 +39,7 @@ implementation UfrmInstallKeyboard, Upload_Settings, utildir, + utilhttp, utilfiletypes; { TKeymanProtocolHandler } @@ -74,9 +75,9 @@ function TKeymanProtocolHandler.DoHandle(Owner: TComponent; if not m.Success then Exit(False); - PackageID := m.Groups[1].Value; + PackageID := URLDecode(m.Groups[1].Value); if m.Groups.Count > 2 - then BCP47 := m.Groups[2].Value + then BCP47 := URLDecode(m.Groups[2].Value) else BCP47 := ''; // Download the package diff --git a/windows/src/desktop/kmshell/install/UfrmInstallKeyboardFromWeb.pas b/windows/src/desktop/kmshell/install/UfrmInstallKeyboardFromWeb.pas index 31e6eafb67d..aff32510372 100644 --- a/windows/src/desktop/kmshell/install/UfrmInstallKeyboardFromWeb.pas +++ b/windows/src/desktop/kmshell/install/UfrmInstallKeyboardFromWeb.pas @@ -91,6 +91,7 @@ implementation Upload_Settings, utilfiletypes, utildir, + utilhttp, utilexecute, VersionInfo; @@ -190,7 +191,7 @@ procedure TfrmInstallKeyboardFromWeb.cefBeforeBrowseEx(Sender: TObject; const Ur if m.Success then begin // We want to install the keyboard found in the path. - PackageID := m.Groups[1].Value; + PackageID := URLDecode(m.Groups[1].Value); uri := TURI.Create(url); try diff --git a/windows/src/desktop/setup/setup.dpr b/windows/src/desktop/setup/setup.dpr index 0d9a16b875a..f1a27cf560b 100644 --- a/windows/src/desktop/setup/setup.dpr +++ b/windows/src/desktop/setup/setup.dpr @@ -45,7 +45,8 @@ uses Keyman.System.MITLicense in '..\..\global\delphi\general\Keyman.System.MITLicense.pas', Keyman.Setup.System.Locales in 'Keyman.Setup.System.Locales.pas', Keyman.System.UILanguageManager in '..\..\global\delphi\general\Keyman.System.UILanguageManager.pas', - Keyman.Setup.System.SetupUILanguageManager in 'Keyman.Setup.System.SetupUILanguageManager.pas'; + Keyman.Setup.System.SetupUILanguageManager in 'Keyman.Setup.System.SetupUILanguageManager.pas', + utilhttp in '..\..\..\..\common\windows\delphi\general\utilhttp.pas'; {$R icons.res} {$R version.res} diff --git a/windows/src/desktop/setup/setup.dproj b/windows/src/desktop/setup/setup.dproj index 33a9d6f4a5d..d1e4c212cf7 100644 --- a/windows/src/desktop/setup/setup.dproj +++ b/windows/src/desktop/setup/setup.dproj @@ -149,6 +149,7 @@ + Cfg_2 Base