keymanapp / keyman

Keyman cross platform input methods system running on Android, iOS, Linux, macOS, Windows and mobile and desktop web
https://keyman.com/
Other
386 stars 107 forks source link

bug(web): Console error reading `modifierCodes` of undefined after installing custom Greek keyboard package #11962

Closed darcywong00 closed 4 weeks ago

darcywong00 commented 1 month ago

A user reports on the Community site forum experiencing Toast errors in Keyman for Android 18 after installing her Greek keyboard package.

She doesn't experience the error on iOS. Keyboard functions fine though.

I can repro on the current master build on my Android 13 device.

Chrome console errors

image

other-polyfills.js:1
Failed to load resource: net::ERR_FILE_NOT_FOUND 

keymangr_all.js:9 
Uncaught 
TypeError: Cannot read properties of undefined (reading 'modifierCodes')
    at new Keyboard_keymangr_all (keymangr_all.js:9:29)
    at keymangr_all.js:5:14

keyboardLoaderBase.ts:64 
Uncaught (in promise) 
Error: Error registering the keymangreek keyboard for Modern Greek (1453-); keyboard script at /data/user/0/com.tavultesoft.kmapro.debug/app_data/packages/keymangr_all/keymangr_all.js may contain an error.
    at _StubBasedErrorBuilder.scriptError (keyboardLoaderBase.ts:64:12)
    at script.onload (domKeyboardLoader.ts:58:39)

sentry.min.js:2 
range unchanged
jahorton commented 1 month ago

OK, I can see what's happening here. Something very important to note - the package provided to us was compiled with "debug mode" active.

In #11174, we added functionality to streamline the loading process for the initial keyboard; part of this includes loading it before the OSK is available so that we don't double-build the OSK. ("Double-build": we otherwise build a "blank, default" keyboard that we immediately throw out in favor of the "correct" keyboard.) Of course, by doing this... there is no existing osk instance available to provide the debug API points referenced by debug-mode keyboards during that load. Looks like I neglected that possibility when putting #11174 together.

The easiest workaround would be to temporarily "mock" the OSK during that load.

For the mobile apps, this would be the major codeblock in need of keyman.osk mocking:

https://github.com/keymanapp/keyman/blob/d705589726a1fb553e7a8b4daf8b92617fc4a67d/web/src/app/webview/src/keymanEngine.ts#L84-L92

For app/browser mode, we do attempt something similar based on user cookies here:

https://github.com/keymanapp/keyman/blob/d705589726a1fb553e7a8b4daf8b92617fc4a67d/web/src/app/browser/src/keymanEngine.ts#L227-L233

Establishing a simple mock before the fetch / restore calls and clearing it immediately after the Promise resolves is probably the simplest way forward, even if it's a bit messy. That's as far as we can minimize mock exposure there, I believe.

jahorton commented 1 month ago

Looking at it a bit more... the reason that the keyboard is working for the user, without any trouble aside from the error message, is that both the codepaths mentioned above allow the main engine initialization to proceed anyway. Once the OSK is built, it will (already) re-attempt the load due to how the app / web-engine interface is structured; this latter attempt succeeds without issue.

jahorton commented 1 month ago

I don't think the mechanism described above exists within app/browser, so we should probably add recovery code anyway.

I think the best way to silence this error message would likely be via window.addEventListener('error').

jahorton commented 1 month ago

Took quite a while to chase down all the tendrils, but I've figured out the places that need handling if we want to simply 'silence' the error without mocking the OSK. (There were a few gaps in the promise chaining that caused one of the errors to be repeatedly reported.) More work will be needed to properly 'filter' for this specific case, though.

mcdurdin commented 1 month ago

Source generated by the keyboard compiler when debug build is enabled:

  ...
  var modCodes = keyman.osk.modifierCodes;
  var keyCodes = keyman.osk.keyCodes;
  ...
mcdurdin commented 1 month ago

Another report received via Slack, this one impacting the Keyman Developer Debugger, 17.0.327.

image

Secondary issue also visible in that screenshot, see #12038, which fully breaks kmw beforehand anyway.