Android-image-cropper: Incorrect documentation: no need for storage permission?

[AndroidDeveloperLB]

This isn't really needed:

<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE"/>
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/>

I have no idea why it's written as needed. You can store images on the app's internal storage path instead.

Even if you do intend to write to external storage, you have it automatically for the current app's storage path, starting from API 19 (docs here), so you can use this instead:

<uses-permission
    android:name="android.permission.WRITE_EXTERNAL_STORAGE" android:maxSdkVersion="18"/>
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" android:maxSdkVersion="18"/>

so unless you want to store the image on some random path, you don't really need it. The image can be stored on the app's storage...

I've just tested a POC I made with this library, using just CropImageView, and I didn't need this permission.

I think you should update the docs, to at least tell what these permissions are used for.

[AndroidDeveloperLB]

I event tried to run the samples of this repo without this permission, on Android 4.2 , 4.4, 8.0, ....
They all still worked, including taking photos from other apps and from the camera app.
I don't get in which case this permission is used.

[ArthurHub]

I don't remember the details but someone had an issue that was solved by adding this permission.
Note that the min API is 14 and was 10 at the time, a far cry from 19.
Android is very fragmented with different manufactures loosely follow Android specification creating a lot of compatibility issue so testing one an emulator is usually not enough.

[AndroidDeveloperLB]

So it's not needed anymore, if my minSdk is 14 like on the library ?

[ArthurHub]

How did you figure that from my answer?

[AndroidDeveloperLB]

Didn't you mean this was from the time the minSdk was 10?
This means the issue was probably from a very long time, and the issue that you had was, as well.
Maybe it's ok now to avoid this permission, then.
In any case, the docs don't say anything about the requirement of this permission, and what it's used for.
It just says "Add permissions to manifest".

[ArthurHub]

Maybe, but I prefer people add this instead of the library not working on some Chinese Android device.

[AndroidDeveloperLB]

If a permission isn't needed for what Android is supposed to work, it shouldn't be mentioned as requirement, but as a recommendation with explanation instead.

[ArthurHub]

Maybe, I prefer to have it.

[AndroidDeveloperLB]

At the very least, write an explanation why it's needed, so that people would have this important information.

[ArthurHub]

I don't think it's important enough

[AndroidDeveloperLB]

Permissions are very important part of libraries.
It can be a crucial reason for users, to decide if the app should be installed or not, especially on old Android versions, when a dialog showing all permissions is shown.
Libraries should use as little permissions as possible, and should always show the reason for each permission, why it's a requirement.

[AndroidDeveloperLB]

Why close it?
Nothing has changed in the repo...

[ArthurHub]

I disagree with your view and not going to change it

[AndroidDeveloperLB]

With that permissions should have an explanation of why and when they are used?
Why?
It should be clear to both users and developers to know it.

[zwang]

It will be great to document that the permission might not needed. I am using this library for upload profile image, and this is the only reason I put the write external permission in my manifest.xml. We are asking google to feature our app and got a response back saying our seems don't need this permission and they ask us to explain. Then I did some testing, it turned out we don't need this permission to use this lib. Our minSDK is 16. Just FYI. (Update: Please see below for the more information, the comment here is not 100% correct, this only applies to app build with debug mode)

Thanks for the library.

[AndroidDeveloperLB]

@zwang That's what I wrote:
It's very important to explain why permissions are needed, if at all.
Libraries are used by apps, and people need to know why they are needed in apps.

[zwang]

Sorry for my information earlier, it turns out in android 22 and below, if the app is build with debug mode, we don't need to specify the permissions since android auto grant that permissions for the app. However, if app is build with Release build type, we do need to specify the permissions in manifest. Just FYI.

@AndroidDeveloperLB I agree the document can be better. Maybe a PR will do the work.

[tomerpetel]

@zwang, I just tried running without any storage permissions in release mode and it still worked.
Tried it on several devices (Galaxy S8, Xiaomi Redmi Note 3, Nexus 4P & Asus ZenPone 5) with several Android versions (5.0, 5.0.2, 8, 8.1.0) and saw no issues.

[pratikbutani]

@AndroidDeveloperLB @ArthurHub Is this really required to take permission runtime? I am not getting any error.

SO Question : https://stackoverflow.com/questions/57176364/read-or-write-external-storage-permission-why-no-error

[AndroidDeveloperLB]

@pratikbutani I think that if your minSdk is at least 19, you don't need it. Reason:
https://developer.android.com/training/data-storage/files#ExternalStoragePermissions

"Beginning with Android 4.4 (API level 19), reading or writing files in your app's private external storage directory—accessed using getExternalFilesDir()—does not require the READ_EXTERNAL_STORAGE or WRITE_EXTERNAL_STORAGE permissions."