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

feat(android)!: Android 13 support #844

Merged
merged 7 commits into from
Aug 26, 2023
Merged

Conversation

erisu
Copy link
Member

@erisu erisu commented Aug 17, 2023

Platforms affected

android

Motivation and Context

Android 13 Support

Description

closes #814
resolves #825

Testing

  • npm t
  • built test app

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
  • I've updated the documentation if necessary

@erisu erisu added this to the 7.0.0 milestone Aug 17, 2023
@erisu erisu requested a review from breautek August 17, 2023 12:21
ochakov and others added 6 commits August 18, 2023 00:59
Replace READ_EXTERNAL_STORAGE and WRITE_EXTERNAL_STORAGE permissions with READ_MEDIA_IMAGES on SDK 33
Handle video permission
Add permissions to manifest
Only require media storage permissions when taking picture when save to album is requested
Copy link
Contributor

@breautek breautek left a comment

Choose a reason for hiding this comment

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

untested, but code-wise lgtm

@erisu
Copy link
Member Author

erisu commented Aug 21, 2023

I believe this would be something you fix within your project.

cordova-plugin-file version 8.0.0 has removed permission WRITE_EXTERNAL_STORAGE.

If you are using a plugin that depends on cordova-plugin-file make sure it is requesting for version 8.0.0.

If it is not, check if there is a new plugin version that hopefully supports cordova-plugin-file 8.0.0.

Note: Some Cordova core plugins depend on the file plugin and not updated, but it is in the process of being updated.

If you are adding the cordova-plugin-file directly and it is not a depedency of another plugin, it might be OK to bump its version.

@christiaan
Copy link

If you are adding the cordova-plugin-file directly and it is not a depedency of another plugin, it might be OK to bump its version.

That is exactly how I fixed it 😄 and seeing the original PR already mentioning this issue I removed the question.

@christiaan
Copy link

christiaan commented Aug 23, 2023

On an Android 8.0 (and 7.1.1) device it fails after it asks for permission the first time with the following error

FATAL EXCEPTION: main
Process: com.myapp.app, PID: 22690
java.lang.RuntimeException: Failure delivering result ResultInfo{who=@android:requestPermissions:, request=1, result=-1, data=Intent { act=android.content.pm.action.REQUEST_PERMISSIONS (has extras) }} to activity {com.myapp.app/com.myapp.app.MainActivity}: java.lang.SecurityException: Permission Denial: reading com.android.providers.media.MediaProvider uri content://media/external/images/media from pid=22690, uid=10176 requires android.permission.READ_EXTERNAL_STORAGE, or grantUriPermission()
    at android.app.ActivityThread.deliverResults(ActivityThread.java:4491)
    at android.app.ActivityThread.handleSendResult(ActivityThread.java:4534)
    at android.app.ActivityThread.-wrap20(Unknown Source:0)
    at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1752)
    at android.os.Handler.dispatchMessage(Handler.java:105)
    at android.os.Looper.loop(Looper.java:164)
    at android.app.ActivityThread.main(ActivityThread.java:6944)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:327)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1374)
Caused by: java.lang.SecurityException: Permission Denial: reading com.android.providers.media.MediaProvider uri content://media/external/images/media from pid=22690, uid=10176 requires android.permission.READ_EXTERNAL_STORAGE, or grantUriPermission()
    at android.os.Parcel.readException(Parcel.java:1967)
    at android.database.DatabaseUtils.readExceptionFromParcel(DatabaseUtils.java:183)
    at android.database.DatabaseUtils.readExceptionFromParcel(DatabaseUtils.java:135)
    at android.content.ContentProviderProxy.query(ContentProviderNative.java:418)
    at android.content.ContentResolver.query(ContentResolver.java:760)
    at android.content.ContentResolver.query(ContentResolver.java:710)
    at android.content.ContentResolver.query(ContentResolver.java:668)
    at org.apache.cordova.camera.CameraLauncher.queryImgDB(CameraLauncher.java:1222)
    at org.apache.cordova.camera.CameraLauncher.takePicture(CameraLauncher.java:325)
    at org.apache.cordova.camera.CameraLauncher.onRequestPermissionResult(CameraLauncher.java:1362)
    at org.apache.cordova.CordovaInterfaceImpl.onRequestPermissionResult(CordovaInterfaceImpl.java:222)
    at org.apache.cordova.CordovaActivity.onRequestPermissionsResult(CordovaActivity.java:527)
    at android.app.Activity.dispatchRequestPermissionsResult(Activity.java:7780)
    at android.app.Activity.dispatchActivityResult(Activity.java:7603)
    at android.app.ActivityThread.deliverResults(ActivityThread.java:4487)
    at android.app.ActivityThread.handleSendResult(ActivityThread.java:4534) 
    at android.app.ActivityThread.-wrap20(Unknown Source:0) 
    at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1752) 
    at android.os.Handler.dispatchMessage(Handler.java:105) 
    at android.os.Looper.loop(Looper.java:164) 
    at android.app.ActivityThread.main(ActivityThread.java:6944) 
    at java.lang.reflect.Method.invoke(Native Method) 
    at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:327) 
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1374) 

If I then choose to pick a picture using using Camera.PictureSourceType.PHOTOLIBRARY it requests permission for storage and after that the Camera.PictureSourceType.CAMERA also works again.

I am calling getPicture like so

navigator.camera.getPicture(this.onCameraSuccess.bind(this), this.onCameraError.bind(this), {
    quality: 50,
    destinationType: Camera.DestinationType.DATA_URL,
    sourceType: Camera.PictureSourceType.CAMERA,
    allowEdit: false,
    encodingType: Camera.EncodingType.JPEG,
    correctOrientation: true,
    targetWidth: 1024,
    targetHeight: 1024
});

It appears to be because of this line in the plugin; on my phone the content store is aparently on the external storage.

        // Save the number of images currently on disk for later
        this.numPics = queryImgDB(whichContentStore()).getCount();

It appears the checkForDuplicateImage needs access to the storage even if you take a picture without saving it anywhere.

I worked around/fixed it by changing

        boolean saveAlbumPermission;
        if (this.saveToPhotoAlbum) {
            saveAlbumPermission = hasPermissions(storagePermissions);
        } else {
            saveAlbumPermission = true;
        }

to

        boolean saveAlbumPermission = hasPermissions(storagePermissions);

Seeing as even when not saving the image you still need storage permission to process duplicates.

@erisu
Copy link
Member Author

erisu commented Aug 25, 2023

@christiaan I updated the PR to fix the Android 8.0 and lower issue you reported. Can you confirm if it resolved for you?

@christiaan
Copy link

christiaan commented Aug 25, 2023

@erisu I can confirm the last commit resolved the issue on Android 8.0 (Samsung A3 2017) 👍 thanks!

@breautek
Copy link
Contributor

untested, but code-wise lgtm

Just an update, I tested this PR against my app and it seems good to go, at least in my usage of the camera plugin.

@erisu erisu merged commit 505ccef into apache:master Aug 26, 2023
5 of 9 checks passed
@erisu erisu deleted the ochakov-android-13-fix branch August 26, 2023 11:22
@clarklight clarklight mentioned this pull request Aug 31, 2023
3 tasks
@michelsondan
Copy link

Thank you, now camera working on android 13

ricardopetrere pushed a commit to ricardopetrere/cordova-plugin-camera that referenced this pull request Sep 4, 2023
* feat(android)!: Android 13 support
* refactor(android): simplify getPermissions logic
* feat(android)!: bump cordova-android requirement to >=12.0.0
* feat(android): update saveAlbumPermission to include Android 9 and below use case

---------

Co-authored-by: ochakov <evgeny@ochakov.com>
# Conflicts:
#	package-lock.json
#	package.json
#	plugin.xml
#	src/android/CameraLauncher.java
ricardopetrere pushed a commit to ricardopetrere/cordova-plugin-camera that referenced this pull request Sep 4, 2023
* feat(android)!: Android 13 support
* refactor(android): simplify getPermissions logic
* feat(android)!: bump cordova-android requirement to >=12.0.0
* feat(android): update saveAlbumPermission to include Android 9 and below use case

---------

Co-authored-by: ochakov <evgeny@ochakov.com>
# Conflicts:
#	package-lock.json
#	package.json
#	plugin.xml
#	src/android/CameraLauncher.java
ricardopetrere pushed a commit to ricardopetrere/cordova-plugin-camera that referenced this pull request Sep 5, 2023
* feat(android)!: Android 13 support
* refactor(android): simplify getPermissions logic
* feat(android)!: bump cordova-android requirement to >=12.0.0
* feat(android): update saveAlbumPermission to include Android 9 and below use case

---------

Co-authored-by: ochakov <evgeny@ochakov.com>
# Conflicts:
#	package-lock.json
#	package.json
#	plugin.xml
#	src/android/CameraLauncher.java
@breautek breautek mentioned this pull request Sep 6, 2023
3 tasks
@EinfachHans

This comment was marked as spam.

@apache apache locked as resolved and limited conversation to collaborators Sep 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Android api 33
7 participants