permissions-dispatcher / PermissionsDispatcher

A declarative API to handle Android runtime permissions.
https://github.com/permissions-dispatcher/PermissionsDispatcher
Apache License 2.0
11.21k stars 1.44k forks source link

[KTX] System dialog doesn't show up, after manually disable permission from setting #705

Closed TurKurT656 closed 3 years ago

TurKurT656 commented 3 years ago

Overview

I've two permissions in my fragment (camera and storage) this is my code for them:

private fun checkCameraPermission() {
        constructPermissionsRequest(
                Manifest.permission.CAMERA,
                onShowRationale = { request ->
                    showPermissionRationaleDialog(
                            Manifest.permission.CAMERA,
                            onAccept = { request.proceed() },
                            onDeny = { navigateHome() }
                    )
                },
                onPermissionDenied = {
                    navigateHome()
                },
                onNeverAskAgain = {
                    showPermissionNeverAskAgainDialog(
                            Manifest.permission.CAMERA,
                            onDeny = { navigateHome() }
                    )
                }
        ) {
            showCamera()
        }.launch()
    }

    private fun checkStoragePermission() {
        constructPermissionsRequest(
                Manifest.permission.WRITE_EXTERNAL_STORAGE,
                onShowRationale = { request ->
                    showPermissionRationaleDialog(
                        Manifest.permission.WRITE_EXTERNAL_STORAGE,
                        onAccept = { request.proceed() }
                    )
                },
                onNeverAskAgain = {
                    showPermissionNeverAskAgainDialog(Manifest.permission.WRITE_EXTERNAL_STORAGE)
                }
        ) {
            saveFile()
        }.launch()
    }

I'm calling checkCameraPermission() on my fragment's onStart() function. but there is a save button for calling checkStoragePermission(). Everything works great. until when I go to setting and manually disable storage permission from there. After returning to the fragment when I press the save button, Rationale shows up but when I accept button the system permission dialog doesn't show up. (I set a breakpoint and I saw that request.proceed() called. but It doesn't jump to the requiresPermission method.) The Strange part is that If I disable checkCameraPermission() from onStart() It will work.

Expected

System dialog shows up event after manually disabling permission from setting

Actual

System dialog doesn't show up, after manually disable permission from setting (when there is two permission checks on the fragment)

Environment

TurKurT656 commented 3 years ago

I will be glad if someone give me a temporary solution

hotchemi commented 3 years ago

@TurKurT656 Hi sorry for the late I am back from the vacation. would you mind trying to call constructPermissionsRequest only in onCreate or something?

https://github.com/permissions-dispatcher/PermissionsDispatcher/blob/3a238dad5a27d90b0bf27f22be920a2d6ac968a8/ktx-sample/src/main/java/permissions/dispatcher/ktx/sample/MainFragment.kt#L18-L31

hotchemi commented 3 years ago

Admittedly this is a bit counter-intuitive, but Google official way has the same restriction and we're unifying an interface..we might need lint to detect it.

https://developer.android.com/reference/androidx/activity/result/ActivityResultCaller#registerForActivityResult(androidx.activity.result.contract.ActivityResultContract%3CI,%20O%3E,%20androidx.activity.result.ActivityResultCallback%3CO%3E)

TurKurT656 commented 3 years ago

I've tried onAttach() and onCreate() too. None of them worked for me. Also I've to use this inside onstart() method because I need to check camera permission every time fragment resumed and resume camera if the permission granted.

hotchemi commented 3 years ago

um, let us take a look at this weekend 🙏

hotchemi commented 3 years ago

I tried with ktx-sample but could not repro on API level Q. Which env did you try? https://imgur.com/a/9oCLAyK

TurKurT656 commented 3 years ago

I'm using api level 29. I'll try to make a sample project and share it with you.

xie438305665 commented 3 years ago

Encountered the same problem And appeared FragmentManager is already executing transactions

kenshin171 commented 3 years ago

encountered the same issue with permissionsRequester.launch() in onStart and initialization of permissionsRequester in onAttach(Context) in a fragment.

E  java.lang.IllegalStateException: FragmentManager is already executing transactions
E      at androidx.fragment.app.FragmentManager.ensureExecReady(SourceFile:1790)
E      at androidx.fragment.app.FragmentManager.execSingleAction(SourceFile:1826)
E      at androidx.fragment.app.BackStackRecord.commitNowAllowingStateLoss(SourceFile:303)
E      at permissions.dispatcher.ktx.PermissionsRequesterImpl$launch$requestFun$1.invoke(SourceFile:30)
E      at permissions.dispatcher.ktx.PermissionsRequesterImpl$launch$requestFun$1.invoke(SourceFile:7)
E      at permissions.dispatcher.ktx.PermissionsRequesterImpl.launch(SourceFile:35)
E      at a.b.fragment.myFragment.onStart(SourceFile:355)
... omitted ...
hotchemi commented 3 years ago

thx, let me take a look again 🙏

hotchemi commented 3 years ago

um, testing on API level 28~30 but haven't been able to repro the issue 🤔 If someone could prepare an example that can repro the issue that'd be awesome 🙏

~But for java.lang.IllegalStateException: FragmentManager is already executing transactions I assume we can replace commitNowAllowingStateLoss with executePendingTransactions.~ Maybe we should use childFragmentManager.

hotchemi commented 3 years ago

I assume ktx-1.0.2 has resolved the issue 🙏

hotchemi commented 3 years ago

@xie438305665 @kenshin171 @TurKurT656 https://github.com/permissions-dispatcher/PermissionsDispatcher/issues/705#issuecomment-789107946 and please reopen the issue as appropriate 🙇

kenshin171 commented 3 years ago

I assume ktx-1.0.2 has resolved the issue 🙏

it seems 1.0.2 relies on 4.9.0-SNAPSHOT and the module name has changed, the read me is still showing the old module name.

com.github.permissions-dispatcher:permissionsdispatcher-ktx is now com.github.permissions-dispatcher:ktx

hotchemi commented 3 years ago

good catch and sorry for messing up after maven central migration🙏 now released 1.0.3 and updated readme 🙇

kenshin171 commented 3 years ago

@hotchemi just tried ktx 1.0.3, crash is not happening anymore. Just a suggestion, or maybe even a nitpick.

Maybe should only create requestFun when PermissionUtils.shouldShowRequestPermissionRationale(activity, *permissions) == false ?

like below

override fun launch() {
        if (permissionRequestType.checkPermissions(activity, permissions)) {
            requiresPermission()
        } else {
            ViewModelProvider(activity).get(PermissionRequestViewModel::class.java).observe(
                activity,
                requiresPermission,
                onPermissionDenied,
                onNeverAskAgain
            )

            if (PermissionUtils.shouldShowRequestPermissionRationale(activity, *permissions)) {
                onShowRationale?.invoke(KtxPermissionRequest.create(onPermissionDenied, requestFun))
            } else {
               val requestFun: Fun = {
                    activity.supportFragmentManager
                        .beginTransaction()
                        .replace(android.R.id.content, permissionRequestType.fragment(permissions))
                        .commitAllowingStateLoss()
                }
                requestFun.invoke()
            }
        }
    }

And excuse my ignorance if i cannot figure out the reason, but maybe for fragments, could use childfragmentmanager(as u mentioned before) and fragment scope viewmodels instead? It looks more correct to encapsulate the permissions request to the fragment scope itself instead of the activity scope if the permissions is request from the fragment.

like below

override fun launch() {
        if (permissionRequestType.checkPermissions(activity, permissions)) {
            requiresPermission()
        } else {
            ViewModelProvider(activity).get(PermissionRequestViewModel::class.java).observe(
                fragment,
                requiresPermission,
                onPermissionDenied,
                onNeverAskAgain
            )

            if (PermissionUtils.shouldShowRequestPermissionRationale(activity, *permissions)) {
                onShowRationale?.invoke(KtxPermissionRequest.create(onPermissionDenied, requestFun))
            } else {
               val requestFun: Fun = {
                    fragment.childFragmentManager
                        .beginTransaction()
                        .replace(android.R.id.content, permissionRequestType.fragment(permissions))
                        .commitAllowingStateLoss()
                }
                requestFun.invoke()
            }
        }
    }

and one more thing/suggestion. Sorry for the long winded comment. Could PermissionRequester perhaps store the PermissionRequest so that it can be retrieved in other places instead of only in the onShowRationale function specified? Reason is current i am triggering the PermissionRequest.proceed() from a button click. What i am doing currently is shown below.

// button click listener
    binding.btnCameraPermissionOk.clicks()
            .debounce(300)
            .onEach {               
                    if (gotoSettings)
                        goToSystemSettingsPage()
                    else {
                        permissionRequest?.proceed() ?: permissionsRequester.launch()
                    }
                }
            }
            .launchIn(viewLifecycleOwner.lifecycleScope)

// onRationale function, triggering a showing of a button Ui on screen
    fun showRationaleStartCameraSource(permrequest: PermissionRequest) {
        permissionRequest = permrequest
       // fullscreen UI to show camera permissions needs to be enable 
        binding.flCamPermission.visibility = View.VISIBLE
        binding.flCamExistence.visibility = View.GONE
        binding.btnCameraPermissionOk.text = getString(R.string.button_ok)
    }
hotchemi commented 3 years ago

thx for the suggestion, let me check!

kenshin171 commented 3 years ago

@hotchemi any updates on this?

1) storing a reference to PermissionRequest in PermissionRequester 2) using childfragmentmanager and a fragment scope viewmodel

hotchemi commented 3 years ago

@kenshin171 sorry for the late,

Maybe should only create requestFun when PermissionUtils.shouldShowRequestPermissionRationale(activity, *permissions) == false ?

LGTM. would you mind creating PR?

storing a reference to PermissionRequest in PermissionRequester

do you have any proposal for this? for now I'm not so much positive as it looks beyond the responsibility of the library but might be because I haven't been able to imagine that well.

using childfragmentmanager and a fragment scope viewmodel

Actually when I was trying to fix the original issue, the initial approach was to use childfragmentmanager, but it didn't go well(sorry I forgot why but might be avoidable).

kenshin171 commented 3 years ago

@hotchemi

ok i have tried to implement some changes and found out

Maybe should only create requestFun when PermissionUtils.shouldShowRequestPermissionRationale(activity, *permissions) == false ?

this is not valid anymore i missed out onShowRationale?.invoke(KtxPermissionRequest.create(onPermissionDenied, requestFun)) is also using the requestFun, no change needed here.

storing a reference to PermissionRequest in PermissionRequester

do you have any proposal for this? for now I'm not so much positive as it looks beyond the responsibility of the library but might be because I haven't been able to imagine that well.

I was basically trying to find a way to implement orientation change support, which did not work as the permissionRequest relies on higher order functions that were passed as arguments to the permissionsRequester which is no longer valid in an orientation change even if i saved the permissionRequest object in the viewmodel.

And due to these higher order function arguments, the ktx lib will not work for orientation change scnario. Annotation will still work though as no higher order function are being used as arguments.

using childfragmentmanager and a fragment scope viewmodel

Actually when I was trying to fix the original issue, the initial approach was to use childfragmentmanager, but it didn't go well(sorry I forgot why but might be avoidable).

and the childfragmentmanager is not possible due to this No view found for id 0x1020002 (android:id/content) for fragment NormalRequestPermissionFragment{69f2b36 #0 id=0x1020002} There is not container to inflate the permission fragment in the childfragmentmanager.

hotchemi commented 3 years ago

got it for point 1 and 3. for point 2, let me think it over...it may have some degrades from my initial design 🤔