search

Baseflow / flutter-permission-plugins

This repo contains a collection of permission related Flutter plugins which can be used to request permissions to access device resources in a cross-platform way.
https://baseflow.com
MIT License
52 stars 34 forks source link

shouldShowRequestPermissionRationale always returns false #10

Closed hikaritenchi closed 4 years ago

hikaritenchi commented 5 years ago

🔙 Regression

shouldShowRequestPermissionRationale() receives a Context, but updates to onMethodCall() guarantees that it is not an Activity, and will always log an error and return false.

Old (and correct) behavior

onMethodCall() saved a final reference to a Context via a deleted method getActiveContext(), which used either mRegistrar.activeContext() or mRegistrar.activity().

That Context was passed to shouldShowRequestPermissionRationale(), which checked if it was instanceof an Activity, which would succeed if mRegistrar.activity() was called in getActiveContext().

Current behavior

onMethodCall() saves a final reference to a Context via mRegistrar.context(). From the documentation, that method returns the Application's Context, which by definition will not be an Activity, thereby guaranteeing the error in shouldShowRequestPermissionRationale() and returning false.

Reproduction steps

Call LocationPermissions().shouldShowRequestPermissionRationale() in Flutter.

Suggested Steps

In onMethodCall(), I suggest replacing final Context context = mRegistrar.context(); with final Context context = mRegistrar.activeContext(); The documentation states that it returns the current Activity, or the Application if that's null, which is exactly what the getActiveContext() method was trying to do. As I'm not familiar with the rest of the logic, a cursory review of the other methods that accept a Context from onMethodCall() seem to be unaffected by this change.

Relevant Commit

8bd14831582fda4b74855e9dabc43fe4c5f99307

Version: 2.0.1

Platform:

mvanbeusekom commented 5 years ago

@hikaritenchi thank you very much for reporting this bug and including the extensive analysis. I will try to fix this a.s.a.p. and run an update this week.

cremor commented 5 years ago

@mvanbeusekom Has this been fixed? Because the changelog of version 2.0.2 says

Fixed bug where method shouldShowRequestPermissionRationale always returns false.

but the PR #11 has not been merged. I've also checked the commit history and except for 71c5017d56f9da08a7489e1ac8d6a4ad522f3a4c, which only changed the version number, there seems to have been no code change between 2.0.1 and 2.0.2.

Also, when I check the code of version 2.0.2 in my local pub-cache folder, the changes of PR #11 are not there.