Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(android/app): Cleanup AndroidManifest handling of *.kmp #10624

Merged
merged 1 commit into from
Feb 6, 2024
Merged

fix(android/app): Cleanup AndroidManifest handling of *.kmp #10624

merged 1 commit into from
Feb 6, 2024

Conversation

darcywong00
Copy link
Contributor

Fixes #10095
This PR cleans up the AndroidManifest.xml file for associating .kmp files with Keyman.

The Play Store was flagging an error:

tag failed
Add a "/" to the beginning of the android:path attribute in the tag. The attribute might also be android:pathPrefix or android:pathPattern.

Additional AndroidManifest.xml cleanup

  • Remove the ugly Android pattern matcher
  • Add mimeType x-keyman-package which matches the mimeType from curl -I https://downloads.keyman.com/keyboards/fv_all/12.6/fv_all.kmp

Things to address on separate PR's:

User Testing

Verifies that kmp files can be downloaded and installed with Keyman for Android

Setup - On an Android device/emulator, install the PR build of Keyman for Android. Note the limitations above. Do not use the following:

  • Samsung device

  • device/emulator of Android 13.0+ (SDK 33+)

  • TEST_KEYMAN_SEARCH - Verifies kmp files install from Keyman search

  1. Launch Keyman for Android and dismiss the "Get Started" menu
  2. In Keyman settings, Install a keyboard from keyman.com (e.g. khmer_angkor)
  3. Verify khmer_angkor.kmp is downloaded and installed
  • TEST_DOWNLOADS - Verifies kmp files can be installed from Chrome Downloads
  1. Launch Chrome
  2. From Chrome, download sil_cameroon_qwerty from https://downloads.keyman.com/keyboards/sil_cameroon_qwerty/6.1.0/sil_cameroon_qwerty.kmp
  3. When sil_cameroon_qwerty.kmp is done downloading, go to Chrome --> Downloads
  4. From the Downloads app, select sil_cameroon_qwerty.kmp
  5. Verify keyboard install wizard appears (you may be prompted to first choose Keyman to handle kmp)
  6. Complete the keyboard installation
  7. Verify sil_cameroon_qwerty is installed.

@keymanapp-test-bot keymanapp-test-bot bot added has-user-test user-test-required User tests have not been completed labels Feb 5, 2024
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Feb 5, 2024 •
edited
Loading

User Test Results

Test specification and instructions

  • ✅ TEST_KEYMAN_SEARCH (PASSED): Tested with the attached PR build (version 17.0.261-alpha-test-10624) in the Android mobile emulator (Android 12 / API 31) and here is my observation: 1. Opened Keyman In-App. 2. Clicked Settings / Install Keyboard or Dictionary / Install from keyman.com to open the Keyboard Search bar. 3. Searched for the 'Khmer Angkor' keyboard, then installed it successfully. Seems to be working fine. (notes)
  • ✅ TEST_DOWNLOADS (PASSED): Tested with the attached PR build (version 17.0.261-alpha-test-10624) in the Android mobile emulator (Android 12 / API 31) and here is my observation: 1. Launched Chrome browser. 2. Downloaded the sil_cameroon_qwerty.kmp file from the given link. 3. Clicked Chrome / Downloads. 4. Clicked the sil_cameroon_qwerty.kmp file from the Downloads folder. 5. Completed the keyboard installation. 6. Verified that the sil_cameroon_qwerty keyboard was installed and appeared on the screen. Seems to be working fine. (notes)

Test Artifacts

@darcywong00 darcywong00 added this to the B17S1 milestone Feb 5, 2024
mcdurdin
mcdurdin reviewed Feb 5, 2024
View reviewed changes
android/KMAPro/kMAPro/src/main/AndroidManifest.xml
Comment on lines +106 to +108
<data
android:mimeType="application/x-keyman-package"
android:scheme="content" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a application/zip mime type? Given .kmp will be sniffed as a zip file.

Suggested change
<data
android:mimeType="application/x-keyman-package"
android:scheme="content" />
<data
android:mimeType="application/x-keyman-package"
android:scheme="content" />
<data
android:mimeType="application/zip"
android:scheme="content" />

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, I tried adding application/zip but then we'll need additional code handling in the engine to expect .zip files instead of strictly .kmp files.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feature for another day then

android/KMAPro/kMAPro/src/main/AndroidManifest.xml
<data android:pathPattern=".*\\..*\\..*\\..*\\..*\\.kmp" />
<data android:pathPattern=".*\\..*\\..*\\..*\\..*\\..*\\.kmp" />
<data android:pathPattern=".*\\..*\\..*\\..*\\..*\\..*\\..*\\.kmp" />
<data android:pathPattern="/.*\\.kmp" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use android:pathSuffix=".kmp" instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From https://developer.android.com/guide/topics/manifest/data-element

pathSuffix has an API prereq

pathSuffix and pathAdvancePattern were introduced in API level 31.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "/" at the start shouldn't be needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the slash is what the Play Store is flagging:

Add a "/" to the beginning of the android:path attribute in the tag. The attribute might also be android:pathPrefix or android:pathPattern.

😕

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh I see

@bharanidharanj
Copy link

Test Results

  • TEST_KEYMAN_SEARCH (PASSED): Tested with the attached PR build (version 17.0.261-alpha-test-10624) in the Android mobile emulator (Android 12 / API 31) and here is my observation: 1. Opened Keyman In-App. 2. Clicked Settings / Install Keyboard or Dictionary / Install from keyman.com to open the Keyboard Search bar. 3. Searched for the 'Khmer Angkor' keyboard, then installed it successfully. Seems to be working fine.

@bharanidharanj
Copy link

Test Results

  • TEST_DOWNLOADS (PASSED): Tested with the attached PR build (version 17.0.261-alpha-test-10624) in the Android mobile emulator (Android 12 / API 31) and here is my observation: 1. Launched Chrome browser. 2. Downloaded the sil_cameroon_qwerty.kmp file from the given link. 3. Clicked Chrome / Downloads. 4. Clicked the sil_cameroon_qwerty.kmp file from the Downloads folder. 5. Completed the keyboard installation. 6. Verified that the sil_cameroon_qwerty keyboard was installed and appeared on the screen. Seems to be working fine.

..from the Downloads folder

..during installation

..successfully installed sil_cameroon_qwerty keyboard

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Feb 5, 2024
@darcywong00 darcywong00 merged commit dadbf05 into master Feb 6, 2024
6 checks passed
@darcywong00 darcywong00 deleted the chore/android/cleanup-kmp-manifest branch February 6, 2024 06:51
@keyman-server
Copy link
Collaborator

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

@darcywong00 darcywong00 mentioned this pull request Feb 7, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcdurdin mcdurdin mcdurdin approved these changes

@sgschantz sgschantz Awaiting requested review from sgschantz sgschantz is a code owner

Assignees
No one assigned
Labels
android/app/ android/ fix has-user-test
Projects
None yet
Milestone
B17S1
Development

Successfully merging this pull request may close these issues.

bug(android): file association pattern is not valid -- file:*.kmp deep link
4 participants
@darcywong00 @bharanidharanj @keyman-server @mcdurdin