Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(web): fix engine initialization when initial keyboard is debug-compiled #11968

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -886,10 +886,12 @@ private static void copyAssets(Context context) {

// Copy default keyboard font
copyAsset(context, KMDefault_KeyboardFont, "", true);
// Includes something needed up to Chrome 61, which has full ES5 support.
// Thus, this one isn't legacy-only.
copyAsset(context, KMFilename_JSPolyfill2, "", true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addresses a side detail, though that side detail's documentation is within the same base issue. Got lumped together there, so "why not here?"


if(legacyMode) {
copyAsset(context, KMFilename_JSPolyfill, "", true);
copyAsset(context, KMFilename_JSPolyfill2, "", true);
copyAsset(context, KMFilename_JSPolyfill3, "", true);
}

Expand Down Expand Up @@ -1638,7 +1640,7 @@ public static boolean removeKeyboard(Context context, int position) {

public static boolean isDefaultKey(String key) {
return (
key != null &&
key != null &&
key.equals(KMString.format("%s_%s", KMDefault_LanguageID, KMDefault_KeyboardID)));
}

Expand Down Expand Up @@ -2084,11 +2086,11 @@ public static Point getWindowSize(Context context) {
wm.getDefaultDisplay().getSize(size);
return size;
}

WindowMetrics windowMetrics = wm.getCurrentWindowMetrics();
return new Point(
windowMetrics.getBounds().width(),
windowMetrics.getBounds().height());
windowMetrics.getBounds().height());
}

public static float getWindowDensity(Context context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,28 @@ import { Keyboard, KeyboardHarness, KeyboardLoaderBase, KeyboardLoadErrorBuilder

import { ManagedPromise } from '@keymanapp/web-utils';

/**
* Add this function as an event listener for the main `window`'s 'error' event
* in order to filter out the sanitized error generated by a failed
* script-loading attempt.
*
* Added to address #11962.
* @param err
*/
export function keyboardScriptErrorFilterer(err: ErrorEvent) {
if(/script error/i.test(err.message) && !err.filename) {
// Tends to be reached on Android devices; some sort of 'security' is
// enacted that obscures the actual error details.
err.preventDefault();
err.stopPropagation();
} else if(/modifierCodes/.test(err.message) && /undefined/.test(err.message)) {
// Tends to be reached for Keyman Engine for Web in the browser for
// locally-hosted files.
err.preventDefault();
err.stopPropagation();
}
}

export class DOMKeyboardLoader extends KeyboardLoaderBase {
public readonly element: HTMLIFrameElement;
private readonly performCacheBusting: boolean;
Expand Down
27 changes: 15 additions & 12 deletions web/src/app/browser/src/contextManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -485,8 +485,9 @@ export default class ContextManager extends ContextManagerBase<BrowserConfigurat
}
}

public async activateKeyboard(keyboardId: string, languageCode?: string, saveCookie?: boolean): Promise<boolean> {
public async activateKeyboard(keyboardId: string, languageCode?: string, saveCookie?: boolean, silenceErrorLogging?: boolean): Promise<boolean> {
saveCookie ||= false;
silenceErrorLogging ||= false;
const originalKeyboardTarget = this.currentKeyboardSrcTarget();

// If someone tries to activate a keyboard before we've had a chance to load it,
Expand Down Expand Up @@ -538,14 +539,16 @@ export default class ContextManager extends ContextManagerBase<BrowserConfigurat
const message = (err as Error)?.message ||
'Sorry, the ' + keyboardId + ' keyboard for ' + languageCode + ' is not currently available.';

if(err instanceof KeyboardScriptError) {
// We get signaled about error log messages if the site is connected to our Sentry error reporting
// system; we want to know if we have a broken keyboard that's been published.
console.error(err || message);
} else {
// If it's just internet connectivity or "file not found" issues, that's not worth reporting
// to Sentry.
console.warn(err || message);
if(!silenceErrorLogging) {
if(err instanceof KeyboardScriptError) {
// We get signaled about error log messages if the site is connected to our Sentry error reporting
// system; we want to know if we have a broken keyboard that's been published.
console.error(err || message);
} else {
// If it's just internet connectivity or "file not found" issues, that's not worth reporting
// to Sentry.
console.warn(err || message);
}
}

if(this.engineConfig.alertHost) {
Expand Down Expand Up @@ -809,7 +812,7 @@ export default class ContextManager extends ContextManagerBase<BrowserConfigurat
/**
* Restore the most recently used keyboard, if still available
*/
restoreSavedKeyboard(kbd: string) {
async restoreSavedKeyboard(kbd: string) {
// If no saved keyboard, defaults to US English
const d=kbd;

Expand All @@ -824,9 +827,9 @@ export default class ContextManager extends ContextManagerBase<BrowserConfigurat

// Sets the default stub (as specified with the `getSavedKeyboard` call) as active.
if(stub) {
return this.activateKeyboard(stub.id, stub.langId);
return this.activateKeyboard(stub.id, stub.langId, true, true);
} else {
return null;
return Promise.resolve(false);
}
}

Expand Down
24 changes: 22 additions & 2 deletions web/src/app/browser/src/keymanEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
VisualKeyboard
} from 'keyman/engine/osk';
import { ErrorStub, KeyboardStub, CloudQueryResult, toPrefixedKeyboardId as prefixed } from 'keyman/engine/package-cache';
import { DeviceSpec, Keyboard, KeyboardObject } from "@keymanapp/keyboard-processor";
import { DeviceSpec, Keyboard, KeyboardObject, KeyboardScriptError } from "@keymanapp/keyboard-processor";

import * as views from './viewsAnchorpoint.js';
import { BrowserConfiguration, BrowserInitOptionDefaults, BrowserInitOptionSpec } from './configuration.js';
Expand All @@ -26,6 +26,7 @@ import { UIModule } from './uiModuleInterface.js';
import { HotkeyManager } from './hotkeyManager.js';
import { BeepHandler } from './beepHandler.js';
import KeyboardInterface from './keyboardInterface.js';
import { keyboardScriptErrorFilterer } from '@keymanapp/keyboard-processor/dom-keyboard-loader';

export default class KeymanEngine extends KeymanEngineBase<BrowserConfiguration, ContextManager, HardwareEventKeyboard> {
touchLanguageMenu?: LanguageMenu;
Expand Down Expand Up @@ -219,6 +220,13 @@ export default class KeymanEngine extends KeymanEngineBase<BrowserConfiguration,
// Let any deferred, pre-init stubs complete registration
await this.config.deferForInitialization;

// We know the upcoming keyboard-load attempt may fail for debug-mode keyboards,
// so we aim to silence the script's emitted error if the type matches.
//
// Other areas also use the same filter function, so we create a distinct function
// unique to this location for our listener.
const errorFilterer = (event: ErrorEvent) => keyboardScriptErrorFilterer(event);
window.addEventListener('error', errorFilterer);
/*
Attempt to restore the user's last-used keyboard from their previous session.
The method auto-loads the default stub if one is available and the last-used keyboard
Expand All @@ -234,7 +242,19 @@ export default class KeymanEngine extends KeymanEngineBase<BrowserConfiguration,
// empty OSK that we'll instantly discard after.
try {
await loadingKbd;
} catch { /* in case of failed fetch due to network error or bad URI; we must still let the OSK init. */ };
} catch (err) {
// If there's a keyboard-script error, there's a non-zero chance that it's
// a debug-mode keyboard, which needs the OSK to be in place.
if(err instanceof KeyboardScriptError) {
// No error-filtering will be applied to the deferred keyboard load attempt,
// so error messages due to truly malformed keyboards will still be
// surfaced.
this.config.deferForOsk.then(() => this.contextManager.restoreSavedKeyboard(savedKeyboardStr));
}
/* in case of failed fetch due to network error or bad URI; we must still let the OSK init. */
} finally {
window.removeEventListener('error', errorFilterer);
};

const firstKbdConfig = {
keyboardToActivate: this.contextManager.activeKeyboard
Expand Down
21 changes: 19 additions & 2 deletions web/src/app/webview/src/keymanEngine.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { DefaultRules, DeviceSpec, RuleBehavior, Keyboard } from '@keymanapp/keyboard-processor'
import { DefaultRules, DeviceSpec, RuleBehavior, Keyboard, KeyboardScriptError } from '@keymanapp/keyboard-processor'
import { KeymanEngine as KeymanEngineBase, KeyboardInterface } from 'keyman/engine/main';
import { AnchoredOSKView, ViewConfiguration, StaticActivator } from 'keyman/engine/osk';
import { getAbsoluteX, getAbsoluteY } from 'keyman/engine/dom-utils';
Expand All @@ -8,6 +8,7 @@ import { WebviewConfiguration, WebviewInitOptionDefaults, WebviewInitOptionSpec
import ContextManager, { ContextHost } from './contextManager.js';
import PassthroughKeyboard from './passthroughKeyboard.js';
import { buildEmbeddedGestureConfig, setupEmbeddedListeners } from './oskConfiguration.js';
import { keyboardScriptErrorFilterer } from '@keymanapp/keyboard-processor/dom-keyboard-loader';

export default class KeymanEngine extends KeymanEngineBase<WebviewConfiguration, ContextManager, PassthroughKeyboard> {
// Ideally, we would be able to auto-detect `sourceUri`: https://stackoverflow.com/a/60244278.
Expand Down Expand Up @@ -81,14 +82,30 @@ export default class KeymanEngine extends KeymanEngineBase<WebviewConfiguration,
const kbdCache = this.keyboardRequisitioner.cache;
const firstStub = kbdCache.defaultStub;
let firstKeyboard: Keyboard;

if(firstStub) {
// If so, the common engine will have automatically started fetching it.
// Wait for the keyboard to load in order to avoid the high cost of building
// a stand-in we'll automatically replace.
try {
// Re-uses pre-existing fetch requests and fetched keyboards.
//
// As we haven't yet initialized the OSK, the osk-based API for debug
// values isn't yet available. Debug-mode keyboards will generate errors
// on load as a result, but we can silence those errors here...
window.addEventListener('error', keyboardScriptErrorFilterer);
firstKeyboard = await kbdCache.fetchKeyboardForStub(firstStub);
} catch { /* in case of failed fetch due to network error or bad URI; we must still let the OSK init. */ };
} catch (e) {
if(e instanceof KeyboardScriptError) {
// ... and retry the keyboard-load once the OSK is available to provide
// the debug-keyboard code API points.
this.config.deferForOsk.then(() => this.setActiveKeyboard(firstStub.id, firstStub.langId));
} else {
throw e;
}
} finally {
window.removeEventListener('error', keyboardScriptErrorFilterer);
}
}

const keyboardConfig: ViewConfiguration['keyboardToActivate'] = firstKeyboard ? {
Expand Down
16 changes: 9 additions & 7 deletions web/src/engine/main/src/contextManagerBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,11 +236,11 @@ export abstract class ContextManagerBase<MainConfig extends EngineConfiguration>
// still async-loading, we should go with the later setting - the preloaded one.
this.findAndPopActivation(this.currentKeyboardSrcTarget());

const activatingKeyboard = this.prepareKeyboardForActivation(keyboardId, languageCode);
let activatingKeyboard = this.prepareKeyboardForActivation(keyboardId, languageCode);

const originalKeyboardTarget = this.currentKeyboardSrcTarget();

const keyboard = await activatingKeyboard.keyboard;
let keyboard = await activatingKeyboard.keyboard;
if(keyboard == null && activatingKeyboard.metadata) {
// The activation was async and was cancelled - either by `beforeKeyboardChange` first-pass
// cancellation or because a different keyboard was requested before completion of the async load.
Expand Down Expand Up @@ -357,17 +357,19 @@ export abstract class ContextManagerBase<MainConfig extends EngineConfiguration>

let combinedPromise = Promise.race([keyboardPromise, timeoutPromise]);

// Ensure the async-load Promise completes properly.
// Ensure the async-load Promise completes properly, allowing us to
// consistently clear keyboard-loading alerts.
combinedPromise.then(() => {
completionPromise.resolve(null);
// Prevent any 'unhandled Promise rejection' events that may otherwise occur from the timeout promise.
// Prevent any 'unhandled Promise rejection' events that may otherwise
// occur from the timeout promise.
timeoutPromise.catch(() => {});
});
combinedPromise.catch((err) => {
}).catch((err) => {
completionPromise.resolve(err);
throw err;
});

// Any errors from the keyboard-loading process will continue to be
// forwarded through the main returned Promise.
return combinedPromise;
});

Expand Down
2 changes: 2 additions & 0 deletions web/src/engine/main/src/engineConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export class EngineConfiguration extends EventEmitter<EventMap> {
public hostDevice: DeviceSpec;
readonly sourcePath: string;
readonly deferForInitialization: ManagedPromise<void>;
deferForOsk: ManagedPromise<void>;

private _paths: PathConfiguration;
public activateFirstKeyboard: boolean;
Expand All @@ -39,6 +40,7 @@ export class EngineConfiguration extends EventEmitter<EventMap> {
this.sourcePath = sourcePath;
this.hostDevice = device;
this.deferForInitialization = new ManagedPromise<void>();
this.deferForOsk = new ManagedPromise<void>();
}

initialize(options: Required<InitOptionSpec>) {
Expand Down
36 changes: 31 additions & 5 deletions web/src/engine/main/src/keymanEngine.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { type Keyboard, KeyboardKeymanGlobal, ProcessorInitOptions } from "@keymanapp/keyboard-processor";
import { DOMKeyboardLoader as KeyboardLoader } from "@keymanapp/keyboard-processor/dom-keyboard-loader";
import { type Keyboard, KeyboardKeymanGlobal, ProcessorInitOptions, ManagedPromise, KeyboardScriptError } from "@keymanapp/keyboard-processor";
import { DOMKeyboardLoader as KeyboardLoader, keyboardScriptErrorFilterer } 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";
Expand Down Expand Up @@ -263,7 +263,7 @@ export default class KeymanEngine<
});

kbdCache.on('stubadded', (stub) => {
let eventRaiser = () => {
let eventRaiser = async () => {
// The corresponding event is needed in order to update UI modules as new keyboard stubs "come online".
this.legacyAPIEvents.callEvent('keyboardregistered', {
internalName: stub.KI,
Expand All @@ -275,8 +275,27 @@ export default class KeymanEngine<

// If this is the first stub loaded, set it as active.
if(this.config.activateFirstKeyboard && this.keyboardRequisitioner.cache.defaultStub == stub) {
// Note: leaving this out is super-useful for debugging issues that occur when no keyboard is active.
this.contextManager.activateKeyboard(stub.id, stub.langId, true);
// Note: this can trigger before the OSK is first built; such a
// situation can present a problem for debug-mode keyboards (as
// certain values are accessed through `keyman.osk`).
//
// Other areas also use the same filter function, so we create a distinct function
// unique to this location for our listener.
const errorFilterer = (event: ErrorEvent) => keyboardScriptErrorFilterer(event);
window.addEventListener('error', errorFilterer);

try {
// Note: leaving this out is super-useful for debugging issues that occur when no keyboard is active.
await this.contextManager.activateKeyboard(stub.id, stub.langId, true);
} catch (err) {
if(err instanceof KeyboardScriptError) {
this.config.deferForOsk.then(() => this.setActiveKeyboard(stub.id, stub.langId));
} else {
throw err;
}
} finally {
window.removeEventListener('error', errorFilterer);
}
}
}

Expand Down Expand Up @@ -360,6 +379,13 @@ export default class KeymanEngine<
}
value.on('keyevent', this.keyEventListener);
this.core.keyboardProcessor.layerStore.handler = value.layerChangeHandler;

// Trigger any deferred operations that require a present OSK.
// For example, debug-mode keyboards use APIs available through the OSK.
this.config.deferForOsk.resolve();
} else {
// If the OSK was cleared, we may need to restore deferment for debug-mode keyboards.
this.config.deferForOsk = new ManagedPromise<void>();
}
}

Expand Down
12 changes: 9 additions & 3 deletions web/src/engine/package-cache/src/stubAndKeyboardCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,16 @@ export default class StubAndKeyboardCache extends EventEmitter<EventMap> {
// Overrides the built-in ID in case of keyboard namespacing.
kbd.scriptObject["KI"] = keyboardID;
this.addKeyboard(kbd);
}).catch((err) => {
return kbd;
}).catch(() => {
delete this.keyboardTable[keyboardID];
throw err;
})
// Do NOT throw; the returned `promise` will throw the error as well;
// it will be handled via that pathway.
//
// Otherwise, we get duplicate errors... and become unable to silence
// the error here even if we're silencing it elsewhere (say, when
// trying to load a debug-mode keyboard too early).
});

return promise;
}
Expand Down
16 changes: 8 additions & 8 deletions web/src/test/manual/web/chirality/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -34,24 +34,24 @@
-->
<script src="../../../../../build/publish/debug/kmwuitoggle.js"></script>

<!-- Add keyboard management script for local selection of keyboards to use -->
<script src="./utilities.js"></script>

<!-- Initialization: set paths to keyboards, resources and fonts as required -->
<script>
var kmw=window.keyman;
kmw.init({
attachType: 'auto'
});
loadKeyboards();
keyman.init({
attachType: 'auto'
});
</script>

<!-- Add keyboard management script for local selection of keyboards to use -->
<script src="./utilities.js"></script>

<script>

</script>
</head>

<!-- Sample page HTML -->
<body onload='loadKeyboards();'>
<body>
<h2>KeymanWeb Sample Page - Chirality Testing</h2>
<p>This page is designed to stress-test the new support for chiral modifiers and state keys.</p>
<p>Be sure to reference the developer console for additional feedback. </p>
Expand Down
Loading
Loading