-
Couldn't load subscription status.
- Fork 150
Fix location permissions warning issue (closes #193) #194
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
Fix location permissions warning issue (closes #193) #194
Conversation
|
Thanks! I will take a look at it later. It also appears that the iPhone size detection is not working correctly on the iPhone 12 sizes too... Something to fix later for myself |
|
Oh wow, not sure what code could be causing this. Will look into it over the coming days. Marking this PR as draft till then... |
|
Interestingly, I can't make it throw that exception when I have a breakpoint right before the |
|
Honestly I'm having a hard time reproducing the issue - I got it a few times but it is not reliable, and I've got no idea what is triggering it. I will keep trying. I think it's too sketchy to merge. Have you had more luck? In my experience sometimes Core Data threading issues rear their head with EDIT: Based on your screenshot and what I've seen it's interesting that the crash happens on a background thread. I think that's a clue. |
|
On device, getting this. Metal-related. might not be caused by this pr, but rather maybe a new iOS 14 thing Also: this when it doesn't crash viewController:: didEnterBackground
2020-12-22 17:08:06.814686+0800 OpenGpxTracker[419:139392] [Renderer] IconRenderer: Left horizontal padding (18.000000) plus right horizontal padding (18.000000) is larger than the image width (34.000000). Image will now use the center column of pixels to stretch.
2020-12-22 17:08:07.019343+0800 OpenGpxTracker[419:139397] [Renderer] IconRenderer: Left horizontal padding (18.000000) plus right horizontal padding (18.000000) is larger than the image width (34.000000). Image will now use the center column of pixels to stretch. |
|
I haven't checked if this is still replicable in more recent iOS 14 builds, but I feel maybe the benefits outweigh the risks? (if we are doing an update anytime soon) |
Fixed an issue where a location permissions warning alert controller was presented in all cases for a fresh install of the app. The problem was that the app did not check if the location permissions were requested before, so even if the user was never presented with a prompt the warning case would trigger - meaning users would always be shown the permissions denied warning no matter what they chose. The fix is to rely on the `CLLocationManagerDelegate` call back of `locationManagerDidChangeAuthorization(_ :)` which is called both at initialization time of `CLLocationManager` and when permissions are changed. The check at `appDidBecomeActive` time has been removed because the aforementioned delegate call accomplishes the same thing.
e3ca437 to
db3aca1
Compare
I've just rebased the branch and ran the app a few times - no issues. But I have not tested it to where I can say that it's not crashing anymore. Will check more tomorrow. It sounds like you're planning a release soon? |
|
Welp - got the crash. I just panned around then swiped for the app to go into the background. Same stuff as before. iPhone 12 mini iOS 14.5.1 Will continue to investigate. I just really don't see what code here could be the issue 👀 |
That’s not really something I can say, but it seems like iOS 14 related bugs are pilling up, and iOS 15 is around the corner. So it’s good to prepare. I forgot the crash that this originally caused, and have opened #203. Different place, Same error in console. Since it occurs in a different place, this should be more of a app-wide thing then with your patch… I wonder what causes it though… so random. Users of the App Store published version might be seeing this crash too, as I am receiving feedback about crashes on recently, running iOS 14 |
|
As with #202, I decided to merge this today. Thank you @SebastianWild. Sorry for the long wait. |


Summary
Fixed an issue (#193) where a location permissions warning alert controller was presented in all cases for a fresh install of the app.
Root Cause
The problem was that the app did not check if the location permissions were requested before, so even if the user was never presented with a prompt the warning case would trigger - meaning users would always be shown the permissions denied warning no matter what they chose.
The fix is to rely on the
CLLocationManagerDelegatecall back oflocationManagerDidChangeAuthorization(_ :)which is called both at initialization time of
CLLocationManagerand when permissions are changed.The check at
appDidBecomeActivetime has been removed because the aforementioned delegate call accomplishes the same thing.See below:

My first contribution here, please let me know if anything isn't up to snuff :D
Especially on the watch side of things - I applied the same changes but am not 100% on the CoreLocation APIs there.