-
Notifications
You must be signed in to change notification settings - Fork 761
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
fix(android): hasWritePermission for SDK 33 #608
fix(android): hasWritePermission for SDK 33 #608
Conversation
in Android 13 we don't need to ask for permissions
if (android.os.Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU) { | ||
int requestCode = pendingRequests.createRequest(rawArgs, action, callbackContext); | ||
PermissionHelper.requestPermission(this, requestCode, Manifest.permission.WRITE_EXTERNAL_STORAGE); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is happening here in the else
case?
Mostly asking because getWritePermission
is always called providing a callbackContext
.
Are we sure it does not wait indefinitely?
Is doing nothing really fine, or should callbackContext
be called in every case, to guarantee the execution flow continues?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I "found" my own answer:
in current code, in the case of Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU
, getWritePermission
is never called ...
So my question does not really mater, even if strictly speaking it is probably a problem for the execution flow, indefinitely waiting for the callbackContext
.
Details:
getWritePermission
calls are always preceded by a check for needPermission(nativeURL, WRITE)
, that will return false
in this same given versions condition, because hasWritePermission
was changed to always return true
for it.
So, the if (android.os.Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU) {
added in getWritePermission
is mostly useless, and should whether be removed or be fixed.
Platforms affected
android
Motivation and Context
rebase, fix, and simplify changes from #581
Closes #581
Description
This PR performed following changes against #581:
getWritePermission
but applied suggestion to simplify. the condition check and to add extra accidental calls, even though the method would never be called in that scenario.hasWritePermission
logichasWritePermission
Testing
build test
Checklist
(platform)
if this change only applies to one platform (e.g.(android)
)