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.

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

fix(android/app): Update storage permissions for Android 12.0+ #11299

Merged
merged 1 commit into from
Apr 25, 2024

Conversation

darcywong00
Copy link
Contributor

@darcywong00 darcywong00 commented Apr 25, 2024

Fixes #10659 and follows the permission refactoring from #10904

Reference
https://developer.android.com/about/versions/13/behavior-changes-13#granular-media-permissions

This updates CheckPermissions to ask for the corresponding storage permissions needed to install local kmp files.

For Android API 30-32

  • Manifest.permission.READ_EXTERNAL_STORAGE

For Android API 33+

  • Manifest.permission.READ_MEDIA_IMAGES and
  • Manifest.permissions.READ_MEDIA_VIDEO.
    (I tested without READ_MEDIA_AUDIO and it seemed to worked)

Note:
For these devices, the user needs to still manually grant these permissions from Android Settings before Keyman can access local .kmp files. This is documented in the updated help page: grant-storage-permission.md

User Testing

Setup - Install the PR build of Keyman for Android on the corresponding Android device/emulator (API number in the test name)
Also, from Chrome, download a local copy of sil_ipa.kmp

  • TEST_API_31 - Verifies Keyman can access local kmp file on Android API 31
  1. Verify device/emulator is Android 12.0 "S" / API 31
  2. From Chrome --> Downloads --> select sil_ipa.kmp (Do not use Samsung MyFiles browser to access the file)
  3. Select Keyman to handle the kmp file
  4. Verify Keyman displays error about storage permission being denied
  5. From Keyman for Android --> Info --> Troubleshooting --> How To - Grant Storage Permission
  6. Follow the steps for the corresponding Android device to grant storage permission to Keyman
  7. From Chrome --> Downloads --> select sil_ipa.kmp
  8. Select Keyman to handle the kmp file
  9. Verify the keyboard installation wizard appears
  10. Finish installing sil_ipa and verify the IPA (SIL) keyboard appears
  • TEST_API_34 - Verifies Keyman can access local kmp file on Android API 34
  1. Verify device/emulator is Android 14.0 "UpsideDownCake" / API 34
  2. From Chrome --> Downloads --> select sil_ipa.kmp (Do not use Samsung MyFiles browser to access the file)
  3. Select Keyman to handle the kmp file
  4. Verify Keyman displays error about storage permission being denied
  5. From Keyman for Android --> Info --> Troubleshooting --> How To - Grant Storage Permission
  6. Follow the steps for the corresponding Android device to grant storage permission to Keyman
  7. From Chrome --> Downloads --> select sil_ipa.kmp
  8. Select Keyman to handle the kmp file
  9. Verify the keyboard installation wizard appears
  10. Finish installing sil_ipa and verify the IPA (SIL) keyboard appears

@darcywong00 darcywong00 requested a review from sgschantz as a code owner April 25, 2024 04:01
@keymanapp-test-bot keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label Apr 25, 2024
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Apr 25, 2024

User Test Results

Test specification and instructions

  • TEST_API_31 (PASSED): Tested with the attached PR build (Keyman 17.0.314-beta-test-11299) on an Android 12 / API 31 emulator and here is my observation: 1. Opened the page in Chrome browser. 2. Tried to download the sil_ipa.kmp file. Got an error message about storage permission being denied. 3. Opened the Info / Troubleshotting/ How To - Grant Storage permission. Followed the steps for the corresponding Android device to grant storage permission to Keyman. 4. Able to install sil_ipa.kmp file from Chrome/ Download folder. Verified the IPA (SIL) keyboard appears in Keyman In-App. Seems to be working as expected. (notes)
  • TEST_API_34 (PASSED): Tested with the attached PR build (Keyman 17.0.314-beta-test-11299) on an Android 14 / API 34 emulator and here is my observation: 1. Opened the page in Chrome browser. 2. Tried to download the sil_ipa.kmp file. Got an error message about storage permission being denied. 3. Opened the Info / Troubleshooting/ How To - Grant Storage permission. Followed the steps for the corresponding Android device to grant storage permission to Keyman. 4. Able to install sil_ipa.kmp file from Chrome/ Download folder. Verified the IPA (SIL) keyboard appears in Keyman In-App. Seems to be working as expected.

Test Artifacts

@keymanapp-test-bot keymanapp-test-bot bot added this to the B17S6 milestone Apr 25, 2024
@darcywong00 darcywong00 linked an issue Apr 25, 2024 that may be closed by this pull request
@keymanapp-test-bot 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 Apr 25, 2024
Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

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

LGTM

Can we not request permission to access the files at the time it is needed? I've certainly seen that before. I would expect:

  1. user selects file to open
  2. Keyman pops a message saying it'll need file/video/photo access in order to open the file
  3. User clicks OK, Keyman pops permission access request.
  4. User clicks Allow, Keyman installs the package.

@darcywong00
Copy link
Contributor Author

Can we not request permission to access the files at the time it is needed? I've certainly seen that before. I would expect:

I think on older versions of Android, it used to prompt for the permission when the file is accessed. That seems to have changed over the years.

@bharanidharanj
Copy link

Test Results

  • TEST_API_31 (PASSED): Tested with the attached PR build (Keyman 17.0.314-beta-test-11299) on an Android 12 / API 31 emulator and here is my observation: 1. Opened the page in Chrome browser. 2. Tried to download the sil_ipa.kmp file. Got an error message about storage permission being denied. 3. Opened the Info / Troubleshotting/ How To - Grant Storage permission. Followed the steps for the corresponding Android device to grant storage permission to Keyman. 4. Able to install sil_ipa.kmp file from Chrome/ Download folder. Verified the IPA (SIL) keyboard appears in Keyman In-App. Seems to be working as expected.

..error message

..Grant Storage permission help

..Allow Access

..Installed sil_ipa keyboard

@bharanidharanj
Copy link

Test Results

  • TEST_API_34 (PASSED): Tested with the attached PR build (Keyman 17.0.314-beta-test-11299) on an Android 14 / API 34 emulator and here is my observation: 1. Opened the page in Chrome browser. 2. Tried to download the sil_ipa.kmp file. Got an error message about storage permission being denied. 3. Opened the Info / Troubleshooting/ How To - Grant Storage permission. Followed the steps for the corresponding Android device to grant storage permission to Keyman. 4. Able to install sil_ipa.kmp file from Chrome/ Download folder. Verified the IPA (SIL) keyboard appears in Keyman In-App. Seems to be working as expected.

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-required User tests have not been completed label Apr 25, 2024
@darcywong00 darcywong00 merged commit 32a8632 into beta Apr 25, 2024
6 checks passed
@darcywong00 darcywong00 deleted the fix/android/storage-permission-2 branch April 25, 2024 12:24
@keyman-server
Copy link
Collaborator

Changes in this pull request will be available for download in Keyman version 17.0.314-beta

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(android/engine): Storage permission denied on Android 14.0+
4 participants