keymanapp / keyman

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

bug(ios): no keyboard set during embedded engine init #9227

Open jahorton opened 1 year ago

jahorton commented 1 year ago

So this was use-before-init? Just making sure that the root cause of the warning can't be addressed. No problem eliminating the warning but unnecessary calls are ... unnecessary!

I believe so, but I'm not 100% sure. I could try to investigate more thoroughly in the future, if we'd prefer that.

Thinking a little bit more on it... I wonder if the iOS engine is just setting OSK height "later" than the Android app does. If it were set 100% in advance, we wouldn't get the warning then, I believe. Then again... our Android dev is on break at the moment, thus he's probably not running the latest alpha - which could be the only reason we're not seeing the error from Android too.

So, a little more info is now available to us thanks to #9217:

{
  configReport: {
    embeddingApp: AppleMobile, 
    hostDevice: {
      browser: native, 
      formFactor: phone, 
      OS: ios, 
      touchable: True
    }, 
    initialized: False, 
    keymanEngine: app/webview
  }, 
  keyboard: {
    id: '', 
    langId: '', 
    version: ''
  }, 
  model: {
    id: ''
  }, 
  osk: {
    banner: blank, 
    layer: default
  }
}

Therefore, we may assume that this is occurring when there's no loaded keyboard yet. This smells to me like an old warning from previous versions that we eventually silenced:

https://github.com/keymanapp/keyman/blob/3148aa7b8c8a4b5de574d49bdf6edc80dcb6761a/web/source/kmwdom.ts#L1495-L1497

(12.0 permalink, but it was finally removed in 15.0 stable / 16.0 alpha by #6890.)

The logic 'gatekeeping' the warning is a bit different, but the underlying state that's triggering it seems to match pretty well.

Originally posted by @jahorton in https://github.com/keymanapp/keyman/issues/9206#issuecomment-1627917615


Additional info:

At the time of the original PR, simply loading up the Keyman app, changing the keyboard once, and rotating the mobile device from portrait to landscape, then back - just once - was enough to reliably trigger the error. It may not have even needed that much.

There is a slight flash (flash-of-unstyled-content style) of an improperly-laid-out keyboard during the init that's visible. It's only about a frame, and it seemed to happen just once, but it's as if the keyboard was loading before the size information was properly registered.

This may also provide some hints toward the long-running keyboard-load flash-of-unstyled-content we see in the Android app.

sentry-io[bot] commented 1 year ago

Sentry issue: KEYMAN-WEB-CK

Upon looking into the stacktrace with more detail... the OSK is running into this while initializing. It tries to present itself automatically.

Of course, in the mobile apps, there are hooks that allow the host app to specify the OSK size, rather than having Web compute it. If they're not set in advance... that'd be where we run into this. Sounds like size is being set after the OSK initializes and not before.

jahorton commented 5 months ago

Significant recent work toward this has occurred with #11174. It's not quite enough to fully resolve the issue for iOS, it seems - I can still see the slightest bit of a flash indicating that an empty OSK exists, even if for just a fraction of a second, before the proper keyboard is loaded and displayed.

The PR is related enough, so if nothing else, I did want to link it here.