fix(android): differentiate between fontPath and fontUrl in Keyman Engine for Android by ermshiperete · Pull Request #16188 · keymanapp/keyman

ermshiperete · 2026-07-02T16:35:27Z

PR #16146 introduced a problem with selecting a different font as display font so that we always ended up with not setting the font. This was caused by the font filenames now being a URL (which is necessary because they get processed by the web engine). However, the Android code checks for the existence of the font in order to create the typeface, which only works for local paths.

This PR modifies and simplifies KMKeyboard.getFontFilename to return the full path, renames KMKeyboard.txtFont and KMKeyboard.oskFont to make it clearer that they contain a path and not a URL.

Also initialize KMKeyboard.oskFontPath with empty string instead of null. This makes it consistent with txtFontPath and with the documented behavior of KMManager.getKeyboardOskFontFilename() (which returns KMKeyboard.oskFontPath).

Follows: #16146
Replaces: #16178
Fixes: #16187
Build-bot: release:android

User Testing

TEST_KEYBOARDHARNESS:

install the KeyboardHarness apk from this PR
Switch to the longpress '"|5% + keyboard
verify that the OSK uses the Code2001 font (compare with screenshot below)
type gbqJ and verify that the Code2001 font is used (compare with screenshot)

TEST_KEYMAN:

install Keyman for Android apk
try different keyboards, verify that the OSK and displayed text look correct

TEST_FV:

install FirstVoices Keyboards for Android apk
try different keyboards, verify that the OSK and displayed text look correct

.

keymanapp-test-bot · 2026-07-02T16:35:53Z

User Test Results

Test specification and instructions

🟩 TEST_KEYBOARDHARNESS (SKIPPED): blocked because keyboardharness does not install
✅ TEST_KEYMAN (PASSED) (notes)
✅ TEST_FV (PASSED) (notes)

Test Artifacts

Android

PR #16146 introduced a problem with selecting a different font as display font so that we always ended up with not setting the font. This was caused by the font filenames now being a URL (which is necessary because they get processed by the web engine). However, the Android code checks for the existence of the font in order to create the typeface, which only works for local paths. This PR modifies and simplifies `KMKeyboard.getFontFilename` to return the full path, renames `KMKeyboard.txtFont` and `KMKeyboard.oskFont` to make it clearer that they contain a path and not a URL. Also initialize `KMKeyboard.oskFontPath` with empty string instead of `null`. This makes it consistent with` txtFontPath` and with the documented behavior of `KMManager.getKeyboardOskFontFilename()` (which returns `KMKeyboard.oskFontPath`). Follows: #16146 Fixes: #16187 Build-bot: release:android

mcdurdin · 2026-07-03T09:28:21Z

+  /**
+   * Returns the filename without path of the display font of the current keyboard.
+   */
+  private String getKeyboardTextFontFilename() {


Should we call this getKeyboardTextFontFilenameOnly() because of the legacy of KMManager.getKeyboardTextFontFilename() returning a full path?

mcdurdin · 2026-07-03T09:30:18Z

+    // REVIEW: Do we have to do anything if font is a JSONObject?
+    // See makeFontObj.
+


I don't really understand this question

In makeFontObj we have this questionable block of code that deals with a font as JSONObject. If that is still needed, then we'll likely have to do something here with a JSONObject as well.

Any suggestion for a better review or todo comment?

After some spelunking, came to the conclusion that the input parameter is always a filename, not a JSON object. So we can hopefully clean that up a bit.

mcdurdin · 2026-07-03T09:31:29Z

+      // REVIEW: Why do we need the complicated code below? Can this still
+      // happen, or can we remove it? (see also getFontFilename)
+      KMLog.LogInfo(TAG, "Got font without font extension: " + font);


Let's open an issue on this to follow up in the future once we have some logging. (Note that LogInfo doesn't go to Sentry though? Can we capture a Sentry event?)

LogInfo() calls Sentry.captureMessage, so I think it should end up on Sentry.

I think we concluded that this code path is not relevant, so we should clean up in a follow-up PR. We can run a set of user tests to verify that this doesn't break existing users.

Co-authored-by: Marc Durdin <marc@durdin.net> Co-authored-by: Eberhard Beilharz <ermshiperete@users.noreply.github.com>

This addresses a code review comment.

mcdurdin

LGTM, with some cleanup as a follow-up PR or two

mcdurdin · 2026-07-08T07:19:56Z

+    JSONObject jDisplayFont = makeFontObject(kFont, packageID);
+    JSONObject jOskFont = makeFontObject(kOskFont, packageID);

-    txtFont = getFontFilename(jDisplayFont);
-    oskFont = getFontFilename(jOskFont);
+    txtFontPath = getFontFilename(kFont, packageID);
+    oskFontPath = getFontFilename(kOskFont, packageID);


Let's try and make these font names consistent - we have font, displayfont, txtfont, oskfont, textfont -- but only two conceptual fonts -- TextFont and OskFont. For a follow-up cleanup PR.

mcdurdin · 2026-07-08T07:21:03Z

+  private String getPackageRootUrl(String packageID) {
+    if (packageID.equals(KMManager.KMDefault_UndefinedPackageID)) {
+      return getDataRootUrl() + KMManager.KMDefault_UndefinedPackageID + "/";
+    }
+    return getDataRootUrl() + KMManager.KMDefault_AssetPackages + "/" + packageID + "/";
+  }
+
+  private String getPackageRootPath(String packageID) {


Can we add comments on the difference between RootUrl and RootPath for future maintainers?

mcdurdin · 2026-07-08T07:23:37Z

+    if (FileUtils.hasFontExtension(font)) {
+      String fontRoot = KMManager.isDefaultFont(font) ? getDataRootPath() : getPackageRootPath(packageID);
+      return fontRoot + font;
    }


Can we reverse this, so it becomes a validation precondition instead?

Suggested change

if (FileUtils.hasFontExtension(font)) {

String fontRoot = KMManager.isDefaultFont(font) ? getDataRootPath() : getPackageRootPath(packageID);

return fontRoot + font;

}

if (!FileUtils.hasFontExtension(font)) {

// QUESTION: do we log this?

return "";

}

String fontRoot = KMManager.isDefaultFont(font) ? getDataRootPath() : getPackageRootPath(packageID);

return fontRoot + font;

mcdurdin · 2026-07-08T07:30:07Z

+    // REVIEW: Do we have to do anything if font is a JSONObject?
+    // See makeFontObj.
+


After some spelunking, came to the conclusion that the input parameter is always a filename, not a JSON object. So we can hopefully clean that up a bit.

mcdurdin · 2026-07-08T07:33:21Z

+      // REVIEW: Why do we need the complicated code below? Can this still
+      // happen, or can we remove it? (see also getFontFilename)
+      KMLog.LogInfo(TAG, "Got font without font extension: " + font);


I think we concluded that this code path is not relevant, so we should clean up in a follow-up PR. We can run a set of user tests to verify that this doesn't break existing users.

Meng-Heng · 2026-07-08T08:27:59Z

Test Specs

Keyman for Android 19.0.251-alpha-test-16188
Android Studio Pixel 9 Android v15.0
FirstVoice 19.0.251

Test Results

TEST_KEYMAN (PASSED):

Launch Keyman without enabling anything in startup menu
Go to Settings -> Install Keyboard or Dictionary -> Get:
- Khmer Angkor
- GFF Amharic
- EuroLatin (SIL) default
- Tamil Suratha Bamini
Return to Home screen
Verified: OSK, displayed text, typing, and applying predictive text work as expected, no error.
Verified: After enabling everything in the startup menu work, OSK and typing does seem to work well.
Verified: Compare keyboards to Keyman 18.0.249 doesn't seem to have any font dissimilarity.

TEST_FV (PASSED):

Launch FirstVoice and select keyboards:
- Dakelh
- EuroLatin (SIL)
Verified: OSK, font, and typing in Google Chrome work as expected.
Verified: Character on the OSK and typed text seems to be normal size and in the center.

Co-authored-by: Marc Durdin <marc@durdin.net>

ermshiperete · 2026-07-08T10:15:22Z

Follow-up-PR: #16211

This change renames the font variable names so that they match the two conceptual fonts we have. Also remove obsolete code that dealt with the `font` parameter being a JSON object instead of a string containing the font filename. Having the `font` parameter containing a JSON object was legacy code that won't happen with current Keyman versions. Follow-up-of: #16188 Build-bot: release:android

mcdurdin · 2026-07-08T12:02:11Z

TEST_KEYBOARDHARNESS SKIP blocked because keyboardharness does not install

This change renames the font variable names so that they match the two conceptual fonts we have. Also remove obsolete code that dealt with the `font` parameter being a JSON object instead of a string containing the font filename. Having the `font` parameter containing a JSON object was legacy code that won't happen with current Keyman versions. Follow-up-of: #16188 Build-bot: release:android

keyman-server · 2026-07-08T18:09:17Z

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

chore(android): cleanup font variable names, remove obsolete code This change renames the font variable names so that they match the two conceptual fonts we have. Also remove obsolete code that dealt with the `font` parameter being a JSON object instead of a string containing the font filename. Having the `font` parameter containing a JSON object was legacy code that won't happen with current Keyman versions. Follow-up-of: #16188

github-project-automation Bot moved this to Todo in Keyman Jul 2, 2026

github-project-automation Bot added this to Keyman Jul 2, 2026

github-actions Bot added android/ android/engine/ android/app/ fix labels Jul 2, 2026

keymanapp-test-bot Bot added the user-test-missing User tests have not yet been defined for the PR label Jul 2, 2026

keymanapp-test-bot Bot added this to the A19S32 milestone Jul 2, 2026

keymanapp-test-bot Bot added has-user-test user-test-required User tests have not been completed and removed user-test-missing User tests have not yet been defined for the PR labels Jul 2, 2026

ermshiperete force-pushed the fix/android/16187_font branch 2 times, most recently from 92edddc to a26d081 Compare July 2, 2026 17:49

ermshiperete requested review from jahorton and mcdurdin July 2, 2026 17:49

ermshiperete force-pushed the fix/android/16187_font branch from a26d081 to 1191e43 Compare July 2, 2026 17:53

ermshiperete marked this pull request as ready for review July 2, 2026 17:54

mcdurdin reviewed Jul 3, 2026

View reviewed changes

ermshiperete and others added 2 commits July 3, 2026 17:23

Apply suggestions from code review

1867a76

Co-authored-by: Marc Durdin <marc@durdin.net> Co-authored-by: Eberhard Beilharz <ermshiperete@users.noreply.github.com>

fix(android): rename method to getKeyboardTextFontFilenameOnly

70282bf

This addresses a code review comment.

keyman-server modified the milestones: A19S32, A19S33 Jul 4, 2026

mcdurdin approved these changes Jul 8, 2026

View reviewed changes

mcdurdin changed the title ~~fix(android): fix setting font~~ fix(android): differentiate between fontPath and fontUrl in Keyman Engine for Android Jul 8, 2026

Meng-Heng self-assigned this Jul 8, 2026

fix(android): address code review comments

71aefb8

Co-authored-by: Marc Durdin <marc@durdin.net>

ermshiperete mentioned this pull request Jul 8, 2026

chore(android): cleanup font variable names, remove obsolete code #16211

Merged

keymanapp-test-bot Bot removed the user-test-required User tests have not been completed label Jul 8, 2026

ermshiperete merged commit fcf8988 into master Jul 8, 2026
7 checks passed

ermshiperete deleted the fix/android/16187_font branch July 8, 2026 12:35

github-project-automation Bot moved this from Todo to Done in Keyman Jul 8, 2026

		// REVIEW: Do we have to do anything if font is a JSONObject?
		// See makeFontObj.

Uh oh!

Uh oh!

Conversation

ermshiperete commented Jul 2, 2026 • edited by mcdurdin Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User Testing

Uh oh!

keymanapp-test-bot Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User Test Results

Test Artifacts

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mcdurdin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Meng-Heng commented Jul 8, 2026

Test Specs

Test Results

Uh oh!

ermshiperete commented Jul 8, 2026

Uh oh!

mcdurdin commented Jul 8, 2026

Uh oh!

Uh oh!

keyman-server commented Jul 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ermshiperete commented Jul 2, 2026 •

edited by mcdurdin

Loading

keymanapp-test-bot Bot commented Jul 2, 2026 •

edited

Loading