fix(android): Allow external keyboard to work when OSK hidden
Fixes #12366 Reverts part of #7612 which had changed where the Javascript handler gets attached
User Testing
Setup - Install the PR build of Keyman for Android on a physical device. Also pair an external keyboard (USB/bluetooth) to the device
- TEST_KEYBOARD - Verifies external keyboard works when virtual keyboard hidden
- Launch Keyman
- From "Get Started", enable Keyman as the default system keyboard
- Launch a separate app where you can type in (ideally a note taking app like Keep or OneNote as reported in the issue). If those aren't available, Chrome browser works
- Select a text area with Keyman sil_euro_latin selected as the system keyboard
- Verify the system keyboard types fine (longpresses, switching, keys, etc).
- With the virtual keyboard displayed, type on the physical keyboard ` a
- Verify the text becomes
à. - From the Android system preferences, select "Language and Keyboard" --> Physical keyboard --> Toggle "show on-screen keyboard" so that the OSK is hidden
- Return to the note-taking / Chrome app and select a text field
- Type on the physical keyboard ` a
- Verify the text becomes
à
I am uncertain about the ramifications of this change. This means the
Runnableruns on a new thread, not the UI thread, which seems to go against the discussion in the previously linked SO post. Can you elucidate?
I was just returning the call to how it had been for stable-15.0
https://github.com/keymanapp/keyman/blob/2dcc11448ee6e715fcfbbbaf6701a29f449e5817/android/KMEA/app/src/main/java/com/tavultesoft/kmea/KMKeyboard.java#L253-L272
I was just returning the call to how it had been for stable-15.0
OK, so this was changed because of reported errors -- see https://github.com/keymanapp/keyman/pull/7612#discussion_r1024688563:
True. Note that I placed it here because I saw a corresponding error occur once during development. Thought that was interesting, since "shouldn't the code be doing this already?"
I would like to understand why this is causing the event handlers not to attach -- is it because there is no UI thread when the keyboard is hidden?
Test Results
I tested this issue with the attached "Keyman 18.0.108-alpha-test-12381" build on the Android 14 mobile (physical device and keyboard connected with USB cable). I am sharing my observation.
TEST_KEYBOARD (Passed):
- Installed the "Keyman-18.0.108.apk" file from the PR build. and gave all permissions to the application.
- Checked the "Enable Keyman as system-wide keyboard" and set the keyboard as the default keyboard box on the settings page.
- Open the Keyman app. Enable the "Predictions" to install the "Dictionary."
- Launch the Google Keep application.
- Create a new note. Keyman "sil_euro_latin" was selected as the system keyboard.
- Typed some text using the "sil_euro_latin" keyboard.
- Verified the system keyboard types fine when doing long presses, switching, keys, and pull-down.
- Type `a in the physical keyboard, and OSK appears at the same time.
- Verified the text appeared as à.
- Disabled the OSK by selecting "Language and Keyboard" --> Physical keyboard --> Toggle "show the on-screen keyboard."
- Again, launch the Google Keep application.
- Create a new note.
- Verified that the OSK does not appear in the Android mobile app.
- Type `a in the physical keyboard.
- Verified the text appeared as à. It works well using the physical keyboard if OSK is enabled or disabled. I tested this same scenario in the "https://editpad.org/" on the Chrome browser. It works great for me.
is it because there is no UI thread when the keyboard is hidden?
That's what seems to be the behavior
When the OSK is hidden, executing hardware keystrokes never gets into this handler on the right. And I see the javascriptAfterLoad queue keeps accumulating the Javascript calls.
When the OSK is shown again, all the Javascript calls execute and the text updates.
This reversion may appear to work but it is not thread safe -- which is probably why it was changed in the first place. Calling this.postDelayed() is scheduling it to run on the UI thread (per the documentation). But calling new Handler().postDelayed() schedules it to run on a new thread.
- I guess this could cause the error
"All WebView methods must be called on the same thread."(per the Stack Overflow discussion). Because now we are calling WebView methods from the wrong thread, right? (We'll only see it happen ifjavascriptAfterLoad.size() > 0. - In the runner, we are referencing parent object member variables that could be being modified by another thread, e.g.
javascriptAfterLoad,keyboardSet. This will likely lead to thread races -- access to these members needs to be sychronized. - Also all calls to member functions are on the wrong thread, as they expect to be called on the UI thread, e.g.
loadUrl,callJavascriptAfterLoad. - It is possible, albeit probably unlikely, that the runner could execute after the parent object is destroyed, resulting in a crash.
So we need to find a better way to solve this.
Replaced by #12871