From 2d6a74bc0470746da326c712b7922d9ddbbd98ca Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Fri, 6 Oct 2023 13:02:30 +0700 Subject: [PATCH 1/8] fix(web): unit test breakers, osk input emulation (partial) --- .../gestures/matchers/gestureSequence.ts | 4 +++- .../gestures/matchers/matcherSelector.ts | 5 +++- .../osk/src/keyboard-layout/oskLayerGroup.ts | 23 +++++-------------- .../tools/testing/recorder/browserDriver.ts | 17 ++++++++++---- 4 files changed, 26 insertions(+), 23 deletions(-) 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 6838647cb96..538bf7d2bbb 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 @@ -130,7 +130,9 @@ export class GestureSequence extends EventEmitter> { } public get allSourceIds(): string[] { - return this.stageReports[this.stageReports.length - 1]?.allSourceIds; + // Note: there is a brief window of time - between construction & the deferred first + // 'stage' event - during which this array may be of length 0. + return this.stageReports[this.stageReports.length - 1]?.allSourceIds ?? []; } private get baseGestureSetId(): string { 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 15d346f23b0..e4572a4d459 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 @@ -221,7 +221,10 @@ export class MatcherSelector extends EventEmitter> { matcher: null, result: { matched: false, - action: null + action: { + type: 'complete', + item: null + } } }); } diff --git a/web/src/engine/osk/src/keyboard-layout/oskLayerGroup.ts b/web/src/engine/osk/src/keyboard-layout/oskLayerGroup.ts index 4237c5fe2da..16a126a6e47 100644 --- a/web/src/engine/osk/src/keyboard-layout/oskLayerGroup.ts +++ b/web/src/engine/osk/src/keyboard-layout/oskLayerGroup.ts @@ -104,28 +104,17 @@ export default class OSKLayerGroup { private nearestKey(coord: Omit, 'item'>, layer: OSKLayer): KeyElement { const baseRect = this.element.getBoundingClientRect(); - /** - * Transforms the client rect of child elements to use a coordinate system where the top-left - * of the layer group's bounding rectangle serves as the origin - the same coordinate - * system output by the gesture engine. - * @param childRect - * @returns - */ - const translation = (childRect: DOMRect) => { - return new DOMRect(childRect.x - baseRect.x, childRect.y - baseRect.y, childRect.width, childRect.height); - } - let row: OSKRow = null; let bestMatchDistance = Number.MAX_VALUE; // Find the row that the touch-coordinate lies within. for(const r of layer.rows) { - const rowRect = translation(r.element.getBoundingClientRect()); - if(rowRect.top <= coord.targetY && coord.targetY < rowRect.bottom) { + const rowRect = r.element.getBoundingClientRect(); + if(rowRect.top <= coord.clientY && coord.clientY < rowRect.bottom) { row = r; break; } else { - const distance = rowRect.top > coord.targetY ? rowRect.top - coord.targetY : coord.targetY - rowRect.bottom; + const distance = rowRect.top > coord.clientY ? rowRect.top - coord.clientY : coord.clientY - rowRect.bottom; if(distance < bestMatchDistance) { bestMatchDistance = distance; @@ -144,12 +133,12 @@ export default class OSKLayerGroup { let dxMax = 24; let dxMin = 100000; - const x = coord.targetX; + const x = coord.clientX; 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 = translation(keySquare.getBoundingClientRect()); + const squareRect = keySquare.getBoundingClientRect(); // Find the actual key element. let childNode = keySquare.firstChild ? keySquare.firstChild as HTMLElement : keySquare; @@ -179,7 +168,7 @@ export default class OSKLayerGroup { if (dxMin < 100000) { const t = row.keys[closestKeyIndex].square; - const squareRect = translation(t.getBoundingClientRect()); + const squareRect = t.getBoundingClientRect(); const x1 = squareRect.left; const x2 = squareRect.right; diff --git a/web/src/tools/testing/recorder/browserDriver.ts b/web/src/tools/testing/recorder/browserDriver.ts index 53b16ab53d9..7b2626b584f 100644 --- a/web/src/tools/testing/recorder/browserDriver.ts +++ b/web/src/tools/testing/recorder/browserDriver.ts @@ -52,6 +52,11 @@ export class BrowserDriver { simulateOSKEvent(eventSpec: OSKInputEventSpec) { let target = this.target; let oskKeyElement = document.getElementById(eventSpec.keyID); + const boundingBox = oskKeyElement.getBoundingClientRect(); + const center = { + clientX: boundingBox.left + boundingBox.width/2, + clientY: boundingBox.top + boundingBox.height/2 + } if(!oskKeyElement) { console.error('Could not find OSK key "' + eventSpec.keyID + '"!'); @@ -65,14 +70,18 @@ export class BrowserDriver { if(keyman.config.hostDevice.touchable) { downEvent = new Event(BrowserDriver.oskDownTouchType); upEvent = new Event(BrowserDriver.oskUpTouchType); - downEvent['touches'] = [{"target": oskKeyElement}]; - upEvent['touches'] = [{"target": oskKeyElement}]; - downEvent['changedTouches'] = [{"target": oskKeyElement}]; - upEvent['changedTouches'] = [{"target": oskKeyElement}]; + downEvent['touches'] = [{"target": oskKeyElement, ...center}]; + upEvent['touches'] = [{"target": oskKeyElement, ...center}]; + downEvent['changedTouches'] = [{"target": oskKeyElement, ...center}]; + upEvent['changedTouches'] = [{"target": oskKeyElement, ...center}]; } else { downEvent = new Event(BrowserDriver.oskDownMouseType); upEvent = new Event(BrowserDriver.oskUpMouseType); + downEvent.clientX = center.clientX; + downEvent.clientY = center.clientY; downEvent['relatedTarget'] = target; + upEvent.clientX = center.clientX; + upEvent.clientY = center.clientY; upEvent['relatedTarget'] = target; // Mouse-click driven OSK use involves use of at least one mouse button. downEvent['button'] = upEvent['button'] = 0; From 7d740a850431eb032e84a3269a37a8f11d0fa3af Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Fri, 6 Oct 2023 13:50:31 +0700 Subject: [PATCH 2/8] fix(web): selector map cleanup on sequence termination --- .../gestures/matchers/gestureSequence.ts | 3 ++- .../gestures/matchers/matcherSelector.ts | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) 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 538bf7d2bbb..562836b510a 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 @@ -108,7 +108,8 @@ export class GestureSequence extends EventEmitter> { this.selector = selector; this.selector.on('rejectionwithaction', this.modelResetHandler); this.once('complete', () => { - this.selector.off('rejectionwithaction', this.modelResetHandler) + this.selector.off('rejectionwithaction', this.modelResetHandler); + this.selector.cleanSourceIdsForSequence(this.allSourceIds); // Dropping the reference here gives us two benefits: // 1. Allows garbage collection to do its thing; this might be the last reference left to the selector instance. 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 e4572a4d459..d84357b41d6 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 @@ -5,6 +5,7 @@ import { ManagedPromise } from "@keymanapp/web-utils"; import { GestureSource, GestureSourceSubview } from "../../gestureSource.js"; import { GestureMatcher, MatchResult, PredecessorMatch } from "./gestureMatcher.js"; import { GestureModel } from "../specs/gestureModel.js"; +import { GestureSequence } from "./index.js"; interface GestureSourceTracker { /** @@ -149,6 +150,12 @@ export class MatcherSelector extends EventEmitter> { ? [source instanceof GestureSourceSubview ? source.baseSource : source] : (source.sources as GestureSourceSubview[]).map((source) => source.baseSource); + if(sourceNotYetStaged) { + source.path.on('invalidated', () => { + this.cleanSourceIdsForSequence([source.identifier]); + }) + } + const matchPromise = new ManagedPromise>(); /* @@ -283,6 +290,15 @@ export class MatcherSelector extends EventEmitter> { this._sourceSelector.forEach((entry) => resetHooks(entry.source)); } + public cleanSourceIdsForSequence(idsToClean: string[]) { + for(const id of idsToClean) { + const index = this._sourceSelector.findIndex((entry) => entry.source.identifier); + if(index > -1) { + this._sourceSelector.splice(index, 1); + } + } + } + private matchersForSource(source: GestureSource) { return this.potentialMatchers.filter((matcher) => { return !!matcher.sources.find((src) => src.identifier == source.identifier) From 28f1d2589dd5e6cfa22c16fe8a14a1a221d9a240 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Mon, 9 Oct 2023 10:08:12 +0700 Subject: [PATCH 3/8] fix(web): integrated test fixes, sync -> async for test sequences --- .../gestures/matchers/matcherSelector.ts | 2 + .../tests/node/basic-engine.js | 4 +- .../tests/node/chirality.js | 4 +- .../keyboard-processor/tests/node/deadkeys.js | 4 +- common/web/recorder/src/index.ts | 20 +-- common/web/recorder/src/nodeProctor.ts | 4 +- common/web/recorder/src/proctor.ts | 4 +- web/src/engine/dom-utils/src/stylesheets.ts | 25 +++- web/src/engine/osk/src/views/oskView.ts | 9 +- web/src/test/auto/integrated/cases/engine.js | 18 +-- .../auto/integrated/cases/engine_chirality.js | 8 +- web/src/test/auto/integrated/cases/events.js | 12 +- web/src/test/auto/integrated/test_utils.js | 15 +-- .../tools/testing/recorder/browserDriver.ts | 124 ++++++++++++------ .../tools/testing/recorder/browserProctor.ts | 15 ++- 15 files changed, 168 insertions(+), 100 deletions(-) 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 d84357b41d6..123cc751591 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 @@ -151,6 +151,8 @@ export class MatcherSelector extends EventEmitter> { : (source.sources as GestureSourceSubview[]).map((source) => source.baseSource); if(sourceNotYetStaged) { + // Cancellation before a first stage is possible; in this case, there's no sequence + // to trigger cleanup. We can do that here. source.path.on('invalidated', () => { this.cleanSourceIdsForSequence([source.identifier]); }) diff --git a/common/web/keyboard-processor/tests/node/basic-engine.js b/common/web/keyboard-processor/tests/node/basic-engine.js index 15fd1efc5d8..1e7ff3a205e 100644 --- a/common/web/keyboard-processor/tests/node/basic-engine.js +++ b/common/web/keyboard-processor/tests/node/basic-engine.js @@ -42,10 +42,10 @@ describe('Engine - Basic Simulation', function() { if(!proctor.compatibleWithSuite(testSuite)) { it.skip(set.toTestName() + " - Cannot run this test suite on Node."); } else { - it(set.toTestName(), function() { + it(set.toTestName(), async function() { // Refresh the proctor instance at runtime. let proctor = new NodeProctor(keyboardWithHarness, device, assert.equal); - set.test(proctor); + await set.test(proctor); }); } } diff --git a/common/web/keyboard-processor/tests/node/chirality.js b/common/web/keyboard-processor/tests/node/chirality.js index b7f13214d48..9df87c6eda2 100644 --- a/common/web/keyboard-processor/tests/node/chirality.js +++ b/common/web/keyboard-processor/tests/node/chirality.js @@ -42,10 +42,10 @@ describe('Engine - Chirality', function() { if(!proctor.compatibleWithSuite(testSuite)) { it.skip(set.toTestName() + " - Cannot run this test suite on Node."); } else if(set.constraint.target == 'hardware') { - it(set.toTestName(), function() { + it(set.toTestName(), async function() { // Refresh the proctor instance at runtime. let proctor = new NodeProctor(keyboardWithHarness, device, assert.equal); - set.test(proctor); + await set.test(proctor); }); } else { it.skip(set.toTestName() + " - modifier state simulation for OSK not yet supported in headless KeyboardProcessor"); diff --git a/common/web/keyboard-processor/tests/node/deadkeys.js b/common/web/keyboard-processor/tests/node/deadkeys.js index c7e195bc2f7..e2fe70837eb 100644 --- a/common/web/keyboard-processor/tests/node/deadkeys.js +++ b/common/web/keyboard-processor/tests/node/deadkeys.js @@ -42,10 +42,10 @@ describe('Engine - Deadkeys', function() { if(!proctor.compatibleWithSuite(testSuite)) { it.skip(set.toTestName() + " - Cannot run this test suite on Node."); } else { - it(set.toTestName(), function() { + it(set.toTestName(), async function() { // Refresh the proctor instance at runtime. let proctor = new NodeProctor(keyboardWithHarness, device, assert.equal); - set.test(proctor); + await set.test(proctor); }); } } diff --git a/common/web/recorder/src/index.ts b/common/web/recorder/src/index.ts index 940f663b684..51a64c67276 100644 --- a/common/web/recorder/src/index.ts +++ b/common/web/recorder/src/index.ts @@ -212,7 +212,7 @@ export abstract class TestSequence { // Start with an empty OutputTarget and a fresh KeyboardProcessor. if(!target) { target = new Mock(); @@ -220,7 +220,7 @@ export abstract class TestSequence> { addTest(seq: Sequence): void; isValidForDevice(device: utils.DeviceSpec, usingOSK?: boolean): boolean; - test(proctor: Proctor): TestFailure[]; + test(proctor: Proctor): Promise; } /** @@ -563,13 +563,13 @@ export class EventSpecTestSet implements TestSet { } // Validity should be checked before calling this method. - test(proctor: Proctor): TestFailure[] { + async test(proctor: Proctor): Promise { var failures: TestFailure[] = []; let testSet = this.testSet; for(var i=0; i < testSet.length; i++) { var testSeq = this[i]; - var simResult = testSet[i].test(proctor); + var simResult = await testSet[i].test(proctor); if(!simResult.success) { // Failed test! failures.push(new TestFailure(this.constraint, testSeq, simResult.result)); @@ -613,13 +613,13 @@ export class RecordedSequenceTestSet implements TestSet { var failures: TestFailure[] = []; let testSet = this.testSet; for(var i=0; i < testSet.length; i++) { var testSeq = this[i]; - var simResult = testSet[i].test(proctor); + var simResult = await testSet[i].test(proctor); if(!simResult.success) { // Failed test! failures.push(new TestFailure(this.constraint, testSeq, simResult.result)); @@ -729,11 +729,11 @@ export class KeyboardTest { newSet.addTest(seq); } - test(proctor: Proctor) { + async test(proctor: Proctor) { var setHasRun = false; var failures: TestFailure[] = []; - proctor.beforeAll(); + await proctor.beforeAll(); // The original test spec requires a browser environment and thus requires its own `.run` implementation. if(!(proctor.compatibleWithSuite(this))) { @@ -745,7 +745,7 @@ export class KeyboardTest { var testSet = this.inputTestSets[i]; if(proctor.matchesTestSet(testSet)) { - var testFailures = testSet.test(proctor); + var testFailures = await testSet.test(proctor); if(testFailures) { failures = failures.concat(testFailures); } diff --git a/common/web/recorder/src/nodeProctor.ts b/common/web/recorder/src/nodeProctor.ts index 5548905e6e9..6afd7fc8fff 100644 --- a/common/web/recorder/src/nodeProctor.ts +++ b/common/web/recorder/src/nodeProctor.ts @@ -21,7 +21,7 @@ export default class NodeProctor extends Proctor { this.keyboardWithHarness = kbdHarness; } - beforeAll() { + async beforeAll() { // } @@ -47,7 +47,7 @@ export default class NodeProctor extends Proctor { return true; } - simulateSequence(sequence: TestSequence, target?: OutputTarget): string { + async simulateSequence(sequence: TestSequence, target?: OutputTarget): Promise { // Start with an empty OutputTarget and a fresh KeyboardProcessor. if(!target) { target = new Mock(); diff --git a/common/web/recorder/src/proctor.ts b/common/web/recorder/src/proctor.ts index 50b07032b6d..47d5b5d81ee 100644 --- a/common/web/recorder/src/proctor.ts +++ b/common/web/recorder/src/proctor.ts @@ -29,7 +29,7 @@ export default abstract class Proctor { } // Performs global test prep. - abstract beforeAll(); + abstract beforeAll(): Promise; // Performs per-test setup abstract before(); @@ -49,5 +49,5 @@ export default abstract class Proctor { * Simulates the specified test sequence for use in testing. * @param sequence The recorded sequence, generally provided by a test set. */ - abstract simulateSequence(sequence: TestSequence, target?: OutputTarget); + abstract simulateSequence(sequence: TestSequence, target?: OutputTarget): Promise; } \ No newline at end of file diff --git a/web/src/engine/dom-utils/src/stylesheets.ts b/web/src/engine/dom-utils/src/stylesheets.ts index fde753f313c..8dbefe1f02e 100644 --- a/web/src/engine/dom-utils/src/stylesheets.ts +++ b/web/src/engine/dom-utils/src/stylesheets.ts @@ -1,4 +1,4 @@ -import { DeviceSpec } from '@keymanapp/web-utils'; +import { DeviceSpec, ManagedPromise } from '@keymanapp/web-utils'; import { type InternalKeyboardFont as KeyboardFont } from '@keymanapp/keyboard-processor'; type FontFamilyStyleMap = {[family: string]: HTMLStyleElement}; @@ -28,6 +28,27 @@ export class StylesheetManager { this.linkNode.appendChild(sheet); } + /** + * Provides a `Promise` that resolves when all currently-linked stylesheets have loaded. + * Any change to the set of linked sheets after the initial call will be ignored. + */ + async allLoadedPromise() { + const promises: Promise[] = []; + + for(const sheetElem of this.linkedSheets) { + // Based on https://stackoverflow.com/a/21147238 + if(sheetElem.sheet?.cssRules) { + promises.push(Promise.resolve()); + } else { + const promise = new ManagedPromise(); + sheetElem.addEventListener('load', () => promise.resolve()); + promises.push(promise.corePromise); + } + } + + await Promise.all(promises); + } + /** * Build a stylesheet with a font-face CSS descriptor for the embedded font appropriate * for the browser being used @@ -212,6 +233,8 @@ export class StylesheetManager { sheet.parentNode.removeChild(sheet); } } + + this.linkedSheets.splice(0, this.linkedSheets.length); } } diff --git a/web/src/engine/osk/src/views/oskView.ts b/web/src/engine/osk/src/views/oskView.ts index 968a6333974..fee24a92a8b 100644 --- a/web/src/engine/osk/src/views/oskView.ts +++ b/web/src/engine/osk/src/views/oskView.ts @@ -698,14 +698,17 @@ export default abstract class OSKView extends EventEmitter implements // 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. let subpath = 'osk/'; if(this.config.isEmbedded) { subpath = ''; } - - // Install the default OSK stylesheet - 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. for(let sheetFile of OSKView.STYLESHEET_FILES) { const sheetHref = `${this.config.pathConfig.resources}/${subpath}${sheetFile}`; this.uiStyleSheetManager.linkExternalSheet(sheetHref); diff --git a/web/src/test/auto/integrated/cases/engine.js b/web/src/test/auto/integrated/cases/engine.js index d6fba7d400c..7a44d534367 100644 --- a/web/src/test/auto/integrated/cases/engine.js +++ b/web/src/test/auto/integrated/cases/engine.js @@ -178,31 +178,31 @@ describe('Engine - Browser Interactions', function() { assert.equal(inputElem.value, "ຫ"); }); - it('Simple OSK click', function() { + it('Simple OSK click', async function() { var inputElem = document.getElementById('singleton'); var lao_s_osk_json = {"type": "osk", "keyID": 'shift-K_S'}; var lao_s_event = new KMWRecorder.OSKInputEventSpec(lao_s_osk_json); let eventDriver = new KMWRecorder.BrowserDriver(inputElem); - eventDriver.simulateEvent(lao_s_event); + await eventDriver.simulateEvent(lao_s_event); if(inputElem['base']) { inputElem = inputElem['base']; } assert.equal(inputElem.value, ";"); }); - }) + }); describe('Sequence Simulation Checks', function() { this.timeout(testconfig.timeouts.scriptLoad); - it('Keyboard simulation', function() { - return runKeyboardTestFromJSON('/engine_tests/basic_lao_simulation.json', {usingOSK: false}, assert.equal, testconfig.timeouts.scriptLoad); + it('Keyboard simulation', async function() { + return await runKeyboardTestFromJSON('/engine_tests/basic_lao_simulation.json', {usingOSK: false}, assert.equal, testconfig.timeouts.scriptLoad); }); - it('OSK simulation', function() { - return runKeyboardTestFromJSON('/engine_tests/basic_lao_simulation.json', {usingOSK: true}, assert.equal, testconfig.timeouts.scriptLoad); + it('OSK simulation', async function() { + return await runKeyboardTestFromJSON('/engine_tests/basic_lao_simulation.json', {usingOSK: true}, assert.equal, testconfig.timeouts.scriptLoad); }) }); }); @@ -231,10 +231,10 @@ describe('Unmatched Final Groups', function() { fixture.cleanup(); }); - it('matches rule from early group AND performs default behavior', function() { + it('matches rule from early group AND performs default behavior', async function() { // While a TAB-oriented version would be nice, it's much harder to write the test // to detect change in last input element. - return runKeyboardTestFromJSON('/engine_tests/ghp_enter.json', {usingOSK: true}, assert.equal, testconfig.timeouts.scriptLoad); + return await runKeyboardTestFromJSON('/engine_tests/ghp_enter.json', {usingOSK: true}, assert.equal, testconfig.timeouts.scriptLoad); }); }); diff --git a/web/src/test/auto/integrated/cases/engine_chirality.js b/web/src/test/auto/integrated/cases/engine_chirality.js index a6607a8899a..a5021d7c5fb 100644 --- a/web/src/test/auto/integrated/cases/engine_chirality.js +++ b/web/src/test/auto/integrated/cases/engine_chirality.js @@ -30,23 +30,23 @@ describe('Engine - Chirality', function() { fixture.cleanup(); }); - it('Keyboard + OSK simulation', function() { + it('Keyboard + OSK simulation', async function() { this.timeout(testconfig.timeouts.scriptLoad * (testconfig.mobile ? 1 : 2)); /* Interestingly, this still works on iOS, probably because we're able to force-set * the 'location' property in the simulated event on mobile devices, even when iOS neglects to * set it for real events. */ - return runKeyboardTestFromJSON('/engine_tests/chirality.json', + return await runKeyboardTestFromJSON('/engine_tests/chirality.json', {usingOSK: false}, assert.equal, - testconfig.timeouts.scriptLoad).then(() => { + testconfig.timeouts.scriptLoad).then(async () => { /* We only really care to test the 'desktop' OSK because of how it directly models the modifier keys. * * The 'phone' and 'layout' versions take shortcuts that bypass any tricky chiral logic; * a better test for those would be to ensure the touch OSK is constructed properly. */ if(!testconfig.mobile) { - return runKeyboardTestFromJSON('/engine_tests/chirality.json', {usingOSK: true}, assert.equal, testconfig.timeouts.scriptLoad); + return await runKeyboardTestFromJSON('/engine_tests/chirality.json', {usingOSK: true}, assert.equal, testconfig.timeouts.scriptLoad); } }); }); diff --git a/web/src/test/auto/integrated/cases/events.js b/web/src/test/auto/integrated/cases/events.js index 2068a86aeab..4220ecf9cca 100644 --- a/web/src/test/auto/integrated/cases/events.js +++ b/web/src/test/auto/integrated/cases/events.js @@ -49,7 +49,7 @@ describe('Event Management', function() { assert.isNull(ele.onchange, '`onchange` handler was not called'); }); - it('OSK-based onChange event generation', function() { + it('OSK-based onChange event generation', async function() { var simple_A = {"type":"osk","keyID":"default-K_A"}; var event = new KMWRecorder.OSKInputEventSpec(simple_A); @@ -62,7 +62,7 @@ describe('Event Management', function() { keyman.setActiveElement(ele); let eventDriver = new KMWRecorder.BrowserDriver(ele); - eventDriver.simulateEvent(event); + await eventDriver.simulateEvent(event); let focusEvent = new FocusEvent('blur', {relatedTarget: ele}); ele.dispatchEvent(focusEvent); @@ -94,7 +94,7 @@ describe('Event Management', function() { assert.equal(counterObj.i, fin, "Event handler not called the expected number of times"); }); - it('OSK-based onInput event generation', function() { + it('OSK-based onInput event generation', async function() { var simple_A = {"type":"osk","keyID":"default-K_A"}; var event = new KMWRecorder.OSKInputEventSpec(simple_A); @@ -109,9 +109,9 @@ describe('Event Management', function() { }); let eventDriver = new KMWRecorder.BrowserDriver(ele); - eventDriver.simulateEvent(event); - eventDriver.simulateEvent(event); - eventDriver.simulateEvent(event); + await eventDriver.simulateEvent(event); + await eventDriver.simulateEvent(event); + await eventDriver.simulateEvent(event); assert.equal(counterObj.i, fin, "Event handler not called the expected number of times"); }); diff --git a/web/src/test/auto/integrated/test_utils.js b/web/src/test/auto/integrated/test_utils.js index ac98fa7f82d..6001b891327 100644 --- a/web/src/test/auto/integrated/test_utils.js +++ b/web/src/test/auto/integrated/test_utils.js @@ -201,32 +201,25 @@ export async function loadKeyboardFromJSON(jsonPath, timeout, params) { return loadKeyboardStub(stub, timeout, params); } -function runLoadedKeyboardTest(testDef, device, usingOSK, assertCallback) { +async function runLoadedKeyboardTest(testDef, device, usingOSK, assertCallback) { var inputElem = document.getElementById('singleton'); let proctor = new KMWRecorder.BrowserProctor(inputElem, device, usingOSK, assertCallback); - testDef.test(proctor); + await testDef.test(proctor); } -export function runKeyboardTestFromJSON(jsonPath, params, assertCallback, timeout) { +export async function runKeyboardTestFromJSON(jsonPath, params, assertCallback, timeout) { var testSpec = new KMWRecorder.KeyboardTest(fixture.load(jsonPath, true)); let device = new Device(); device.detect(); return loadKeyboardStub(testSpec.keyboard, timeout).then(() => { - runLoadedKeyboardTest(testSpec, device.coreSpec, params.usingOSK, assertCallback); + return runLoadedKeyboardTest(testSpec, device.coreSpec, params.usingOSK, assertCallback); }).finally(() => { keyman.removeKeyboards(testSpec.keyboard.id); }); } -// function retrieveAndReset(Pelem) { -// let val = Pelem.value; -// Pelem.value = ""; - -// return val; -// } - // Useful for tests related to strings with supplementary pairs. export function toSupplementaryPairString(code) { var H = Math.floor((code - 0x10000) / 0x400) + 0xD800; diff --git a/web/src/tools/testing/recorder/browserDriver.ts b/web/src/tools/testing/recorder/browserDriver.ts index 7b2626b584f..2dfadc551f5 100644 --- a/web/src/tools/testing/recorder/browserDriver.ts +++ b/web/src/tools/testing/recorder/browserDriver.ts @@ -4,6 +4,7 @@ import { OSKInputEventSpec, PhysicalInputEventSpec } from "@keymanapp/recorder-core"; +import { ManagedPromise, timedPromise } from "@keymanapp/web-utils"; import { type KeymanEngine } from 'keyman/app/browser'; @@ -26,13 +27,13 @@ export class BrowserDriver { this.target = target; } - simulateEvent(eventSpec: InputEventSpec) { + async simulateEvent(eventSpec: InputEventSpec) { switch(eventSpec.type) { case "key": this.simulateHardwareEvent(eventSpec as PhysicalInputEventSpec); break; case "osk": - this.simulateOSKEvent(eventSpec as OSKInputEventSpec); + await this.simulateOSKEvent(eventSpec as OSKInputEventSpec); break; } } @@ -49,57 +50,96 @@ export class BrowserDriver { this.target.dispatchEvent(event); } - simulateOSKEvent(eventSpec: OSKInputEventSpec) { - let target = this.target; - let oskKeyElement = document.getElementById(eventSpec.keyID); - const boundingBox = oskKeyElement.getBoundingClientRect(); - const center = { - clientX: boundingBox.left + boundingBox.width/2, - clientY: boundingBox.top + boundingBox.height/2 - } + async simulateOSKEvent(eventSpec: OSKInputEventSpec) { + const originalLayer = keyman.osk.vkbd.layerId; - if(!oskKeyElement) { - console.error('Could not find OSK key "' + eventSpec.keyID + '"!'); - // The following lines will throw an appropriate-enough error. - return; - } + // Calculations go wrong if the key's layer is not visible. + const keyID = eventSpec.keyID; + const targetLayer = keyID.indexOf('-') == -1 ? keyID : keyID.substring(0, keyID.lastIndexOf('-')); - // To be safe, we replicate the MouseEvent similarly to the keystroke event. - var downEvent; - var upEvent; - if(keyman.config.hostDevice.touchable) { - downEvent = new Event(BrowserDriver.oskDownTouchType); - upEvent = new Event(BrowserDriver.oskUpTouchType); - downEvent['touches'] = [{"target": oskKeyElement, ...center}]; - upEvent['touches'] = [{"target": oskKeyElement, ...center}]; - downEvent['changedTouches'] = [{"target": oskKeyElement, ...center}]; - upEvent['changedTouches'] = [{"target": oskKeyElement, ...center}]; - } else { - downEvent = new Event(BrowserDriver.oskDownMouseType); - upEvent = new Event(BrowserDriver.oskUpMouseType); - downEvent.clientX = center.clientX; - downEvent.clientY = center.clientY; - downEvent['relatedTarget'] = target; - upEvent.clientX = center.clientX; - upEvent.clientY = center.clientY; - upEvent['relatedTarget'] = target; - // Mouse-click driven OSK use involves use of at least one mouse button. - downEvent['button'] = upEvent['button'] = 0; - downEvent['buttons'] = 1; - upEvent['buttons'] = 0; + if(targetLayer != originalLayer) { + keyman.osk.vkbd.layerGroup.layers[originalLayer].element.style.display = 'none'; + keyman.osk.vkbd.layerGroup.layers[targetLayer].element.style.display = 'block'; + keyman.osk.vkbd.layerId = targetLayer; + + // Only the "current" layer of the OSK is laid out on refresh; a non-default + // layer won't have proper layout before this! + keyman.osk.vkbd.refreshLayout(); } - oskKeyElement.dispatchEvent(downEvent); - oskKeyElement.dispatchEvent(upEvent); + try { + let target = this.target; + let oskKeyElement = document.getElementById(eventSpec.keyID); + const boundingBox = oskKeyElement.getBoundingClientRect(); + const center = { + clientX: boundingBox.left + boundingBox.width/2, + clientY: boundingBox.top + boundingBox.height/2 + } + + if(!oskKeyElement) { + console.error('Could not find OSK key "' + eventSpec.keyID + '"!'); + // The following lines will throw an appropriate-enough error. + return; + } + + // To be safe, we replicate the MouseEvent similarly to the keystroke event. + var downEvent; + var upEvent; + if(keyman.config.hostDevice.touchable) { + downEvent = new Event(BrowserDriver.oskDownTouchType); + upEvent = new Event(BrowserDriver.oskUpTouchType); + downEvent['touches'] = [{"target": oskKeyElement, ...center}]; + upEvent['touches'] = [{"target": oskKeyElement, ...center}]; + downEvent['changedTouches'] = [{"target": oskKeyElement, ...center}]; + upEvent['changedTouches'] = [{"target": oskKeyElement, ...center}]; + } else { + downEvent = new Event(BrowserDriver.oskDownMouseType); + upEvent = new Event(BrowserDriver.oskUpMouseType); + downEvent.clientX = center.clientX; + downEvent.clientY = center.clientY; + downEvent['relatedTarget'] = target; + upEvent.clientX = center.clientX; + upEvent.clientY = center.clientY; + upEvent['relatedTarget'] = target; + // Mouse-click driven OSK use involves use of at least one mouse button. + downEvent['button'] = upEvent['button'] = 0; + downEvent['buttons'] = 1; + upEvent['buttons'] = 0; + } + + oskKeyElement.dispatchEvent(downEvent); + oskKeyElement.dispatchEvent(upEvent); + + // Note: our gesture engine's internal structure means that even simple keystrokes like this + // involve async processing. We'll need to sync up. + const defermentPromise = new ManagedPromise(); + + // Easiest way to resolve? Just wait for the key event. Currently, our integration tests + // at this level only use simple-taps, so there shouldn't be any cases that emit multiple + // key events at this time. + keyman.osk.on('keyevent', () => { defermentPromise.resolve() }); + + // Just in case something goes wrong and no event occurs, we apply a timeout to keep + // the tests moving along. + await Promise.race([defermentPromise.corePromise, timedPromise(50)]); + } finally { + // The alt-layer needs to be maintained until the key is generated. + if(targetLayer != originalLayer) { + keyman.osk.vkbd.layerGroup.layers[targetLayer].element.style.display = 'none'; + keyman.osk.vkbd.layerGroup.layers[originalLayer].element.style.display = 'block'; + keyman.osk.vkbd.layerId = originalLayer; + keyman.osk.vkbd.refreshLayout(); + } + } } // Execution of a test sequence depends on the testing environment; integrated // testing requires browser-specific code. - simulateSequence(sequence: InputEventSpecSequence): string { + async simulateSequence(sequence: InputEventSpecSequence): Promise { let ele = this.target; for(var i=0; i < sequence.inputs.length; i++) { - this.simulateEvent(sequence.inputs[i]); + await this.simulateEvent(sequence.inputs[i]); } if(ele instanceof HTMLInputElement || ele instanceof HTMLTextAreaElement) { diff --git a/web/src/tools/testing/recorder/browserProctor.ts b/web/src/tools/testing/recorder/browserProctor.ts index 8a4d4f6b860..2c3deeb27d0 100644 --- a/web/src/tools/testing/recorder/browserProctor.ts +++ b/web/src/tools/testing/recorder/browserProctor.ts @@ -20,6 +20,7 @@ import { TestSet } from "@keymanapp/recorder-core"; import { BrowserDriver } from "./browserDriver.js"; +import { type StylesheetManager } from "keyman/engine/dom-utils"; type AssertCallback = (s1: any, s2: any, msg?: string) => void; @@ -51,9 +52,15 @@ export class BrowserProctor extends Proctor { } // Performs browser-specific global test prep. - beforeAll() { + async beforeAll() { let ele = this.target; - (window['keyman'] as any).setActiveElement(ele['base'] ? ele['base'] : ele); + keyman.setActiveElement(ele, true); + + // If the CSS isn't fully loaded, the element positions will not match their expected + // locations in the keyboard layout and OSK keys won't be triggered properly by the + // gesture engine. + const styleManager = keyman.osk['uiStyleSheetManager'] as StylesheetManager; + await styleManager.allLoadedPromise(); } before() { @@ -74,12 +81,12 @@ export class BrowserProctor extends Proctor { // Execution of a test sequence depends on the testing environment; this handles // the browser-specific aspects. - simulateSequence(sequence: TestSequence, outputTarget?: OutputTarget): string { + async simulateSequence(sequence: TestSequence, outputTarget?: OutputTarget): Promise { let driver = new BrowserDriver(this.target); // For the version 10.0 spec if(sequence instanceof InputEventSpecSequence) { - return driver.simulateSequence(sequence); + return await driver.simulateSequence(sequence); // For the version 14.0+ spec } else if(sequence instanceof RecordedKeystrokeSequence) { From 107a6dc0461bd059de611fcb60f54e8da6a067f0 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Mon, 9 Oct 2023 10:14:17 +0700 Subject: [PATCH 4/8] fix(web): integrated-test stability fix --- web/src/app/browser/src/contextManager.ts | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/web/src/app/browser/src/contextManager.ts b/web/src/app/browser/src/contextManager.ts index 492c627146d..cbe3ab30c48 100644 --- a/web/src/app/browser/src/contextManager.ts +++ b/web/src/app/browser/src/contextManager.ts @@ -184,7 +184,17 @@ export default class ContextManager extends ContextManagerBase, sendEvents: boolean) { From 3874a52cba352efddf8198dc84c5837f757d8dcc Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Mon, 9 Oct 2023 11:07:08 +0700 Subject: [PATCH 5/8] fix(web): tracked source cleanup / robustness interactions --- .../gestures/matchers/gestureSequence.ts | 2 +- .../gestures/matchers/matcherSelector.ts | 18 +++++++++++++++--- .../gestures/touchpointCoordinator.spec.ts | 5 +++++ 3 files changed, 21 insertions(+), 4 deletions(-) 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 562836b510a..8364ed2a2e9 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 @@ -109,7 +109,7 @@ export class GestureSequence extends EventEmitter> { this.selector.on('rejectionwithaction', this.modelResetHandler); this.once('complete', () => { this.selector.off('rejectionwithaction', this.modelResetHandler); - this.selector.cleanSourceIdsForSequence(this.allSourceIds); + this.selector.dropSourcesWithIds(this.allSourceIds); // Dropping the reference here gives us two benefits: // 1. Allows garbage collection to do its thing; this might be the last reference left to the selector instance. 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 123cc751591..3a4c0d1a5f1 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 @@ -154,7 +154,7 @@ export class MatcherSelector extends EventEmitter> { // Cancellation before a first stage is possible; in this case, there's no sequence // to trigger cleanup. We can do that here. source.path.on('invalidated', () => { - this.cleanSourceIdsForSequence([source.identifier]); + this.dropSourcesWithIds([source.identifier]); }) } @@ -292,11 +292,23 @@ export class MatcherSelector extends EventEmitter> { this._sourceSelector.forEach((entry) => resetHooks(entry.source)); } - public cleanSourceIdsForSequence(idsToClean: string[]) { + public dropSourcesWithIds(idsToClean: string[]) { for(const id of idsToClean) { const index = this._sourceSelector.findIndex((entry) => entry.source.identifier); if(index > -1) { - this._sourceSelector.splice(index, 1); + // Ensure that any pending MatcherSelector and/or GestureSequence promises dependent + // on the source fully resolve (with cancellation). + const droppedSelector = this._sourceSelector.splice(index, 1)[0]; + droppedSelector.matchPromise.resolve({ + matcher: null, + result: { + matched: false, + action: { + type: 'none', + item: null + } + } + }); } } } diff --git a/common/web/gesture-recognizer/src/test/auto/headless/gestures/touchpointCoordinator.spec.ts b/common/web/gesture-recognizer/src/test/auto/headless/gestures/touchpointCoordinator.spec.ts index 87921d27c1f..bcf94894859 100644 --- a/common/web/gesture-recognizer/src/test/auto/headless/gestures/touchpointCoordinator.spec.ts +++ b/common/web/gesture-recognizer/src/test/auto/headless/gestures/touchpointCoordinator.spec.ts @@ -306,9 +306,14 @@ describe("TouchpointCoordinator", () => { }); const runnerPromise = fakeClock.runToLastAsync(); + const sequence = await sequencePromise; + const completeStub = sinon.fake(); + sequence.on('complete', completeStub); // was not called! Confirms a suspicion. await Promise.all([runnerPromise, completionPromise1, completionPromise2]); + assert.isTrue(completeStub.calledOnce); + // Verify that all sources and sequences are cleared. assert.sameOrderedMembers(touchpointCoordinator.activeSources, []); assert.sameOrderedMembers(touchpointCoordinator.activeGestures, []); From 1a3b02319ff57b8d5fb22505af182a29ccb4cfff Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Mon, 9 Oct 2023 12:10:44 +0700 Subject: [PATCH 6/8] fix(web): Touch[] != TouchList --- .../tools/testing/recorder/browserDriver.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/web/src/tools/testing/recorder/browserDriver.ts b/web/src/tools/testing/recorder/browserDriver.ts index 2dfadc551f5..0c906765f9b 100644 --- a/web/src/tools/testing/recorder/browserDriver.ts +++ b/web/src/tools/testing/recorder/browserDriver.ts @@ -10,6 +10,17 @@ import { type KeymanEngine } from 'keyman/app/browser'; declare var keyman: KeymanEngine; +function asTouchList(arr: any[]) { + return { + get length() { + return arr.length; + }, + + item(index: number) { + return arr[index]; + } + } +} export class BrowserDriver { static readonly physicalEventClass: string = "KeyboardEvent"; static readonly physicalEventType: string = "keydown"; @@ -88,10 +99,10 @@ export class BrowserDriver { if(keyman.config.hostDevice.touchable) { downEvent = new Event(BrowserDriver.oskDownTouchType); upEvent = new Event(BrowserDriver.oskUpTouchType); - downEvent['touches'] = [{"target": oskKeyElement, ...center}]; - upEvent['touches'] = [{"target": oskKeyElement, ...center}]; - downEvent['changedTouches'] = [{"target": oskKeyElement, ...center}]; - upEvent['changedTouches'] = [{"target": oskKeyElement, ...center}]; + downEvent['touches'] = asTouchList([{"target": oskKeyElement, ...center}]); + upEvent['touches'] = asTouchList([{"target": oskKeyElement, ...center}]); + downEvent['changedTouches'] = asTouchList([{"target": oskKeyElement, ...center}]); + upEvent['changedTouches'] = asTouchList([{"target": oskKeyElement, ...center}]); } else { downEvent = new Event(BrowserDriver.oskDownMouseType); upEvent = new Event(BrowserDriver.oskUpMouseType); From cf82a2f8404ed80295919891f937f8e899053e8f Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Mon, 9 Oct 2023 12:12:36 +0700 Subject: [PATCH 7/8] docs(web): updates comment --- web/src/app/browser/src/contextManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/app/browser/src/contextManager.ts b/web/src/app/browser/src/contextManager.ts index cbe3ab30c48..42b79109ef7 100644 --- a/web/src/app/browser/src/contextManager.ts +++ b/web/src/app/browser/src/contextManager.ts @@ -188,7 +188,7 @@ export default class ContextManager extends ContextManagerBase Date: Tue, 10 Oct 2023 08:15:01 +0700 Subject: [PATCH 8/8] fix(web): robustness for OSK onChange integrated test --- web/src/test/auto/integrated/cases/events.js | 6 ++++++ web/src/test/auto/integrated/test_utils.js | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/web/src/test/auto/integrated/cases/events.js b/web/src/test/auto/integrated/cases/events.js index 4220ecf9cca..5d7b3d20412 100644 --- a/web/src/test/auto/integrated/cases/events.js +++ b/web/src/test/auto/integrated/cases/events.js @@ -2,6 +2,7 @@ var assert = chai.assert; import { loadKeyboardFromJSON, + oskResourceLoadPromise, setupKMW, teardownKMW } from "../test_utils.js"; @@ -61,6 +62,11 @@ describe('Event Management', function() { keyman.setActiveElement(ele); + // Browsers will only start loading OSK resources (the CSS) once both a keyboard and target + // are set... and that's an async operation. + await oskResourceLoadPromise(); + + // OSK CSS is needed for successful simulation for integration tests involving the gesture engine. let eventDriver = new KMWRecorder.BrowserDriver(ele); await eventDriver.simulateEvent(event); diff --git a/web/src/test/auto/integrated/test_utils.js b/web/src/test/auto/integrated/test_utils.js index 6001b891327..9d0227bae52 100644 --- a/web/src/test/auto/integrated/test_utils.js +++ b/web/src/test/auto/integrated/test_utils.js @@ -220,6 +220,14 @@ export async function runKeyboardTestFromJSON(jsonPath, params, assertCallback, }); } +export async function oskResourceLoadPromise() { + // If the CSS isn't fully loaded, the element positions will not match their expected + // locations in the keyboard layout and OSK keys won't be triggered properly by the + // gesture engine. + const styleManager = keyman.osk['uiStyleSheetManager']; // is private + await styleManager.allLoadedPromise(); +} + // Useful for tests related to strings with supplementary pairs. export function toSupplementaryPairString(code) { var H = Math.floor((code - 0x10000) / 0x400) + 0xD800;