[Android API 23+] Request for permissions on runtime by livecodepanos · Pull Request #6614 · livecode/livecode

livecodepanos · 2018-07-26T11:18:29Z

No description provided.

…runtime

montegoulding · 2018-07-27T08:34:32Z

@livecodepanos I think it would make things easier to have an api and callback handler in mblandroiddc.cpp and call it before calling anything that needs the permission. That way it's easier to setup the a static sentinel variable and a wait loop while waiting for the callback. Also it would hopefully be easier to reuse the same code in all the places required.

montegoulding · 2018-07-27T08:36:08Z

MCScreenDC::popupanswerdialog & Java_com_runrev_android_Engine_doAnswerDialogDone is an example of what I mean with the wait loop:

while(s_in_popup_dialog)
		MCscreen -> wait(60.0, !p_blocking, True);

livecodepanos · 2018-07-27T17:32:35Z

still WIP until adding a androidRequestPermission command so apps can explicitly request permission at an appropriate time

montegoulding · 2018-07-28T00:16:03Z

+    MCAndroidEngineRemoteCall("askPermission", "bx", &t_result, p_permission);
+
+    while (s_in_permission_dialog)
+        MCscreen -> wait(60.0, True, True);


@livecodepanos I think it might be simpler to make this a blocking wait MCscreen -> wait(60.0, False, True); as if we are using a dispatching wait it's feasible that there could be another request during it and the implementation would need to be much more complicated to handle that.

If we do need dispatching wait then I think we would need to generate a request code from a sequence static uint and have s_in_permission_dialog and s_permission_granted be maps of request code -> bool and include the request code as a param in Java_com_runrev_android_Engine_doAskPermissionDone the correct wait loop can be broken.

montegoulding · 2018-07-28T00:19:42Z

-		}
-	}
-
+    public static final int CAMERA_PERMISSION_REQUEST_CODE = 1;


If we aren't actually using the request code then we could just call all requests with 1. If we do need dispatching waits (see the other comment) then we probably need a unique code to come from the engine and map to the actual request.

montegoulding · 2018-07-29T22:03:25Z

+
+Introduced: 9.0.1
+
+OS: mobile


@livecodepanos OS and Platform entries here need to be swapped

montegoulding · 2018-07-29T22:06:43Z

+local tCameraPermissionGranted
+put androidRequestPermission("android.permission.CAMERA") into tCameraPermissionGranted
+if not tCameraPermissionGranted then
+   answer "This app is not permitted to access the device camera. You can change this \


@livecodepanos this does not compile. Should be:

answer "This app is not permitted to access the device camera. You can change this" & \ "in the Settings app."

Interestingly enough I chatted to @runrevmark once about \ escaping newlines in string literals and it's both feasible and backwards compatible to do at some point.

Good catch @montegoulding :)

I guess I need to use && instead of & (or add a space at the end of the first string)

montegoulding · 2018-07-29T22:14:38Z

 Boolean MCAndroidSystem::GetStandardFolder(MCNameRef p_folder, MCStringRef &r_folder)
 {
+    // accessing "external documents", "external cache" etc requires Write External Storage permission
+    if (MCStringBeginsWith(MCNameGetString(p_folder), MCSTR("external"), kMCStringOptionCompareCaseless) && !MCAndroidCheckRuntimePermission(MCSTR("android.permission.WRITE_EXTERNAL_STORAGE")))


@livecodepanos can you put everything after && onto a new line here

montegoulding · 2018-07-29T22:15:00Z

+{
+    if (p_sensor == kMCSensorTypeLocation)
+    {
+        bool t_success = MCAndroidCheckRuntimePermission(MCSTR("android.permission.ACCESS_COARSE_LOCATION")) && MCAndroidCheckRuntimePermission(MCSTR("android.permission.ACCESS_FINE_LOCATION"));


@livecodepanos again with the newline

montegoulding · 2018-07-29T22:28:36Z

@panos.merakos there’s a couple of minor patches to make but I’m also wondering if the LCS api should use a case insensitive enum camera, coarselocation, ... rather than the permission strings directly. I guess the advantage of the way it is implemented is any new permissions will work with it without patching. The disadvantage is at the moment there's no checking if the permission string is valid. What happens if you try to request foo? Perhaps there's an exception we need to handle if we expose the API this way.

livecodepanos · 2018-07-29T22:49:23Z

Hehe I was thinking the same (i.e. to request just camera rather than the exact permission string), but I left it as it is now for being able to add more permissions in the future. Currently if you request permission for foo the function will just return false

montegoulding · 2018-07-29T22:56:38Z

OK, I think we had better check with @runrevmark on the API to see what he thinks before reviewing OK. I'm not sure we expose many things in a low level way like this in the engine although I agree it does reduce longer term maintenance.

runrevmark · 2018-07-30T04:46:23Z

I suggest:

function androidPermissionExists pTag -- returns true if pTag identifies a known permission
function androidHasPermission pTag -- returns true if the permission identified by pTag is granted, throws if pTag is not an existing permission
command androidRequestPermission pTag -- requests the given permission, throwing if pTag is not a known permission

Note: RequestPermission does not return anything. Grant of permission is done with HasPermission subsequently.

I suggest pTag is either a full permissions name (which is done case-sensitively) or if there is no '.' then it builds it as .toUpper(pTag).

montegoulding · 2018-07-30T22:25:31Z

-put androidRequestPermission("android.permission.CAMERA") into tCameraPermissionGranted
+put androidHasPermission("android.permission.CAMERA") into tCameraPermissionGranted
+if not tCameraPermissionGranted then
+   androidRequestPermission("android.permission.CAMERA")


@livecodepanos you should drop the parentheses here and from the syntax element

montegoulding · 2018-07-30T22:26:23Z

+local tLocationPermissionGranted
+put androidHasPermission("android.permission.ACCESS_FINE_LOCATION") into tLocationPermissionGranted
+if not tLocationPermissionGranted then
+   androidRequestPermission("android.permission.ACCESS_FINE_LOCATION")


Parentheses again

montegoulding · 2018-07-30T22:34:24Z


+bool MCSystemRequestPermission(MCStringRef p_permission, bool& r_granted)
+{
+    r_granted = MCAndroidCheckRuntimePermission(p_permission);


@livecodepanos you need to add do-nothing stubs for these to mbliphonemisc.mm I think

montegoulding · 2018-07-30T22:55:57Z

+permissionName (enum):
+The name of the permission to request.
+
+-   "android.permission.CAMERA": permission to access the device camera.


@livecodepanos should this list all the dangerous permissions: https://developer.android.com/guide/topics/permissions/overview#permission-groups

montegoulding · 2018-07-31T09:56:37Z

@livecode-vulcan review ok 39b59ee

livecode-vulcan · 2018-07-31T09:58:50Z

💙 review by @montegoulding ok 39b59ee

[Android API 23+] Request for permissions on runtime

livecode-vulcan · 2018-07-31T11:04:22Z

😎 test success 39b59ee

try-community-armv6-android-sdk26_ndk16r15: success
try-community-armv7-android-sdk26_ndk16r15: success
try-community-arm64-android-sdk26_ndk16r15: success
try-community-x86-android-sdk26_ndk16r15: success
try-community-x86_64-android-sdk26_ndk16r15: success
try-community-js-emscripten-sdk1.35: success
try-community-universal-ios-iphoneos11.4: success
try-community-universal-ios-iphonesimulator11.4: success
try-community-universal-mac-macosx10.9: success
try-community-x86-linux-debian8: success
try-community-x86_64-linux-debian8: success
try-community-x86-win32: success
try-community-x86_64-win32: success

livecodepanos added this to the 9.0.1-rc-2 milestone Jul 26, 2018

[Android API 23+][mobilePickPhoto] Request for camera permissions on …

5264d86

…runtime

livecodepanos force-pushed the bugfix-20427 branch from 4b9bda3 to 5264d86 Compare July 26, 2018 11:19

livecodepanos added bug blocker labels Jul 26, 2018

[Android API 23+] Request for location permissions on runtime

9a8c15b

livecodepanos changed the title ~~[Android API 23+][mobilePickPhoto] Request for camera permissions on runtime~~ [Android API 23+] Request for permissions on runtime Jul 26, 2018

Removed unnecessary comparison

0b857f1

livecodepanos added the WIP label Jul 27, 2018

livecodepanos added 2 commits July 27, 2018 18:21

[Runtime permissions] Created API that checks for permission

3224c7d

Removed unused code

912d98e

montegoulding reviewed Jul 28, 2018

View reviewed changes

livecodepanos added 2 commits July 28, 2018 07:55

Used blocking wait

0509d62

Implemented "androidRequestPermission()" function

5c9490e

livecodepanos added need-docs need-release-notes and removed WIP labels Jul 28, 2018

livecodepanos added 2 commits July 29, 2018 17:44

Added feature note

e5a704e

[androidRequestPermission] Added dictionary entry

62f1651

montegoulding reviewed Jul 29, 2018

View reviewed changes

Fixed a couple of issues on docs and with long lines

4c34eaf

Added "androidHasPermission" and "androidPermissionExists" commands

d2c5a3a

livecodepanos removed need-docs need-release-notes labels Jul 30, 2018

montegoulding reviewed Jul 30, 2018

View reviewed changes

livecodepanos added 2 commits July 31, 2018 09:30

Added stubs for iphone and updated docs

120b71f

[androidRequestPermission] Docs: removed parentheses from syntax

39b59ee

livecode-vulcan added the vulcan/reviewed label Jul 31, 2018

livecode-vulcan added a commit that referenced this pull request Jul 31, 2018

Auto-merge pull request #6614 from livecodepanos/bugfix-20427

07f96a9

[Android API 23+] Request for permissions on runtime

livecodepanos merged commit 720de35 into livecode:release-9.0.1 Jul 31, 2018

livecodepanos mentioned this pull request May 15, 2019

[[ AndroidPermissionsLCB ]] Added permission checking handlers to and… #7027

Merged


		Introduced: 9.0.1

		OS: mobile

Uh oh!

Conversation

livecodepanos commented Jul 26, 2018

Uh oh!

montegoulding commented Jul 27, 2018

Uh oh!

montegoulding commented Jul 27, 2018

Uh oh!

livecodepanos commented Jul 27, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

montegoulding commented Jul 29, 2018

Uh oh!

livecodepanos commented Jul 29, 2018

Uh oh!

montegoulding commented Jul 29, 2018

Uh oh!

runrevmark commented Jul 30, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

montegoulding commented Jul 31, 2018

Uh oh!

livecode-vulcan commented Jul 31, 2018

Uh oh!

livecode-vulcan commented Jul 31, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants