[TIMOB-28182] Android: Only add WRITE_EXTERNAL_STORAGE permission when needed
GitHub Issue | n/a |
---|---|
Type | Improvement |
Priority | Medium |
Status | Closed |
Resolution | Fixed |
Resolution Date | 2020-11-16T21:39:44.000+0000 |
Affected Version/s | n/a |
Fix Version/s | Release 9.3.0 |
Components | Android |
Labels | android, externalstorage, manifest, permission, storage |
Reporter | Joshua Quick |
Assignee | Joshua Quick |
Created | 2020-10-09T01:06:23.000+0000 |
Updated | 2020-11-16T21:39:44.000+0000 |
Description
*Synopsis:*
The "AndroidManifest.xml" permission
WRITE_EXTERNAL_STORAGE
is ignored when targeting Android 10+ and running on those Android OS versions. This is because of scoped-storage and Google only allows an app to write to very particular folders on external storage. The app's sandboxed folders on external storage no longer require permission as of Android 4.4.
https://developer.android.com/training/data-storage/shared/media#request-permissions
*To-Do:*
The Titanium build system should be changed to only inject this permission when needed based on JavaScript APIs used.
If the Ti.Filesystem.requestStoragePermissions()
method is used, then the permission should be added as before like this. This is the only method that requires this permission on Android 10 and above.
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/>
If Ti.Media.showCamera()
or Ti.Media.saveToPhotoGallery()
methods are used, then we should add a maxSdkVersion
attribute set to Android 9 since external storage access is not required on Android 10 and higher.
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" android:maxSdkVersion="28"/>
If none of the above JavaScript methods are detected, then we should *NOT* add this permission.
*Note:*
If the "tiapp.xml" adds the WRITE_EXTERNAL_STORAGE
permission, then it should take priority and the above should be ignored.
Attachments
File | Date | Size |
---|---|---|
CameraPhotoExternalTest.js | 2020-11-09T22:24:20.000+0000 | 982 |
ImageToGalleryTest.js | 2020-11-09T22:23:56.000+0000 | 870 |
RequestStoragePermissionsTest.js | 2020-11-09T23:01:26.000+0000 | 526 |
I'm thinking we should *NOT* implement this because it would be a breaking-change that would cause a lot of tech-support issues. It should be the Titanium app developer's responsibility to do this instead. The reason is because a lot of customer code that looks like the below will always fail to request for external storage permission on Android 10 and above if we set
maxSdkVersion
to29
. Note that therequestStoragePermission()
method call is no longer required anymore as of 9.3.0 and can be removed.*Note1 :* As of Titanium 9.3.0, our
Ti.Media.showCamera()
andTi.Media.saveToPhotoGallery()
method no longer require this permission on Android 10 and above. And ourTi.Filesystem.externalStorageDirectory
does not require this permission on Android 4.4 and above. This means theTi.Filesystem.requestStoragePermission()
is only needed for: * Android 6 - Android 9 when usingshowCamera()
andsaveToPhotoGallery()
APIs. * Sometimes by 3rd party modules or hyperloop code which writes outside of scoped storage. *Note 2:* As of 9.3.0, theTi.Media.requestCameraPermissions()
method is smart enough to only request external storage permission for Android 6 to Android 9. It won't request this permission on Android 10 and above. This method should be used before callingshowCamera()
. *Note 3:* As of 9.3.0, we've addedTi.Media.requestPhotoGalleryPermissions()
support on Android. This method should be used before callingsaveToPhotoGallery()
like iOS. The Android implementation is smart enough to only request external storage permission for Android 6 to Android 9. This method will always return return true on Android 10 and above.Okay. I've changed my mind. Instead of always adding the
WRITE_EXTERNAL_STORAGE
permission with amaxSdkVersion
set to Android 9, we should instead scan the JS code for APIs that need this permission and inject it intelligently like how we handle the camera and location permissions.PR (master): https://github.com/appcelerator/titanium_mobile/pull/12253
FR Passed. PR Merged.