epic: web-core 🎼 #12291

mcdurdin · 2024-08-27T02:17:08Z

Adds support for Keyman Core (incl. LDML keyboards) to KeymanWeb, bringing LDML keyboards to Android, iOS, and web platforms

References

Seed PR:

feat(web): POC of Core WASM integration into Keyman Web #12234

@keymanapp-test-bot skip

`emcc.py` is not marked as executable in emcripten's git repo so the build failed when trying to locate emscripten. However, it turns out that `emcc` is marked as executable, so we use that instead. On Windows however, we still need to use `emcc.py` because otherwise Meson won't detect it as valid compiler.

- add temporary function to Core for this POC - add new CoreProcessor to access Keyman Core WASM - add unit tests for new core processor - add code to KeymanEngine and InputProcessor to load the new CoreProcessor - add web server to manual tests and new action `start` to build script This change requires the manual tests to be loaded from a web server instead of loaded as file, because otherwise the wasm code won't be loaded. Currently we always load CoreProcessor. This should be improved in a future change to only load when it is actually needed. Part-of: #11293

…core chore: merge master into web-core

Co-authored-by: Marc Durdin <[email protected]>

feat(web): POC of Core WASM integration into Keyman Web

keymanapp-test-bot · 2024-08-27T02:17:32Z

User Test Results

Test specification and instructions

User tests are not required

Test Artifacts

Android
Developer
iOS
- Keyman for iOS (simulator image)
- FirstVoices Keyboards for iOS (simulator image)
- TestFlight internal PR build version - 19.0.10 (0.12291.13017)
Keyboards
- Test Keyboards
Linux
macOS
- Keyman for macOS
Web
- KeymanWeb Test Home
Windows

- loading a keyboard from a BLOB - getting the on-screen keyboard layout from Core. This is an internal- only API because of it's use of C++. Part-of: #11293 Part-of: #8093

…core chore: merge master into web-core 🎼

- split keyboard loading into loading into blob and then loading the script - look at first four bytes to see if it's a .js or a .kmx keyboard - for domKeyboardLoader, use fetch to get the blob, then use indirect eval to load the script, instead of injecting a script element. This does not yet implement the loading of .kmx keyboards.

srl295

Very nice!

Addresses code review comments.

mcdurdin · 2025-03-01T10:25:37Z

web/src/app/ui/kmwuifloat.ts

Let's cherry-pick the async changes in ui back to master

mcdurdin · 2025-03-01T10:38:49Z

web/src/app/browser/src/keymanEngine.ts

-    this.keyboardRequisitioner.cloudQueryEngine.once('unboundregister', () => {
+    this.keyboardRequisitioner.cloudQueryEngine.once('unboundregister', async () => {
      if(!this.contextManager.activeKeyboard?.keyboard) {
        // Autoselects this.keyboardRequisitioner.cache.defaultStub, which will be
        // set to an actual keyboard on mobile devices.
-        this.setActiveKeyboard('', '');
+        await this.setActiveKeyboard('', '');


We don't technically need to await here because this is a callback which has no other action after the call to this.setActiveKeyboard. But we should consider code hygiene around await because it is far too easy to miss an important await -- perhaps eslint can help us with this?

mcdurdin · 2025-03-01T10:40:07Z

web/src/app/browser/src/keymanEngine.ts

+    // TODO-web-core: implement for KMX keyboards if needed
+    return kbd && kbd instanceof JSKeyboard && kbd.isCJK;


Suggested change

// TODO-web-core: implement for KMX keyboards if needed

return kbd && kbd instanceof JSKeyboard && kbd.isCJK;

// We only support isCJK on legacy .js keyboards; see #7928

return kbd && kbd instanceof JSKeyboard && kbd.isCJK;

mcdurdin · 2025-03-01T10:44:01Z

web/src/engine/core-processor/src/index.ts

@@ -0,0 +1,3 @@
+export { CoreFactory, KM_CORE_STATUS } from './core-factory.js';
+import { type MainModule, type km_core_keyboard, type CoreKeyboardReturn } from './import/core/keymancore.js';
+export { MainModule, km_core_keyboard, CoreKeyboardReturn };


Suggested change

export { MainModule, km_core_keyboard, CoreKeyboardReturn };

export { MainModule as CoreMainModule, km_core_keyboard, CoreKeyboardReturn };

optional but may help for clarity elsewhere

mcdurdin · 2025-03-01T10:47:53Z

web/src/engine/keyboard/src/keyboards/keyboardLoaderBase.ts

+      const result = (await this._km_core).keyboard_load_from_blob(uri, byteArray);
+      if (result.status == KM_CORE_STATUS.OK) {
+        // extract keyboard name from URI
+        const id = uri.split('#')[0].split('?')[0].split('/').pop().split('.')[0];


I suggest refactoring this into a separate function

mcdurdin · 2025-03-01T10:48:09Z

web/src/engine/keyboard/src/keyboards/keyboardLoaderBase.ts

+  private async loadKeyboardInternal(uri: string, errorBuilder: KeyboardLoadErrorBuilder): Promise<Keyboard> {
+    const byteArray = await this.loadKeyboardBlob(uri, errorBuilder);
+
+    if (byteArray.slice(0, 4) == Uint8Array.from([0x4b, 0x58, 0x54, 0x53])) { // 'KXTS'


I suggest a helper function for this test

mcdurdin · 2025-03-01T10:49:57Z

web/src/engine/keyboard/src/keyboards/jsKeyboard.ts

@@ -52,7 +52,7 @@ type KmwKeyboardObject = KeyboardObject & {
 * and keyboard-centered functionality in an object-oriented way without modifying the
 * wrapped keyboard itself.
 */
-export default class Keyboard {
+export class JSKeyboard {


It would be good for this and KMXKeyboard to inherit from an abstract base class.

mcdurdin · 2025-03-01T10:50:15Z

web/src/engine/keyboard/src/keyboards/kmxKeyboard.ts

+/**
+ * Acts as a wrapper class for KMX(+) Keyman keyboards
+ */
+export class KMXKeyboard {


Abstract base class with JSKeyboard

mcdurdin · 2025-03-01T10:51:58Z

web/src/engine/keyboard/src/keyboards/loaders/domKeyboardLoader.ts

+    let buffer: ArrayBuffer;
+    try {
+      buffer = await response.arrayBuffer();
+    } catch (e) {
+      throw errorBuilder.invalidKeyboard(e);
+    }


Suggested change

let buffer: ArrayBuffer;

try {

buffer = await response.arrayBuffer();

} catch (e) {

throw errorBuilder.invalidKeyboard(e);

}

const buffer: ArrayBuffer = await (async () => {

try {

return await response.arrayBuffer();

} catch (e) {

throw errorBuilder.invalidKeyboard(e);

})();

Is this nicer? Hmm, it might be uglier.

mcdurdin · 2025-03-01T10:59:54Z

web/src/engine/main/src/keymanEngine.ts

Why do we have three files all named keymanEngine.ts?

mcdurdin · 2025-03-01T11:00:30Z

web/src/engine/main/src/keymanEngine.ts

          throw new Error(`Keyboard '${k0}' has not been loaded.`);
+        } else if (!(kbdObj instanceof JSKeyboard)) {


Suggested change

} else if (!(kbdObj instanceof JSKeyboard)) {

} else if (kbdObj instanceof KMXKeyboard) {

easier to read

…core chore: merge master into web-core 🎼

feat(web): implement getting properties of KMXKeyboard 🎼

feat(core): expose state and event API to WASM 🎼

- rename `js-processor`'s `OutputTarget` → `OutputTargetBase` - rename `element-wrappers`' `OutputTarget` → `OutputTargetElementWrapper`

Addresses code review comment.

feat(core): expose context and actions APIs to WASM 🎼

…gine refactor(web): rename base `KeymanEngine` to `KeymanEngineBase` 🎼

…arget refactor(web): cleanup OutputTarget class names 🎼

ermshiperete and others added 11 commits August 20, 2024 22:45

feat(web): don't use deprecated emcc parameter

f6c8229

chore(developer): support for emscripten 3.1.64

3912bf4

Merge pull request #12257 from keymanapp/chore/merge-master-into-web-…

8a9bec2

…core chore: merge master into web-core

Merge branch 'epic/web-core' into feat/web/core

1a8d8a7

chore(web): add utils dependency to core-processor (es-bundling)

e015072

chore(common): use npm install and set min ver for emsdk

c021999

Apply suggestions from code review

13f052f

Co-authored-by: Marc Durdin <[email protected]>

chore(core): address code review comments

eb33c53

Merge pull request #12234 from keymanapp/feat/web/core

a0eecab

feat(web): POC of Core WASM integration into Keyman Web

mcdurdin added this to the A18S9 milestone Aug 27, 2024

keymanapp-test-bot bot added the epic-web-core label Aug 27, 2024

mcdurdin added web/ core/ Keyman Core epic A long lived branch, home for a new feature, usually will have child PRs based on it labels Aug 27, 2024

mcdurdin changed the title ~~epic: web-core~~ epic: web-core 🎼 Aug 27, 2024

ermshiperete and others added 2 commits August 30, 2024 14:44

feat(core): spec out API extensions

1deaa32

- loading a keyboard from a BLOB - getting the on-screen keyboard layout from Core. This is an internal- only API because of it's use of C++. Part-of: #11293 Part-of: #8093

Merge branch 'epic/web-core' into chore/merge-master-into-web-core

165829b

srl295 self-requested a review August 30, 2024 17:28

darcywong00 modified the milestones: A18S9, A18S10 Aug 31, 2024

Merge pull request #12322 from keymanapp/chore/merge-master-into-web-…

68566af

…core chore: merge master into web-core 🎼

github-actions bot added common/ common/resources/ Build infrastructure docs labels Aug 31, 2024

srl295 approved these changes Sep 1, 2024

View reviewed changes

feat(linux): add blob size parameter

59019cc

Addresses code review comments.

darcywong00 added this to the B18S2 milestone Feb 15, 2025

darcywong00 modified the milestones: B18S2, B18S3 Feb 28, 2025

mcdurdin commented Mar 1, 2025

View reviewed changes

web/src/app/ui/kmwuifloat.ts Outdated

Copy link

Member Author

mcdurdin Mar 1, 2025

Choose a reason for hiding this comment

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

Let's cherry-pick the async changes in ui back to master

mcdurdin commented Mar 1, 2025

View reviewed changes

web/src/engine/main/src/keymanEngine.ts Outdated

Copy link

Member Author

mcdurdin Mar 1, 2025

Choose a reason for hiding this comment

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

Why do we have three files all named keymanEngine.ts?

mcdurdin commented Mar 1, 2025

View reviewed changes

mcdurdin and others added 10 commits March 1, 2025 18:10

Merge branch 'epic/web-core' into chore/merge-master-into-web-core

78e8e45

feat(web): implement getting properties of KMXKeyboard

fe3e02c

Merge pull request #13390 from keymanapp/chore/merge-master-into-web-…

09a0f05

…core chore: merge master into web-core 🎼

feat(web): fix tests

8e8816a

feat(core): expose state and event API to WASM

c297c66

feat(core): expose context and actions APIs to WASM

b2cccdd

refactor(web): rename base KeymanEngine to KeymanEngineBase

bd1c610

Merge pull request #13392 from keymanapp/feat/web/webcore

0fc9b20

feat(web): implement getting properties of KMXKeyboard 🎼

Merge pull request #13393 from keymanapp/feat/web/state

7c50646

feat(core): expose state and event API to WASM 🎼

refactor(web): cleanup OutputTarget class names

3b2157c

- rename `js-processor`'s `OutputTarget` → `OutputTargetBase` - rename `element-wrappers`' `OutputTarget` → `OutputTargetElementWrapper`

srl295 mentioned this pull request Mar 4, 2025

bug(core): uset wrongly aligned in kmx+ 🎼 #13419

Open

ermshiperete and others added 5 commits March 5, 2025 18:05

refactor(core): address code review comment

5c7030f

docs(web): update parameter name in function headers

626a43f

Addresses code review comment.

Merge pull request #13400 from keymanapp/feat/web/context+actions

36dd449

feat(core): expose context and actions APIs to WASM 🎼

Merge pull request #13404 from keymanapp/refactor/web/rename-keymanen…

19926d2

…gine refactor(web): rename base `KeymanEngine` to `KeymanEngineBase` 🎼

Merge pull request #13406 from keymanapp/refactor/web/cleanup-outputT…

acd7cbf

…arget refactor(web): cleanup OutputTarget class names 🎼

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

epic: web-core 🎼 #12291

epic: web-core 🎼 #12291

mcdurdin commented Aug 27, 2024 •

edited

Loading

keymanapp-test-bot bot commented Aug 27, 2024 •

edited

Loading

srl295 left a comment

mcdurdin Mar 1, 2025

mcdurdin Mar 1, 2025

mcdurdin Mar 1, 2025

mcdurdin Mar 1, 2025

mcdurdin Mar 1, 2025

mcdurdin Mar 1, 2025

mcdurdin Mar 1, 2025

mcdurdin Mar 1, 2025

mcdurdin Mar 1, 2025

mcdurdin Mar 1, 2025

mcdurdin Mar 1, 2025

		// TODO-web-core: implement for KMX keyboards if needed
		return kbd && kbd instanceof JSKeyboard && kbd.isCJK;

	export { MainModule, km_core_keyboard, CoreKeyboardReturn };
	export { MainModule as CoreMainModule, km_core_keyboard, CoreKeyboardReturn };

		throw new Error(`Keyboard '${k0}' has not been loaded.`);
		} else if (!(kbdObj instanceof JSKeyboard)) {

	} else if (!(kbdObj instanceof JSKeyboard)) {
	} else if (kbdObj instanceof KMXKeyboard) {

epic: web-core 🎼 #12291

Are you sure you want to change the base?

epic: web-core 🎼 #12291

Conversation

mcdurdin commented Aug 27, 2024 • edited Loading

References

keymanapp-test-bot bot commented Aug 27, 2024 • edited Loading

User Test Results

Test Artifacts

srl295 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcdurdin commented Aug 27, 2024 •

edited

Loading

keymanapp-test-bot bot commented Aug 27, 2024 •

edited

Loading