Karumi / Dexter

Android library that simplifies the process of requesting permissions at runtime.
http://karumi.com
Apache License 2.0
5.23k stars 671 forks source link

Activity with dexter only closes after the second click on the Back button. #181

Closed oligazar closed 4 years ago

oligazar commented 7 years ago

It's expected that to move from an activity one should hit the Back button just once.

But instead, whenever I use Dexter.withActivity()... inside my onCreate() method, first click on the Back button doesn't close the activity, only the second one does.

Is it intended behaviour? Is there way to avoid that?

Dexter version: 4.1.0

Serchinastico commented 6 years ago

Hi @oligazar

Looks like an interesting behavior, it might have to do with #143. However, I did some quick tests with the sample project by moving the Dexter requests to the onCreate method of the activity and I couldn't experience it myself so it might have to do with your specific configuration?

I'd suggest you to try out the oncoming version 4.2.0 once it's released.

rckahale commented 4 years ago

Yes, its happening in my case too. v 6.0.2 I use Dexter.withActivity(thisActivity) & withPermissions() with overridden listeners

The transparent activity created by the code stays even after closing of the dialog AlertDialog.Builder(context)

May be the fix can be belonging to DexterInstance.java private void startTransparentActivityIfNeeded() { Context context = this.context.get(); if (context == null) { return; }

Intent intent = intentProvider.get(context, DexterActivity.class);
if (context instanceof Application) {
  **intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);**
}
**context.startActivity(intent);**

}`

  1. Either create a exposed method to forcefully close this activity. Called from user activity .... OR
  2. Start Activity (startActivity) wihch doesn't go in the backstack with some flags like FLAG_ACTIVITY_NO_HISTORY or FLAG_ACTIVITY_CLEAR_TOP ...... OR
  3. https://stackoverflow.com/a/13742161/1029110 android:noHistory="true" in xml

If possible please include one of these as fix in next release

pedrovgs commented 4 years ago

Hi @rckahale thanks for reporting the issue. I tried to reproduce it following your instructions but I couldn't reproduce it. Could you please create a sample project reproducing the error or create a branch of this repository reproducing the error so we can review the bug? Thank you in advance.

rckahale commented 4 years ago

Hi @pedrovgs https://github.com/rckahale/doubleback project has the replica. Steps

  1. Tap to next activity from home page
  2. Deny permission of audio
  3. Alert builder dialog opens (any button dismisses the dialog)
  4. Now you need 2 backs to go from activity 2 to 1 If alert dialog is opened without dexter's withPermissions and associated listener functions, it needs one back
ValeryP commented 4 years ago

Another way to reproduce the issue:

  1. Show an alert dialog with a button that opens settings
  2. Button opens app's settings screen using context.startActivity(...)
  3. Back button closes the app's settings and returns the user to the app

Now the app requires two clicks back in order to close it.

You can see that Dexter transparent activity is overlapping the app's activity by checking the activities stack: adb shell dumpsys activity activities | sed -En -e '/Running activities/,/Run #0/p'

pedrovgs commented 4 years ago

Thank you so much @rckahale and @ValeryP for your sample project and reports. I've found what's going on.

The first one is related to the listener. There are two important details to keep in mind when using Dexter and the MultiplePermissionListener. You should invoke token?.continuePermissionRequest() at some point during the permission handling. In your scenario I think invoking this method from the onPermissionRationaleShouldBeShown implementation should be enough. But I'm not sure because I don't really know the effect you want to implement.

The second one is related to the Dexter usage itself. Whenever you invoke check method over a DexterInstance this is going to start a new activity. As you are doing it from the onResume method whenever is executed when the invisible activity Dexter creates is closed your onResume is triggered again and the new activity is created. This is not a library issue, the check method is designed to check the request the permission and check the resulting state. It shouldn't be used to know if the permission is granted or not, even when it's possible.

So the answer to how to fix this issue you are facing in the library usage is simple. Check using ContextCompat if the permission is granted before requesting the user to accept it if you want to do it from the onResume lifecycle. If you can, move the check execution to the onCreate method. Remember to invoke the token?.continuePermissionRequest() as I described in my previous comment.

As this is not a bug in the library. I'm closing the issue. Thank you for your time and patience, all your comments were really helpful to figure out what was going on.

rckahale commented 4 years ago

Call me dumb, but I don't get it.

The function token?.continuePermissionRequest() doesn't return Boolean , but simply calls the permission control dialog with enable/disable buttons How to decide whether there is a need to call subsequent, showSettingsDialog?

I have added the line to sample project. https://github.com/rckahale/doubleback

which is leading to not so desired output. actually more back clicks needed than 2

pedrovgs commented 4 years ago

@rckahale before invoking continuePermissionRequest first avoid using dexter everytime onResume method is invoked. That's the first step. Once this is ready, yo can go and check the usage of the token.