modify(android/engine): Stop logging if keyboard index not found

[darcywong00]

Fixes #6703

Thanks to @jahorton for a thoroughly looking into the issue.

To my chagrin, I've found the valid use case of KeyboardController not finding a language in the keyboard list In displaying some language lists, it's expected for a languageID not to be found. (e.g. "Add languages to installed keyboard" menu where it's filtering for languages not already installed)

So this PR also takes @jahorton's suggestion about checking equalsIgnoresCase, along with disabling the Sentry warning.

User Testing

Setup - Load the PR build of Keyman for Android

• TEST_LANGUAGE_NOT_FOUND

1. Launch the PR build of Keyman for Android
2. Using the Keyman Settings menu, search keyman.com and install sil_cameroon_qwerty keyboard for any language.
3. When the keyboard
4. package finishes installing, observe the OSK is switched to sil_cameroon_qwerty
5. In Keyman Settings --> Install Keyboard or Dictionary --> Add languages to installed keyboard --> sil_cameroon_qwerty
6. Select another language and click "INSTALL"
7. Verify the language is added

[keymanapp-test-bot[bot]]

User Test Results

Test specification and instructions

• ✅ TEST_LANGUAGE_NOT_FOUND (PASSED): Tested this with the attached PR build (Keyman 16.0.76-alpha-test-7415) in API 26 / Android 8.0 emulator and I verified that another cameroon_qwerty keyboard is added (with a message Added language Babanki to Cameroon Qwerty) without showing any error message. (notes)

Test Artifacts

• Android
  • Keyman for Android apk
  • FirstVoices Keyboards for Android apk

[darcywong00]

So is the equalsIgnoreCase change fixing a known issue?

@jahorton suggested the change in case previous bugs left mixed cases in the language list. This would help ensure we do find the matching keyboard. The description from an earlier fix #7239:

This causes the temporary language list to contain duplicates of the same language (e.g. bkc-latn and bkc-Latn).

[jahorton]

LGTM. So is the equalsIgnoreCase change fixing a known issue?

https://github.com/keymanapp/keyman/issues/6703#issuecomment-1267949317 is what motivated the change you're asking about.

[darcywong00]

Okay, so we aren't removing the log message in getCurrentKeyboardIndex

Ah, now that you point it out, maybe we should...

[jahorton]

Okay, so we aren't removing the log message in getCurrentKeyboardIndex

Ah, now that you point it out, maybe we should...

Not until we've confirmed #6703 dead. At which point, we wouldn't be getting log messages about it, anyway. It's a decent enough "canary" for that.

[darcywong00]

Maybe I need to analyze this more :/ . The Sentry stack trace shows the issue happens on SystemKeyboard onStartInput

[jahorton]

Maybe I need to analyze this more :/ . The Sentry stack trace shows the issue happens on SystemKeyboard onStartInput

Yep. I realize that, and this was considered as part of the instigating comment's suggestion. It's part of the long comment history on #6703.

[mcdurdin]

I think it's unlikely that the keyboardid or packageid (aka keyboard and package filenames) would have mixed case -- there might be rare cases where that would happen in a dev scenario, but all keyboards and packages hosted on Keyman Cloud will have lower case IDs, and the compiler complains about mixed case for these ids.

[darcywong00]

I've separated this PR from issue #6703, but we can still proceed with removing the logging if keyboard index not found

[bharanidharanj]

• TEST_LANGUAGE_NOT_FOUND (PASSED): Tested this with the attached PR build (Keyman 16.0.76-alpha-test-7415) in API 26 / Android 8.0 emulator and I verified that another cameroon_qwerty keyboard is added (with a message Added language Babanki to Cameroon Qwerty) without showing any error message.

  

[keyman-server]

Changes in this pull request will be available for download in Keyman version 16.0.84-alpha

[jahorton]

I think it's unlikely that the keyboardid or packageid (aka keyboard and package filenames) would have mixed case -- there might be rare cases where that would happen in a dev scenario, but all keyboards and packages hosted on Keyman Cloud will have lower case IDs, and the compiler complains about mixed case for these ids.

Found something interesting:

https://sentry.io/share/issue/bed2060d7ba24e44854b8f70078d6229/

We have proof of at least one wild mixed-case scenario, unless my interpretation's off somehow?

... noting the quoted bit from above... what checks do we have in place for adhoc installations?

[mcdurdin]

We have proof of at least one wild mixed-case scenario, unless my interpretation's off somehow?

On that example:

1. I doubt it is an ad-hoc install -- it's a keyboard from the repo.

2. The package id arabic_izza and the keyboard id arabic_izza are both lower case

3. Only the BCP 47 tag (which is from package metadata) is mixed case:

     <Languages>
       <Language ID="ar-DZ">ar-DZ</Language>
     </Languages>
   

   The BCP 47 tag is correctly formatted. I note that the language name is incorrect but that is immaterial here.