jamesmontemagno / GeolocatorPlugin

Geolocation plugin for Xamarin and Windows
MIT License
294 stars 158 forks source link

GetPositionAsync issue when choosing "Only when using the App" rather then "Always Allow" #274

Closed nbsoftware closed 6 years ago

nbsoftware commented 6 years ago

Bug Information

Version Number of Plugin: 4.5.0.6 Device Tested On: iPhone 6S - iOS 11.4 Simulator Tested On: iPhone 8 - iOS 11.4 Version of VS: VS for Mac 7.6.3 Version of Xamarin: 11.14.0.14

Steps to reproduce the Behavior

Set up an app that requests background location updates When app prompts for permissions, select "Only when using the App" rather than "Always Allow"

Expected Behavior

GetPositionAsync should still return the location as if we had chosen the "Always Allow" i.e. without hanging.

Actual Behavior

GetPositionAsync hangs for a few seconds (see traces below) and complains. Eventually, it exits with the correct location and without exception.

2018-09-05 08:27:24.970 fipacapp.iOS[7747:5161304] Currently does not have Location permissions, requesting permissions
2018-09-05 08:27:24.987 fipacapp.iOS[7747:5161304] AuthorizedWhenInUse
...
2018-09-05 08:27:32.992 fipacapp.iOS[7747:5161304] Location permission denied, can not get positions async.

It works as expected though if we had chosen "Always Allow".

Worth mentioning, Geolocator version 4.2.0 (which also requires downgrading Permissions to 2.2.1) works as expected in both scenario.

jamesmontemagno commented 6 years ago

Correct, this is how I had to implement it as Apple doesn't raise an event so I wait a while and return a result. Nothing I can do. Don't add the background permit if you don't need it.

nbsoftware commented 6 years ago

Hi James,

Well I don't get your answer... there are a number of apps out there that request for the "Always" permissions for which if we only allow "Only when Using" don't stop from working even on a subset of the possible functionalities.

Meaning, if the system allows for another answer than "Always", choosing it shouldn't break the app, only restrict the way it is allowed to work.

At least, the 4.2.0 version of your plugin does also trace the "Currently does not have Location permissions" and "AuthorizedWhenInUse" trace messages when calling GetPositionAsync but returns as soon as it gets a valid location which is better then the current behavior.

Because of that, I'm forced to stick with this 4.2.0 version rather than the newer one.

jamesmontemagno commented 6 years ago

Perhaps I missunderstood what was going on in your original bug. It is hard without having sample code to fully understand what is being called.

It seems as though what when you called "GetPositionAsync" that it eventually returned and gave you back the position and future calls did not have the issue?

I have a longer thread on how I fixed it here: https://github.com/jamesmontemagno/PermissionsPlugin/issues/75

nbsoftware commented 6 years ago

Yes it seems very related to that issue. But not quite exactly the same, mine being probably a consequence of what you did try to fix #75

With the latest version of the Geolocator plugin, when answering "Only when using the App" rather than "Always Allow", GetPositionAsync always return the correct location but within the timeout you probably set waiting for the event that should fire if the user had chosen "Always", event which you say never fires in that specific case.

GetPositionAsync calls CheckPermissions, so if the latter isn't working properly, the former won't either.

I'll try to make some complementary tests with your sample to see if I can pinpoint the exact issue.

nbsoftware commented 6 years ago

Just a suggestion reading the source... could GetPositionAsync only request for Permission.LocationWhenInUse rather then the full Permission.Location (I suppose that this is why it gets a denied...) at https://github.com/jamesmontemagno/GeolocatorPlugin/blob/master/src/Geolocator.Plugin/Apple/GeolocatorImplementation.apple.cs#L230 ?

Permission.LocationAlways is requested further down anyway and only if it is required (iOS > 9, background location updates enabled): https://github.com/jamesmontemagno/GeolocatorPlugin/blob/master/src/Geolocator.Plugin/Apple/GeolocatorImplementation.apple.cs#L258

anthonypuppo commented 6 years ago

To further build onto this I had an issue where requesting Permission.Location on iOS just caused it to hang forever. Switching to Permission.LocationWhenInUse fixed it.

nbsoftware commented 6 years ago

Hey James, What do you think about my suggestion? Have you had the time to think about it? If valid, do you think you'll be able to release a new version of the package anytime soon? Thanks

jamesmontemagno commented 6 years ago

Can you make a PR

nbsoftware commented 6 years ago

There you go. PR #278

nbsoftware commented 6 years ago

Hi James,

I just tried with the new 4.5.1-beta version of Geolocator (which I assume has the fix) and with the I suppose related 4.0.0-beta version of Permissions... now requesting Location requires NSAppleMusicUsageDescription?!

Is it some kind of mix up?

I tried to downgrade only Permissions to 3.0.0.12 and no weird NSAppleMusicUsageDescription requirement, but behavior is the same as described at the top of this issue...

jamesmontemagno commented 6 years ago

Where and how do you see this error message?

jamesmontemagno commented 6 years ago

If you got this email from apple then their scanner must have changed and now are trying to force even if the API is in the app :( had this issue with contacts too, which is what 4.0 takes place. Looks like iOS 12 sdk may do something odd. I am pushing 4.0.1-beta of permissions for you to try out.

nbsoftware commented 6 years ago

Just for you know, I was seeing this message on the device itself when requesting the Location permission. Now 4.0.1-beta seems to have solved it. And a full clean, restore package, remove bin & obj dirs and rebuild seemed to also let the fix in Geolocator 4.5.1-beta do its job. All in all, everything seems now to work as it should. Thanks

jamesmontemagno commented 6 years ago

Yeah, most likely because i removed the enum from it so it was messed up. I updated and pushed out a new geolocator plugin built against it :)