Permalink
Browse files

fix permission requests on pre-M android (#19734)

Summary:
On pre-M devices, `PermissionsAndroid.request` currently returns a boolean, whereas on newer, it returns GRANTED, DENIED or other constants defined in `PermissionsModule.java`

given the example form the [docs](https://facebook.github.io/react-native/docs/permissionsandroid.html) which I guess many people use, this will lead to code that does not work before M (it will tell you that permissions are not given even if they are in the manifest).

I believe the author of [this](51efaab) forgot to change the resolved value in this one place but changed it in other places, eg [here](51efaab#diff-2a74096453bc8faa5d4a1599ad0ab33fL99).

The docs are written correctly:

> On devices before SDK version 23, the permissions are automatically granted if they appear in the manifest, so check and request should always be true.

but the code is not right because we'd need to check for `if (granted === PermissionsAndroid.RESULTS.GRANTED || granted === true) {`

Also, the behavior is done correctly in [requestMultiplePermissions](https://github.com/facebook/react-native/blob/26684cf3adf4094eb6c405d345a75bf8c7c0bf88/ReactAndroid/src/main/java/com/facebook/react/modules/permissions/PermissionsModule.java#L148) so returning a boolean is an inconsistency.

I tested this locally. The code is the same as on line [148](https://github.com/facebook/react-native/blob/26684cf3adf4094eb6c405d345a75bf8c7c0bf88/ReactAndroid/src/main/java/com/facebook/react/modules/permissions/PermissionsModule.java#L148) where it is done correctly.

[ANDROID] [BUGFIX] [PermissionAndroid] - return GRANTED / DENIED instead of true / false on pre-M android

<!--
  **INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

    CATEGORY
  [----------]      TYPE
  [ CLI      ] [-------------]    LOCATION
  [ DOCS     ] [ BREAKING    ] [-------------]
  [ GENERAL  ] [ BUGFIX      ] [ {Component} ]
  [ INTERNAL ] [ ENHANCEMENT ] [ {Filename}  ]
  [ IOS      ] [ FEATURE     ] [ {Directory} ]   |-----------|
  [ ANDROID  ] [ MINOR       ] [ {Framework} ] - | {Message} |
  [----------] [-------------] [-------------]   |-----------|

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->
Closes #19734

Differential Revision: D8450402

Pulled By: hramos

fbshipit-source-id: 46b0b7f83f81d817d60234f155d43de7f57248c7
  • Loading branch information...
vonovak authored and grabbou committed Jun 15, 2018
1 parent ce73913 commit 6d27bd182fa87cab0c68ca281c869b987cbd2ca6
@@ -86,17 +86,17 @@ public void shouldShowRequestPermissionRationale(final String permission, final
}
/**
* Request the given permission. successCallback is called with true if the permission had been
* granted, false otherwise. For devices before Android M, this instead checks if the user has
* the permission given or not.
* Request the given permission. successCallback is called with GRANTED if the permission had been
* granted, DENIED or NEVER_ASK_AGAIN otherwise. For devices before Android M, this checks if the user has
* the permission given or not and resolves with GRANTED or DENIED.
* See {@link Activity#checkSelfPermission}.
*/
@ReactMethod
public void requestPermission(final String permission, final Promise promise) {
Context context = getReactApplicationContext().getBaseContext();
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) {
promise.resolve(context.checkPermission(permission, Process.myPid(), Process.myUid()) ==
PackageManager.PERMISSION_GRANTED);
PackageManager.PERMISSION_GRANTED ? GRANTED : DENIED);
return;
}
if (context.checkSelfPermission(permission) == PackageManager.PERMISSION_GRANTED) {

0 comments on commit 6d27bd1

Please sign in to comment.