search

apache / cordova-plugin-file

Apache Cordova File Plugin
https://cordova.apache.org/
Apache License 2.0
740 stars 757 forks source link

fix Android 13 write permissions #581

Closed EYALIN closed 7 months ago

EYALIN commented 10 months ago

in Android 13 we don't need to ask for permissions

Platforms affected

Android

Motivation and Context

Closes #606 Closes #577

Description

Testing

Checklist

EYALIN commented 10 months ago

@breautek the errors here are not related to my commits (IOS for example? ) can someone merge it?

MauriceFrank commented 10 months ago

@EYALIN DO you have any experience with the corodva-plugin-saf-mediastore?

I need to write media files into the gallery(external storage) on Android 13 but so far I have no success with the saf-mediastore plugin. Any alternatives?

I would be glad for any help ,thank you!

EYALIN commented 10 months ago

@MauriceFrank i'm using the photo library plugin, you can take my fork: https://github.com/EYALIN/cordova-plugin-photo-library

alexisGandar commented 9 months ago

@EYALIN the fix works great thank you, can someone merge it ?

ath0mas commented 9 months ago

@EYALIN can you please correct the indentation.

Also, remove the private field permissions which is never used (lines 102-116).

And why not completly removing this whole methods hasWritePermission and getWritePermission? since cordova-plugin-file v8 requires cordova-android 12+ with support for Android 24+, while WRITE_EXTERNAL_STORAGE is not supported since 19.

breautek commented 9 months ago

And why not completly removing this whole methods hasWritePermission and getWritePermission? since cordova-plugin-file v8 requires cordova-android 12+ with support for Android 24+, while WRITE_EXTERNAL_STORAGE is not supported since 19.

WRITE_EXTERNAL_STORAGE is still used between API 24-28. It's not used with Scoped Storage mechanism.

Between API-24-28, the permission is not required for your app's external data directory, but is required to write to external shared directories (e.g. the Downloads directly).

But there are other significant issues with using the external filesystem on API 29+ that makes using a filesystem oriented API not feasible and I feel like we should just simply drop support for it in favour of media store plugin that interfaces with the native MediaStore instead. If we do go that route, then we can drop the permissions and probably simplify the file plugin significantly.

ath0mas commented 8 months ago

To me, hasReadPermission and hasWritePermission should be left unchanged, because they already return the correct status for *_EXTERNAL_STORAGE permissions - if considering it strictly.

And, as I work on PR #597, I like the idea that this plugin should concentrate on file operations among the app dirs only, default authorized without any permission ; and that trying to read-write outside should be done using other plugins, to follow the concepts and isolation forced by Google for file-sharing between apps or publicly, relying on SAF, MediaStore, ContentProvider, etc.

For more tech review: