PromiseKit / CoreLocation

Promises for Swift & ObjC
http://promisekit.org
MIT License
30 stars 29 forks source link

Can't ask for always authorization after a location is requested #16

Closed ricsantos closed 6 years ago

ricsantos commented 6 years ago

Similar to #4 but with a nuance.

In iOS 11, it is advised to first ask the user for whenInUse location authorization, and then at a later time upgrade that to always authorization.

Using PromiseKit/CoreLocation 6.2.5, the request for always will fail to return if whileInUse has been granted and a location has been requested.

Here is a small chain that demonstrates the issue:

_ = firstly {
    CLLocationManager.requestAuthorization(type: .whenInUse)
}.then { _ in
    CLLocationManager.requestLocation()
}.then { _ in
    CLLocationManager.requestAuthorization(type: .always)
}.done { _ in
    print("tada")
}

In order to test this in a project, the usage description keys will need to be added to the Info.plist file:

<key>NSLocationWhenInUseUsageDescription</key>
<string>When In Use</string>
<key>NSLocationAlwaysUsageDescription</key>
<string>Always</string>
<key>NSLocationAlwaysAndWhenInUseUsageDescription</key>
<string>Always and When In Use</string>

Running that promise, the log tada is never printed. By removing the requestLocation promise from the chain, it all works as expected.

I also have a small demo project reproducing this, I can supply if that helps.

mxcl commented 6 years ago

Test project would help thanks.

ricsantos commented 6 years ago

Here's a test project: https://github.com/ricsantos/CoreLocationTest

In the simulator the second alert might briefly appear, perhaps indicating that the CLLocationManager instance is being deallocated prematurely.

mxcl commented 6 years ago

For me the second alert stayed visible, but was being triggered by the requestLocation, I ran multiple times and never would it only appear briefly. Which is worrying if you saw that.

I can't see how it can be deallocated though, the instance of CLLocationManager has a retain cycle placed on it.

ricsantos commented 6 years ago

Drat. I had a colleague run the same test on their practically identical setup and both permission alerts displayed correctly... on the simulator. On device however the issue was reproduced.

I see you have made a PR, thank you heaps. I was going to give it a crack myself but by the looks of the diff it was quite a rework.

ricsantos commented 6 years ago

FWIW, I've checked out your branch (commit 13bd096) (using CorePromise + submodules) in the test project and it fixes the issue on both device and simulator.

mxcl commented 6 years ago

Good to hear. I’ve pushed this with promisekit 6.2.6