breautek commented on code in PR #581:
URL:
https://github.com/apache/cordova-plugin-file/pull/581#discussion_r1358134421
##########
src/android/FileUtils.java:
##########
@@ -614,7 +617,11 @@ private boolean hasReadPermission() {
}
private boolean hasWritePermission() {
- return PermissionHelper.hasPermission(this,
Manifest.permission.WRITE_EXTERNAL_STORAGE);
+ if (android.os.Build.VERSION.SDK_INT >=
Build.VERSION_CODES.TIRAMISU) {
+ return true;
Review Comment:
```suggestion
// Starting with API 33, requesting WRITE_EXTERNAL_STORAGE
is an auto permission rejection
// Apps now have read-write access to it's own (created)
files, so we can assume `true`, however not all files are readable.
// See
https://github.com/apache/cordova-plugin-file#androids-external-storage-quirks
for more details.
return true;
```
##########
src/android/FileUtils.java:
##########
@@ -593,14 +593,17 @@ private void getReadPermission(String rawArgs, int
action, CallbackContext callb
if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
PermissionHelper.requestPermissions(this, requestCode,
new String[]{Manifest.permission.READ_MEDIA_IMAGES,
Manifest.permission.READ_MEDIA_VIDEO, Manifest.permission.READ_MEDIA_AUDIO});
- } else {
+ } else {
PermissionHelper.requestPermission(this, requestCode,
Manifest.permission.READ_EXTERNAL_STORAGE);
- }
+ }
}
private void getWritePermission(String rawArgs, int action,
CallbackContext callbackContext) {
- int requestCode = pendingRequests.createRequest(rawArgs, action,
callbackContext);
- PermissionHelper.requestPermission(this, requestCode,
Manifest.permission.WRITE_EXTERNAL_STORAGE);
+ int requestCode = pendingRequests.createRequest(rawArgs, action,
callbackContext);
+ if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
+ } else {
+ PermissionHelper.requestPermission(this, requestCode,
Manifest.permission.WRITE_EXTERNAL_STORAGE);
+ }
Review Comment:
I think this can be rewritten as to invert the condition so we don't have an
empty code block.
```suggestion
if (android.os.Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU)
{
PermissionHelper.requestPermission(this, requestCode,
Manifest.permission.WRITE_EXTERNAL_STORAGE);
}
```
##########
src/android/FileUtils.java:
##########
@@ -614,7 +617,11 @@ private boolean hasReadPermission() {
}
private boolean hasWritePermission() {
- return PermissionHelper.hasPermission(this,
Manifest.permission.WRITE_EXTERNAL_STORAGE);
+ if (android.os.Build.VERSION.SDK_INT >=
Build.VERSION_CODES.TIRAMISU) {
+ return true;
Review Comment:
Just to explain why we are returning true for future contributors/code
maintainers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org
