diff --git a/common/web/gesture-recognizer/src/engine/headless/gesturePath.ts b/common/web/gesture-recognizer/src/engine/headless/gesturePath.ts index 8ba1f59a21d..aae4ba0a7f8 100644 --- a/common/web/gesture-recognizer/src/engine/headless/gesturePath.ts +++ b/common/web/gesture-recognizer/src/engine/headless/gesturePath.ts @@ -128,8 +128,9 @@ export class GesturePath extends EventEmitter extends Ges baseSource.path.off('invalidated', invalidatedHook); baseSource.path.off('step', stepHook); } + + // If the path was already completed, that should be reflected here, too. + if(baseSource.isPathComplete) { + this.path.terminate((baseSource.path.wasCancelled)); + this.disconnect(); + } } private get recognizerTranslation() { 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 b4c307cb986..deb29534a58 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 @@ -103,7 +103,16 @@ export class GestureMatcher implements PredecessorMatch< : predecessor.sources; const sourceTouchpoints = unfilteredSourceTouchpoints.map((entry) => { - return entry.isPathComplete ? null : entry; + if(source && entry == source) { + // Due to internal delays that can occur when an incoming tap triggers + // completion of a previously-existing gesture but is not included in it + // (`resetOnResolve` mechanics), it is technically possible for a very + // quick tap to be 'complete' by the time we start trying to match + // against it on some devices. We should still try in such cases. + return source; + } else { + return entry.isPathComplete ? null : entry; + } }).reduce((cleansed, entry) => { return entry ? cleansed.concat(entry) : cleansed; }, [] as GestureSource[]); diff --git a/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/gestureSequence.ts b/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/gestureSequence.ts index e03694eb195..51055eefc47 100644 --- a/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/gestureSequence.ts +++ b/common/web/gesture-recognizer/src/engine/headless/gestures/matchers/gestureSequence.ts @@ -6,6 +6,7 @@ import { GestureMatcher, MatchResult, PredecessorMatch } from "./gestureMatcher. import { GestureModel, GestureResolution } from "../specs/gestureModel.js"; import { MatcherSelection, MatcherSelector } from "./matcherSelector.js"; import { GestureRecognizerConfiguration, TouchpointCoordinator } from "../../../index.js"; +import { ManagedPromise, timedPromise } from "@keymanapp/web-utils"; export class GestureStageReport { /** @@ -153,7 +154,7 @@ export class GestureSequence extends EventEmitter extends EventEmitter extends EventEmitter { - return this.selector.potentialMatchersForSource(source) + return selectors.map((selector) => selector.potentialMatchersForSource(source) .map((matcher) => matcher.model.id) - }).reduce((deduplicated, arr) => { + ) + }).reduce((flattened, arr) => flattened.concat(arr)) + .reduce((deduplicated, arr) => { for(let entry of arr) { if(deduplicated.indexOf(entry) == -1) { deduplicated.push(entry); @@ -185,7 +193,7 @@ export class GestureSequence extends EventEmitter) => { + private readonly selectionHandler = async (selection: MatcherSelection) => { const matchReport = new GestureStageReport(selection); if(selection.matcher) { this.stageReports.push(matchReport); @@ -196,10 +204,11 @@ export class GestureSequence extends EventEmitter { if(!source.isPathComplete) { - source.terminate(selection.result.action.type == 'none'); + source.terminate(actionType == 'none'); } }); @@ -212,6 +221,34 @@ export class GestureSequence extends EventEmitter { + const promise = new ManagedPromise(); + source.path.on('invalidated', () => promise.resolve()); + source.path.on('complete', () => promise.resolve()); + return promise.corePromise; + }); + + if(sustainCompletionPromises.length > 0 && selection.result.action.awaitNested) { + await Promise.all(sustainCompletionPromises); + // Ensure all nested gestures finish resolving first before continuing by + // waiting against the macroqueue. + await timedPromise(0); + } + + // Actually drops the selection-mode state once all is complete. + // The drop MUST come after the `await` above. + this.touchpointCoordinator?.popSelector(this.pushedSelector); + + // May still need it active? + // this.pushedSelector.off('rejectionwithaction', this.modelResetHandler); + this.pushedSelector = null; + } + // Raise the event, providing a functor that allows the listener to specify an alt config for the next stage. // Example case: longpress => subkey selection - the subkey menu has different boundary conditions. this.emit('stage', matchReport, (command) => { @@ -244,17 +281,18 @@ export class GestureSequence extends EventEmitter 0) { - // Note: if a 'push', that should be handled by an event listener from the main engine driver (or similar) - const modelingSpinupPromise = this.selector.matchGesture(selection.matcher, nextModels); - modelingSpinupPromise.then(async (selectionHost) => this.selectionHandler(await selectionHost.selectionPromise)); + // Note: resolve selection-mode changes FIRST, before building the next GestureModel in the sequence. + // If a selection-mode change is triggered, any openings for new contacts on the next model can only + // be fulfilled if handled by the corresponding (pushed) selector, rather than the sequence's base selector. // Handling 'setchange' resolution actions (where one gesture enables a different gesture set for others // while active. Example case: modipress.) - if(selection.result.action.type == 'chain' && selection.result.action.selectionMode == this.pushedSelector?.baseGestureSetId) { + if(actionType == 'chain' && selection.result.action.selectionMode == this.pushedSelector?.baseGestureSetId) { // do nothing; maintain the existing 'selectionMode' behavior } else { // pop the old one, if it exists - if it matches our expectations for a current one. if(this.pushedSelector) { + this.pushedSelector.off('rejectionwithaction', this.modelResetHandler); this.touchpointCoordinator?.popSelector(this.pushedSelector); this.pushedSelector = null; } @@ -274,16 +312,32 @@ export class GestureSequence extends EventEmitter(targetSet); + changedSetSelector.on('rejectionwithaction', this.modelResetHandler); this.pushedSelector = changedSetSelector; this.touchpointCoordinator?.pushSelector(changedSetSelector); } } } + + /* If a selector has been pushed, we need to delegate the next gesture model in the chain + * to it in case it has extra contacts, as those will be processed under the pushed selector. + * + * Example case: a modipress + multitap key should prevent further multitap if a second, + * unrelated key is tapped. Detecting that second tap is only possible via the pushed + * selector. + * + * Future models in the chain are still drawn from the _current_ selector. + */ + const nextStageSelector = this.pushedSelector ?? this.selector; + + // Note: if a 'push', that should be handled by an event listener from the main engine driver (or similar) + const modelingSpinupPromise = nextStageSelector.matchGesture(selection.matcher, nextModels); + modelingSpinupPromise.then(async (selectionHost) => this.selectionHandler(await selectionHost.selectionPromise)); } else { // Any extra finalization stuff should go here, before the event, if needed. if(!this.markedComplete) { @@ -304,6 +358,10 @@ export class GestureSequence extends EventEmitter sourceIds.indexOf(a) == -1)) { return; } 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 2d6ae5d0189..9d9b894f54b 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 @@ -6,7 +6,6 @@ import { GestureSource, GestureSourceSubview } from "../../gestureSource.js"; import { GestureMatcher, MatchResult, PredecessorMatch } from "./gestureMatcher.js"; import { GestureModel } from "../specs/gestureModel.js"; import { GestureSequence } from "./index.js"; -import { ItemIdentifier } from "../../../configuration/gestureRecognizerConfiguration.js"; interface GestureSourceTracker { /** @@ -34,8 +33,9 @@ interface EventMap { * * This allows us to bypass that, resolving yet providing a pending Promise. */ -interface AsyncPromiseReturner { - selectionPromise: Promise; +interface SelectionSetupResults { + selectionPromise: Promise>; + sustainModeWithoutMatch?: boolean; } /** @@ -65,6 +65,8 @@ export class MatcherSelector extends EventEmitter; + private sustainMode: boolean = false; + constructor(baseSetId?: string) { super(); this.baseGestureSetId = baseSetId || 'default'; @@ -80,7 +82,11 @@ export class MatcherSelector extends EventEmitter matcher.allSourceIds.find((id) => id == source.identifier)); } - public cascadeTermination() { + /** + * + * @returns An array of all sources that will live on in `sustainWhenNested` mode. + */ + public cascadeTermination(): GestureSource[] { const potentialMatchers = this.potentialMatchers; const matchersToCancel = potentialMatchers.filter((matcher) => !matcher.model.sustainWhenNested); @@ -132,6 +138,9 @@ export class MatcherSelector extends EventEmitter matcher.cancel()); + this.sustainMode = true; + + return this._sourceSelector.map((data) => data.source); } /** @@ -156,7 +165,7 @@ export class MatcherSelector extends EventEmitter, gestureModelSet: GestureModel[] - ): Promise>>; + ): Promise>; /** * Facilitates matching a new stage in an ongoing gesture-stage sequence based on a previously- @@ -180,12 +189,12 @@ export class MatcherSelector extends EventEmitter, gestureModelSet: GestureModel[] - ): Promise>>; + ): Promise>; public async matchGesture( source: GestureSource | PredecessorMatch, gestureModelSet: GestureModel[] - ): Promise>> { + ): Promise> { /* * To be clear, this _starts_ the source-tracking process. It's an async process, though. * @@ -297,6 +306,9 @@ export class MatcherSelector extends EventEmitter(); this.pendingMatchSetup = pendingMatchGesture.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(); @@ -341,18 +353,29 @@ export class MatcherSelector extends EventEmitter extends EventEmitter { - return { - tracker: tracker, - // We need to inspect each matcher's `contacts` entries for references to the source. - pendingCount: this.potentialMatchers.filter((matcher) => { - return !!matcher.allSourceIds.find((id) => tracker.source.identifier == id); - }).length // and tally up a count at the end. - }; - }); - - // If we just rejected the last possible matcher for a tracked gesture-source... - // then, for each such affected source... - for(const stat of remainingMatcherStats) { - if(stat.pendingCount == 0) { - // ... report the failure and signal to close-out that source / stop tracking it. - stat.tracker.matchPromise.resolve({ - matcher: null, - result: { - matched: false, - action: { - type: 'complete', - item: null - } - } - }); - } - } + this.finalizeMatcherlessTrackers(sourceMetadata); - // Again, we allow any other matchers against the represented sources to REMAIN AS THEY ARE. - // This is a "didn't resolve" case - we only matched against a "path reset" case. + /* We allow any other matchers against the represented sources to REMAIN AS THEY ARE. + * Special handling is only needed once none are left, which is what the + * `finalizeMatcherlessTrackers` call represents. + * + * This isn't generally a "no matches available case; it's a "_this_ model didn't match" + * case that only _sometimes_ happens to be the final "match not available" case. + */ return; } @@ -548,6 +549,11 @@ export class MatcherSelector extends EventEmitter) => { + if(this.sustainMode && !replacementModel.sustainWhenNested) { + this.finalizeMatcherlessTrackers(sourceMetadata); + return; + } + const replacementMatcher = new GestureMatcher(replacementModel, matcher); /* IMPORTANT: verify that the replacement model is initially valid. @@ -562,6 +568,9 @@ export class MatcherSelector extends EventEmitter extends EventEmitter { + // Triggers resolution of remaining matchers for the model-match, but that's + // asynchronous. matcher.cancel(); }); @@ -608,7 +619,53 @@ export class MatcherSelector extends EventEmitter[]) { + // Check - are there any remaining matchers compatible with the rejected matcher's sources? + const remainingMatcherStats = trackers.map((tracker) => { + return { + tracker: tracker, + // We need to inspect each matcher's `contacts` entries for references to the source. + pendingCount: this.potentialMatchers.filter((matcher) => { + return !!matcher.allSourceIds.find((id) => tracker.source.identifier == id); + }).length // and tally up a count at the end. + }; + }); + + // If we just rejected the last possible matcher for a tracked gesture-source... + // then, for each such affected source... + for(const stat of remainingMatcherStats) { + if(stat.pendingCount == 0) { + // ... report the failure and signal to close-out that source / stop tracking it. + stat.tracker.matchPromise.resolve({ + matcher: null, + result: { + matched: false, + action: { + type: 'complete', + item: null + } + } + }); + } + } + } } \ No newline at end of file diff --git a/common/web/gesture-recognizer/src/engine/headless/gestures/specs/gestureModel.ts b/common/web/gesture-recognizer/src/engine/headless/gestures/specs/gestureModel.ts index 2ba3451dbe7..27b2a4d32fc 100644 --- a/common/web/gesture-recognizer/src/engine/headless/gestures/specs/gestureModel.ts +++ b/common/web/gesture-recognizer/src/engine/headless/gestures/specs/gestureModel.ts @@ -42,7 +42,7 @@ export interface ResolutionChain { * Followup gesture-models must also specify the alternate model set in order to maintain it during * transition between components. Leaving it `undefined` in a followup will fully cancel the * alternate gesture-component selection mode and any gestures activated during the alternate - * selection mode (unless `sustainIfNested` is `true` for the processing gesture model). + * selection mode (unless `sustainWhenNested` is `true` for the processing gesture model). * * Changing to a different ID will do the likewise, then reactivate the alternate gesture-selection * mode with the newly-specified gesture-model set target. @@ -51,7 +51,8 @@ export interface ResolutionChain { } export interface ResolutionComplete { - type: 'complete' + type: 'complete', + awaitNested?: boolean } export interface RejectionDefault { @@ -111,6 +112,9 @@ export interface GestureModel { * Indicates that the corresponding GestureSource should not be considered part of the * Gesture sequence being matched, acting more as a separate gesture that 'triggers' a state * change in the current gesture being processed. + * + * Only takes effect if a model instantly resolves or rejects upon being considered for + * inclusion in the model. */ resetOnResolve?: boolean, /** diff --git a/common/web/gesture-recognizer/src/engine/headless/touchpointCoordinator.ts b/common/web/gesture-recognizer/src/engine/headless/touchpointCoordinator.ts index bf2b6c36750..4caf4ad9b68 100644 --- a/common/web/gesture-recognizer/src/engine/headless/touchpointCoordinator.ts +++ b/common/web/gesture-recognizer/src/engine/headless/touchpointCoordinator.ts @@ -1,10 +1,12 @@ import EventEmitter from "eventemitter3"; import { InputEngineBase } from "./inputEngineBase.js"; -import { buildGestureMatchInspector, GestureSource } from "./gestureSource.js"; +import { GestureSource } from "./gestureSource.js"; import { MatcherSelection, MatcherSelector } from "./gestures/matchers/matcherSelector.js"; 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"; interface EventMap { /** @@ -76,22 +78,64 @@ export class TouchpointCoordinator extends Even selector.on('rejectionwithaction', this.modelResetHandler); } - public popSelector(selector: MatcherSelector) { + public sustainSelectorSubstack(selector: MatcherSelector) { + if(!selector) { + return []; + } + + // If it's already been popped, just silently return. + const index = this.selectorStack.indexOf(selector); + if(index == -1) { + return []; + } + /* c8 ignore start */ if(this.selectorStack.length <= 1) { - throw new Error("May not pop the original, base gesture selector."); + throw new Error("May not force the original, base gesture selector into sustain mode."); } + /* c8 ignore end */ + + let sustainedSources: GestureSource[] = []; + + for(let i = index; i < this.selectorStack.length; i++) { + selector = this.selectorStack[i]; + // If there are any models active with the `sustainWhenNested` property, + // the following Promise resolves once those are also completed. + sustainedSources = sustainedSources.concat(selector.cascadeTermination()); + } + + return sustainedSources; + } + + public popSelector(selector: MatcherSelector) { + if(!selector) { + return; + } + + // If it's already been popped, just silently return. const index = this.selectorStack.indexOf(selector); if(index == -1) { - throw new Error("This selector has not been pushed onto the 'setChange' stack."); + return; + } + + /* c8 ignore start */ + if(this.selectorStack.length <= 1) { + throw new Error("May not pop the original, base gesture selector."); } /* c8 ignore end */ - selector.off('rejectionwithaction', this.modelResetHandler); - selector.cascadeTermination(); + while(index < this.selectorStack.length) { + selector = this.selectorStack[index]; + selector.off('rejectionwithaction', this.modelResetHandler); + + this.selectorStack.splice(index, 1); + } + + // Should be fine as-is for now b/c modipress is always a base-selector gesture and is + // the only thing modifying stateToken within KeymanWeb. May need an async/await in + // the future if other things become able to manipulate state tokens with this engine. - this.selectorStack.splice(index, 1); // Make sure the current state token is set at this stage. this.currentSelector.stateToken = this.stateToken; } @@ -104,60 +148,111 @@ export class TouchpointCoordinator extends Even return this.selectorStack[this.selectorStack.length-1]; } + private buildGestureMatchInspector(selector: MatcherSelector) { + return (source: GestureSource) => { + // Get the selectors at the time of the call, not at the time of the functor's construction. + const selectorIndex = this.selectorStack.indexOf(selector); + const selectors = this.selectorStack.slice(selectorIndex); + + return selectors.map((selector) => selector.potentialMatchersForSource(source).map((matcher) => matcher.model.id)) + .reduce((flattened, entry) => flattened.concat(entry)); + }; + } + protected addEngine(engine: InputEngineBase) { engine.on('pointstart', this.onNewTrackedPath); this.inputEngines.push(engine); } - private readonly onNewTrackedPath = (touchpoint: GestureSource) => { + private readonly onNewTrackedPath = async (touchpoint: GestureSource) => { this.addSimpleSourceHooks(touchpoint); const modelDefs = this.gestureModelDefinitions; + + let potentialSelector: MatcherSelector; + let selectionPromise: Promise>; + do { + potentialSelector = this.currentSelector; + + /* We wait for the source to fully pass through the gesture-model spin-up phase; there's + * a chance that the new source will complete an existing gesture instantly without being + * locked to it, resulting in activation of a different `stateToken`. + * + * This, in turn, can affect what the initial 'item' for the new gesture will be. + */ + const modelingSpinupPromise = potentialSelector.matchGesture(touchpoint, getGestureModelSet(modelDefs, potentialSelector.baseGestureSetId)); + const modelingSpinupResults = await modelingSpinupPromise; + + if(modelingSpinupResults.sustainModeWithoutMatch) { + + const correctSample = (sample: InputSample) => { + sample.stateToken = this.stateToken; + sample.item = touchpoint.currentRecognizerConfig.itemIdentifier(sample, null); + }; + + /* May need to do a state-token change check & update the item; an `awaitNested` 'complete' action + * may have been pending in the meantime that could have triggered a change. + * + * (The MatcherSelector's state token will not have been updated b/c it will have already been popped, + * and because it's popped, it should not be responsible for managing the new GestureSource - + * including shifts in state token.) + * + * Current actual use-case: deferred modipress due to ongoing flick, auto-completed by new incoming touch. + */ + touchpoint.path.coords.forEach(correctSample); + + // Don't forget to also correct the `stateToken` and `baseItem`! + touchpoint.stateToken = this.stateToken; + touchpoint.baseItem = touchpoint.path.coords[0].item; + + // Also, in case a contact model's path-eval references data via stats... + correctSample(touchpoint.path.stats.initialSample); + correctSample(touchpoint.path.stats.lastSample); + continue; + } else { + selectionPromise = modelingSpinupResults.selectionPromise; + break; + } + + // Can only happen if a `sustainWhenNested` model state is resolved, nested within + // a gesture whose completion action requests `awaitNested`. + } while(potentialSelector != this.currentSelector); + const selector = this.currentSelector; - touchpoint.setGestureMatchInspector(buildGestureMatchInspector(selector)); + touchpoint.setGestureMatchInspector(this.buildGestureMatchInspector(selector)); + this.emit('inputstart', touchpoint); - /* We wait for the source to fully pass through the gesture-model spin-up phase; there's - * a chance that the new source will complete an existing gesture instantly without being - * locked to it, resulting in activation of a different `stateToken`. - * - * This, in turn, can affect what the initial 'item' for the new gesture will be. - */ - const modelingSpinupPromise = selector.matchGesture(touchpoint, getGestureModelSet(modelDefs, selector.baseGestureSetId)); - modelingSpinupPromise.then(async (selectionPromiseHost) => { - this.emit('inputstart', touchpoint); + const selection = await selectionPromise; - const selection = await selectionPromiseHost.selectionPromise; + // Any related 'push' mechanics that may still be lingering are currently handled by GestureSequence + // during its 'completion' processing. (See `GestureSequence.selectionHandler`.) + if(!selection || selection.result.matched == false) { + return; + } - // Any related 'push' mechanics that may still be lingering are currently handled by GestureSequence - // during its 'completion' processing. (See `GestureSequence.selectionHandler`.) - if(selection.result.matched == false) { + // For multitouch gestures, only report the gesture **once**. + const sourceIDs = selection.matcher.allSourceIds; + for(let sequence of this._activeGestures) { + if(!!sequence.allSourceIds.find((id1) => !!sourceIDs.find((id2) => id1 == id2))) { + // We've already established (and thus, already reported) a GestureSequence for this selection. return; } + } - // For multitouch gestures, only report the gesture **once**. - const sourceIDs = selection.matcher.allSourceIds; - for(let sequence of this._activeGestures) { - if(!!sequence.allSourceIds.find((id1) => !!sourceIDs.find((id2) => id1 == id2))) { - // We've already established (and thus, already reported) a GestureSequence for this selection. - return; - } + const gestureSequence = new GestureSequence(selection, modelDefs, this.currentSelector, this); + this._activeGestures.push(gestureSequence); + gestureSequence.on('complete', () => { + // When the GestureSequence is fully complete and all related `firstSelectionPromise`s have + // had the chance to resolve, drop the reference; prevent memory leakage. + const index = this._activeGestures.indexOf(gestureSequence); + if(index != -1) { + this._activeGestures.splice(index, 1); } + }); - const gestureSequence = new GestureSequence(selection, modelDefs, this.currentSelector, this); - this._activeGestures.push(gestureSequence); - gestureSequence.on('complete', () => { - // When the GestureSequence is fully complete and all related `firstSelectionPromise`s have - // had the chance to resolve, drop the reference; prevent memory leakage. - const index = this._activeGestures.indexOf(gestureSequence); - if(index != -1) { - this._activeGestures.splice(index, 1); - } - }); - - // Could track sequences easily enough; the question is how to tell when to 'let go'. + // Could track sequences easily enough; the question is how to tell when to 'let go'. - this.emit('recognizedgesture', gestureSequence); - }); + this.emit('recognizedgesture', gestureSequence); } public get activeGestures(): GestureSequence[] { diff --git a/common/web/gesture-recognizer/src/engine/touchEventEngine.ts b/common/web/gesture-recognizer/src/engine/touchEventEngine.ts index 9d6ac4995f1..a2fc2b6b2c6 100644 --- a/common/web/gesture-recognizer/src/engine/touchEventEngine.ts +++ b/common/web/gesture-recognizer/src/engine/touchEventEngine.ts @@ -63,8 +63,11 @@ export class TouchEventEngine extends InputEv private preventPropagation(e: TouchEvent) { // Standard event maintenance - e.preventDefault(); - e.cancelBubble=true; + if(e.cancelable) { + // Chrome generates error-log messages if this is attempted while + // the condition is false. + e.preventDefault(); + } if(typeof e.stopImmediatePropagation == 'function') { e.stopImmediatePropagation(); diff --git a/common/web/gesture-recognizer/src/test/auto/headless/gestures/gestureMatcher.spec.ts b/common/web/gesture-recognizer/src/test/auto/headless/gestures/gestureMatcher.spec.ts index 10969c541c9..0ffb14e3034 100644 --- a/common/web/gesture-recognizer/src/test/auto/headless/gestures/gestureMatcher.spec.ts +++ b/common/web/gesture-recognizer/src/test/auto/headless/gestures/gestureMatcher.spec.ts @@ -904,69 +904,5 @@ describe("GestureMatcher", function() { assert.deepEqual(await secondMatcher.promise, { matched: false, action: { type: 'none', item: null } }); assert.isTrue(sources[0].path.isComplete); }); - - it("rejected: extra touch detected", async function() { - const turtle = new TouchpathTurtle({ - targetX: 1, - targetY: 1, - t: 100, - item: 'a' - }); - turtle.wait(MainLongpressSourceModel.timer.duration, 25); - turtle.hoveredItem = null; - turtle.commitPending(); - turtle.move(45, 50, 100, 4); - turtle.hoveredItem = 'b'; - turtle.commitPending(); - - const { - sources, - modelMatcherPromise, - executor - } = simulateMultiSourceMatcherInput([ - { type: 'sequence', samples: turtle.path, terminate: false } - ], this.fakeClock, LongpressModel); - - let completion = executor(); - const modelMatcher = await modelMatcherPromise; - - await Promise.race([completion, modelMatcher.promise]); - - // Copies from the longpress unit test to find a good spot to split the path. - assert.equal(await promiseStatus(modelMatcher.promise), PromiseStatuses.PROMISE_RESOLVED); - assert.equal(await promiseStatus(completion), PromiseStatusModule.PROMISE_PENDING); - - // And now for the real meat of the test. - - // Starts a new matcher for a followup gesture component/link, which has a number of fun - // intentional side-effects. - const secondMatcher = new gestures.matchers.GestureMatcher(SubkeySelectModel, modelMatcher); - // There's only the one touchpoint, so there's no need for synchronization overhead here. - sources[0].path.on('step', () => secondMatcher.update()); - sources[0].path.on('complete', () => secondMatcher.update()); - sources[0].path.on('invalidated', () => secondMatcher.update()); - - timedPromise(50).then(() => { - const secondContact = new GestureSource(5, null, true); - sources.push(secondContact); - secondMatcher.addContact(secondContact); - secondContact.update({ - targetX: 50, - targetY: 50, - t: 100 + MainLongpressSourceModel.timer.duration + 50, - item: 'c' - }); - }); - - // 100ms left to simulate until the end. - - // With the followup matcher now fully constructed & connected, play the rest of the sequence out. - await completion; - - assert.equal(await promiseStatus(secondMatcher.promise), PromiseStatuses.PROMISE_RESOLVED); - assert.deepEqual(await secondMatcher.promise, { matched: false, action: { type: 'none', item: null } }); - secondMatcher.finalizeSources(); - assert.isTrue(sources[0].path.isComplete); - }); }); }); \ No newline at end of file diff --git a/common/web/gesture-recognizer/src/test/auto/headless/gestures/isolatedGestureSpecs.ts b/common/web/gesture-recognizer/src/test/auto/headless/gestures/isolatedGestureSpecs.ts index e55ac56b5ba..f47d4057361 100644 --- a/common/web/gesture-recognizer/src/test/auto/headless/gestures/isolatedGestureSpecs.ts +++ b/common/web/gesture-recognizer/src/test/auto/headless/gestures/isolatedGestureSpecs.ts @@ -152,13 +152,6 @@ export const SubkeySelectModel: GestureModel = { }, endOnResolve: true, endOnReject: true - }, { - // A second touch while selecting a subkey will trigger instant cancellation - // of subkey mode. (With this setting in place, anyway.) - // - // Might not be ideal for actual production... but it does have benefits for - // unit testing the gesture-matching engine. - model: specs.InstantRejectionModel } ], resolutionAction: { diff --git a/common/web/gesture-recognizer/src/test/auto/headless/gestures/matcherSelector.spec.ts b/common/web/gesture-recognizer/src/test/auto/headless/gestures/matcherSelector.spec.ts index a60ae1fbc3b..47fb2201439 100644 --- a/common/web/gesture-recognizer/src/test/auto/headless/gestures/matcherSelector.spec.ts +++ b/common/web/gesture-recognizer/src/test/auto/headless/gestures/matcherSelector.spec.ts @@ -649,7 +649,9 @@ describe("MatcherSelector", function () { assert.isTrue(sources[0].path.isComplete); // Make sure the macroqueue has a chance to process; the second tap must first cancel the - // potential multitap, and moving past that involves waiting on the macroqueue. + // potential multitap, and moving past that includes 1 wait on the macroqueue... which may + // be scheduled after the first such wait below, hence the second. + await timedPromise(0); await timedPromise(0); await Promise.race([completion, selectionPromises[1]]); @@ -657,6 +659,7 @@ describe("MatcherSelector", function () { // Ignoring the multi-tap leadup and starting a new gesture-stage sequence instead... const selection2 = await selectionPromises[1]; + assert.isOk(selection2); assert.deepEqual(selection2.result, {matched: true, action: { type: 'chain', item: 'b', next: 'multitap' }}); assert.deepEqual(selection2.matcher.model, SimpleTapModel); assert.isTrue(sources[0].path.isComplete); diff --git a/common/web/keyboard-processor/src/text/keyboardProcessor.ts b/common/web/keyboard-processor/src/text/keyboardProcessor.ts index 2a2dc0bb70b..14e819a56dd 100644 --- a/common/web/keyboard-processor/src/text/keyboardProcessor.ts +++ b/common/web/keyboard-processor/src/text/keyboardProcessor.ts @@ -570,6 +570,9 @@ export default class KeyboardProcessor extends EventEmitter { resetContext(target?: OutputTarget) { this.layerId = 'default'; + + // Make sure all deadkeys for the context get cleared properly. + target?.resetContext(); this.keyboardInterface.resetContextCache(); // May be null if it's a keyboard swap. diff --git a/web/src/app/webview/src/configuration.ts b/web/src/app/webview/src/configuration.ts index 10cc8c2d026..bcb59c3e1a6 100644 --- a/web/src/app/webview/src/configuration.ts +++ b/web/src/app/webview/src/configuration.ts @@ -1,16 +1,46 @@ import { isEmptyTransform, RuleBehavior } from "@keymanapp/keyboard-processor"; import { EngineConfiguration, InitOptionSpec, InitOptionDefaults } from "keyman/engine/main"; +import { buildMergedTransform } from '@keymanapp/models-templates'; + import { type OnInsertTextFunc } from "./contextManager.js"; export class WebviewConfiguration extends EngineConfiguration { private _embeddingApp: string; private _oninserttext: OnInsertTextFunc; + private _hostInsert: OnInsertTextFunc; + + private pendingInserts: Transform[] = []; initialize(options: Required) { super.initialize(options); this._embeddingApp = options.embeddingApp; - this._oninserttext = options.oninserttext; + this._oninserttext = (dl, text, dr) => { + this.pendingInserts.push({ + deleteLeft: dl, + insert: text, + deleteRight: dr + }); + + Promise.resolve().then((this.pushInserts)); + } + + this._hostInsert = options.oninserttext; + } + + private readonly pushInserts = () => { + const finalTransform = this.pendingInserts.reduce((concatenated, current) => { + return buildMergedTransform(concatenated, current); + }, { + insert: '', + deleteLeft: 0 + }); + + this.pendingInserts.splice(0); + + if(this._hostInsert) { + this._hostInsert(finalTransform.deleteLeft, finalTransform.insert, finalTransform.deleteRight); + } } get embeddingApp() { diff --git a/web/src/app/webview/src/contextManager.ts b/web/src/app/webview/src/contextManager.ts index 580a1e1cfa7..42ee7887e17 100644 --- a/web/src/app/webview/src/contextManager.ts +++ b/web/src/app/webview/src/contextManager.ts @@ -22,6 +22,16 @@ class ContextHost extends Mock { } } + restoreTo(original: OutputTarget): void { + const reversionTransform = original.buildTransformFrom(this); + + super.restoreTo(original); + + if(this.oninserttext) { + this.oninserttext(reversionTransform.deleteLeft, reversionTransform.insert, reversionTransform.deleteRight); + } + } + // In app/webview, apps are expected to immediately update the selection range AFTER // changing the context's text. setText(text: string): void { diff --git a/web/src/engine/main/src/keymanEngine.ts b/web/src/engine/main/src/keymanEngine.ts index 4337c17d95d..a34ac408894 100644 --- a/web/src/engine/main/src/keymanEngine.ts +++ b/web/src/engine/main/src/keymanEngine.ts @@ -205,6 +205,8 @@ export default class KeymanEngine< this.contextManager.configure({ 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); }, predictionContext: new PredictionContext(this.core.languageProcessor, this.core.keyboardProcessor), 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 94d2c9da292..cdabaec67d7 100644 --- a/web/src/engine/osk/src/input/gestures/browser/keytip.ts +++ b/web/src/engine/osk/src/input/gestures/browser/keytip.ts @@ -49,6 +49,16 @@ export default class KeyTip implements KeyTipInterface { } show(key: KeyElement, on: boolean, vkbd: VisualKeyboard, previewHost: GesturePreviewHost) { + // During quick input sequences - especially during a multitap-modipress - it's possible + // for a user to request a preview for a key from a layer that is currently active, but + // currently not visible due to need previously-requested layout calcs for a different layer. + if(on) { + // Necessary for `key.offsetParent` and client-rect methods referenced below. + // Will not unnecessarily force reflow if the layer is already in proper document flow, + // but otherwise restores it. + vkbd.layerGroup.blinkLayer(key.key.spec.displayLayer); + } + // Create and display the preview // If !key.offsetParent, the OSK is probably hidden. Either way, it's a half- // decent null-guard check. diff --git a/web/src/engine/osk/src/input/gestures/browser/multitap.ts b/web/src/engine/osk/src/input/gestures/browser/multitap.ts index 02f88ef3304..3f1d304df3d 100644 --- a/web/src/engine/osk/src/input/gestures/browser/multitap.ts +++ b/web/src/engine/osk/src/input/gestures/browser/multitap.ts @@ -42,7 +42,7 @@ export default class Multitap implements GestureHandler { this.multitaps = [e.key.spec].concat(e.key.spec.multitap); this.sequence = source; - const startModipress = (tap) => { + const startModipress = (tap: GestureStageReport) => { // In case of a previous modipress that somehow wasn't cleared. this.modipress?.clear(); @@ -77,6 +77,9 @@ export default class Multitap implements GestureHandler { case 'multitap-end': case 'simple-tap': return; + case 'modipress-multitap-lock-transition': + this.modipress?.setLocked(); + return; // Once a multitap starts, it's better to emit keys on keydown; that way, // if a user holds long, they get what they see if they decide to stop, // but also have time to decide if they want to continue to what's next. diff --git a/web/src/engine/osk/src/input/gestures/specsForLayout.ts b/web/src/engine/osk/src/input/gestures/specsForLayout.ts index fae57886b96..b3072f4b7eb 100644 --- a/web/src/engine/osk/src/input/gestures/specsForLayout.ts +++ b/web/src/engine/osk/src/input/gestures/specsForLayout.ts @@ -233,7 +233,8 @@ export function gestureSetForLayout(flags: LayoutGestureSupportFlags, params: Ge modipressEndModel(), modipressMultitapTransitionModel(), withKeySpecFiltering(modipressMultitapStartModel(params), 0), - modipressMultitapEndModel(params) + modipressMultitapEndModel(params), + modipressMultitapLockModel() ]; const defaultSet = [ @@ -656,7 +657,8 @@ export function flickMidModel(params: GestureParams): GestureModel { type: 'chain', item: 'none', next: 'flick-end' - } + }, + sustainWhenNested: true } } @@ -676,7 +678,8 @@ export function flickResetModel(params: GestureParams): GestureModel { resolutionAction: { type: 'chain', next: 'flick-mid' - } + }, + sustainWhenNested: true }; } @@ -718,7 +721,8 @@ export function flickEndModel(params: GestureParams): GestureModel { resolutionAction: { type: 'complete', item: 'current' - } + }, + sustainWhenNested: true } } @@ -798,7 +802,7 @@ export function initialTapModel(params: GestureParams): GestureModel { timer: { duration: params.multitap.holdLength, expectedResult: false - } + }, }, endOnResolve: true }, { @@ -806,6 +810,7 @@ export function initialTapModel(params: GestureParams): GestureModel { resetOnResolve: true } ], + sustainWhenNested: true, rejectionActions: { timer: { type: 'replace', @@ -837,6 +842,7 @@ export function simpleTapModel(): GestureModel { resetOnResolve: true } ], + sustainWhenNested: true, resolutionAction: { type: 'complete', item: 'current' @@ -885,13 +891,6 @@ export function subkeySelectModel(): GestureModel { }, endOnResolve: true, endOnReject: true - }, { - // A second touch while selecting a subkey will trigger instant cancellation - // of subkey mode. (With this setting in place, anyway.) - // - // Might not be ideal for actual production... but it does have benefits for - // unit testing the gesture-matching engine. - model: instantContactRejectionModel() } ], resolutionAction: { @@ -945,6 +944,16 @@ export function modipressHoldModel(params: GestureParams): GestureModel { inheritElapsed: true } } + }, { + // If a new touchpoint comes in while in this state, lock in the modipress + // and prevent multitapping on it, as a different key has been tapped before + // the multitap base key since the latter's release. + model: { + ...instantContactResolutionModel(), + }, + // The incoming tap belongs to a different gesture; we just care to know that it + // happened. + resetOnResolve: true } ], // To be clear: any time modipress-hold is triggered and the timer duration elapses, @@ -1003,7 +1012,8 @@ export function modipressEndModel(): GestureModel { resolutionAction: { type: 'complete', // Key was already emitted from the 'modipress-start' stage. - item: 'none' + item: 'none', + awaitNested: true } } } @@ -1058,6 +1068,16 @@ export function modipressMultitapEndModel(params: GestureParams): GestureModel { + return { + id: 'modipress-multitap-lock-transition', + resolutionPriority: 5, + contacts: [ + // This exists as an intermediate state to transition from + // a modipress-multitap into a plain modipress without further + // multitap rota behavior. + { + model: { + ...instantContactResolutionModel(), + pathResolutionAction: 'resolve' // doesn't end the path; just lets it continue. + }, + } + ], + resolutionAction: { + type: 'chain', + next: 'modipress-end', + selectionMode: 'modipress', + item: 'none' + } + }; +} // #endregion \ No newline at end of file diff --git a/web/src/engine/osk/src/keyboard-layout/oskLayerGroup.ts b/web/src/engine/osk/src/keyboard-layout/oskLayerGroup.ts index bc587e61ff5..044af641fb7 100644 --- a/web/src/engine/osk/src/keyboard-layout/oskLayerGroup.ts +++ b/web/src/engine/osk/src/keyboard-layout/oskLayerGroup.ts @@ -107,6 +107,28 @@ export default class OSKLayerGroup { throw new Error(`Layer id ${layerId} could not be found`); } + this.blinkLayer(layer); + + return this.nearestKey(coord, layer); + } + + /** + * Temporarily enables the specified layer for page layout calculations and + * queues an immediate reversion to the 'true' active layer at the earliest + * opportunity on the JS microtask queue. + * @param layer + */ + public blinkLayer(arg: OSKLayer | string) { + if(typeof arg === 'string') { + const layerId = arg; + arg = this.layers[layerId]; + if(!arg) { + throw new Error(`Layer id ${layerId} could not be found`); + } + } + + const layer = arg; + // Note: we do NOT manipulate `._activeLayerId` here! This is designed // explicitly to be temporary. if(layer.element.style.display != 'block') { @@ -139,9 +161,7 @@ export default class OSKLayerGroup { layer.element.style.display = 'none'; trueLayer.element.style.display = 'block'; } - }) - - return this.nearestKey(coord, layer); + }); } private nearestKey(coord: Omit, 'item'>, layer: OSKLayer): KeyElement { diff --git a/web/src/engine/osk/src/visualKeyboard.ts b/web/src/engine/osk/src/visualKeyboard.ts index 86fe02d4bab..e3c086d1de8 100644 --- a/web/src/engine/osk/src/visualKeyboard.ts +++ b/web/src/engine/osk/src/visualKeyboard.ts @@ -489,7 +489,7 @@ export default class VisualKeyboard extends EventEmitter implements Ke // but only while still permitting new touches. If we're here, that time is over. for(let id of gestureSequence.allSourceIds) { // If the original preview host lives on, ensure it's cancelled now. - if(sourceTrackingMap[id].previewHost) { + if(sourceTrackingMap[id]?.previewHost) { this.gesturePreviewHost = null; sourceTrackingMap[id].previewHost.cancel(); } diff --git a/web/test.sh b/web/test.sh index e8fe74733e7..75a4f10ebcd 100755 --- a/web/test.sh +++ b/web/test.sh @@ -41,8 +41,7 @@ if builder_start_action test:libraries; then # For now, we'll also link in the gesture-recognizer unit tests here. builder_heading "Running gesture-recognizer test suite" - pushd "$KEYMAN_ROOT/common/web/gesture-recognizer" - npm run test -- $HEADLESS_FLAGS + "$KEYMAN_ROOT/common/web/gesture-recognizer/build.sh" test $HEADLESS_FLAGS popd builder_finish_action success test:libraries