apache / cordova-plugin-camera

Apache Cordova Plugin camera
https://cordova.apache.org/
Apache License 2.0
960 stars 1.52k forks source link

Fix: Duplicate Permission Error For WRITE_EXTERNAL_STORAGE #881

Closed mejainankit closed 2 months ago

mejainankit commented 2 months ago

Platforms affected

Android

Motivation and Context

Fix For Duplicate Permission Error For WRITE_EXTERNAL_STORAGE

https://github.com/apache/cordova-plugin-camera/issues/882

Description

Due to minSDK check, this was creating multiple permission in Android manifest file. If you are using any other plugin, which also requires the WRITE_EXTERNAL_STORAGE

Element uses-permission#android.permission.WRITE_EXTERNAL_STORAGE at AndroidManifest.xml:20:5-108 duplicated with element declared at AndroidManifest.xml:13:5-81

Testing

Tested with android build process

Checklist

breautek commented 2 months ago

This isn't fixing the issue, it's just shifting it.

The conflict will always occur if you have 2 or more plugins declaring the same permission with a different configuration. This includes if this plugin declared with no min sdk but another plugin did, and that's completely outside of Cordova's control.

The original discussion on this subject was done inside https://github.com/apache/cordova-plugin-camera/pull/814 (the PR was rebased and merged via https://github.com/apache/cordova-plugin-camera/pull/844)

But to summarise why we choose max sdk 32 because even though WRITE didn't grant any explicit permissions over READ since API 28 with the introduction of scoped storage, requesting WRITE did grant READ access until API 33 which introduces the 3 newer permissions for reading. Additionally it's recommended by Android Studio lint tool to specify max SDK 32 ~and google has recently announced it will start cracking down on applications declaring permissions that isn't actually being used.~ (Found the announcement and I guess this only applies to the 3 new READ permissions). So Cordova opted for what appears to be the more "correct" approach and would expect plugin authors to do the same.

Screenshot from 2024-02-27 09-22-46

For those reasons I'm closing this PR. I'll link to how this issue can be work-around in https://github.com/apache/cordova-plugin-camera/issues/882

mejainankit commented 2 months ago

@breautek Thanks for the update