Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

missing runtime permission request; causes crashes on Android 6 #3

Closed
eksperts opened this issue Apr 19, 2017 · 12 comments · Fixed by #4
Closed

missing runtime permission request; causes crashes on Android 6 #3

eksperts opened this issue Apr 19, 2017 · 12 comments · Fixed by #4

Comments

@eksperts
Copy link

This line here: https://github.com/MaxGraey/fuse-device/blob/master/Device.uno#L124
You can not ask for getDeviceId() until you've requested run-time permissions for READ_PHONE_STATE . The install-time permissions added above the GetUUID() function are not enough.

To solve this, I'd suggest taking a similar approach to what is seen here: https://github.com/bolav/fuse-contacts/blob/master/ContactsImpl.Android.uno#L147

@MaxGraey
Copy link
Owner

MaxGraey commented Apr 19, 2017

Hi, @eksperts. You are free to make PR for runtime permission. I'm pretty busy now and fuse is not in my priority list. I also have no Android device at hand to fully test.

@eksperts
Copy link
Author

Sadly, my hands are full at the moment too. I'll ping here if I get to it.

@eksperts
Copy link
Author

eksperts commented Jun 1, 2017

@MaxGraey you're up ^

@MaxGraey
Copy link
Owner

MaxGraey commented Jun 1, 2017

Can you implement sync UUID property using Future.Wait method instead async?
Something like:

public static extern(Android) string UUID()
  if (_authorizePromise == null) {
    _authorizePromise = new Promise<string>();
    Permissions.Request(Permissions.Android.READ_PHONE_STATE).Then(AuthorizeResolved, AuthorizeRejected);
    _authorizePromise.Wait();
  }
  return _authorizePromise.Result;
}

@eksperts
Copy link
Author

eksperts commented Jun 1, 2017

@MaxGraey nope, my Uno knowledge ends at what I've done

@eksperts
Copy link
Author

eksperts commented Jun 1, 2017

Also, I would like to advocate this approach.
Mostly because that allows you to implement a screen/button that explains why you need that particular permission before you ask for it.

@MaxGraey
Copy link
Owner

MaxGraey commented Jun 1, 2017

Ok, Just copy code above and test on your device if it fail, don't do father investigations

@eksperts
Copy link
Author

eksperts commented Jun 1, 2017

How's that argument above @MaxGraey ? Or you want the request to fire as soon as an app launches?

@MaxGraey
Copy link
Owner

MaxGraey commented Jun 1, 2017

Yes, just above part. I prefer init all lazily, with less launch-time code if possible

@eksperts
Copy link
Author

eksperts commented Jun 1, 2017

Not sure I got what you're saying. Is the async approach OK with you, or do you still insist on getting the static UUID working?

@MaxGraey
Copy link
Owner

MaxGraey commented Jun 1, 2017

Ok, I explain. if there are has possibility leave old sync approach with new permission implementation it would be nice, because we will keep compatibility with the previous interface

@eksperts
Copy link
Author

eksperts commented Jun 1, 2017

@MaxGraey ok, the motivation to not introduce breaking changes is clear. However, the current implementation crashes on Android 6, so this breaking change might still be very welcome for Android target.

With that in mind, I gave it a go and reimplemented the static UUID member on the class that works just like before on iOS and Local targets, and a bit differently on Android. Please see the latest commit in the pull request for details.

I have to point out that I have literally no idea how you could return a dynamic future string in a static context. That .Wait() and .Result suggestion didn't work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants