Skip to content

Commit

Permalink
fix permission requests on pre-M android (#19734)
Browse files Browse the repository at this point in the history
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 facebook-github-bot committed Jun 15, 2018
1 parent 86d8c7a commit 4e1abdd
Showing 1 changed file with 4 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 4e1abdd

Please sign in to comment.