[TIMOB-25322] iOS: Geolocation should be able to handle iOS 11 permission upgrade #9458
Conversation
|
@hansemannn I understand that you choose to be backwards compatible, but this is not going to work out if you want to support iOS 11 from a functional perspective. If you go for the permission upgrade strategy, then this will work now in iOS11, but on iOS 10 you don't get the chance to upgrade. You'll have to add quite some checks on the javascript side to be back- and forwards compatible. Question is why you weren't allowed to upgrade before? This is quite odd.... isn't it? |
|
@jvandijk From my understanding, this was not technically possible from Apple's side before, so we guarded it in the past. If it was, we can remove those guards for a better code-structure. The iOS < 11 checks for incremental updates are correct and seem to be necessary from what I know by now. |
| [locationManager requestWhenInUseAuthorization]; | ||
| } else { | ||
| NSLog(@"[ERROR] The keys NSLocationAlwaysUsageDescription or NSLocationWhenInUseUsageDescription are not defined in your tiapp.xml. Starting with iOS8 this is required."); | ||
| if ([CLLocationManager authorizationStatus] != kCLAuthorizationStatusAuthorizedAlways && |
There was a problem hiding this comment.
Perfect addition! Already ran into issues in the past due to this 'auto' permission request logic.
|
I've been testing this way of implementing it and also ran into the fact that the didChangeAuthorizationStatus is firing on showing the 2nd authorization request. The code in that method is also not capable of handling the authorization, and could fire the callback unnecessary. |
|
@jvandijk You mean upgrading the permissions on iOS < 11? So it's not worth looking into it in the SDK? I really focussed on the iOS 11 changes for this one. For the changes, I can also think of two ones for the client (if you want the improved UX): Normal geo-request:
Going background / Always-auth / geofencing:
|
|
No it is worth looking into the SDK, because I experienced this on iOS 11 indeed. The |
|
Can you shoot me a test-case? EDIT: PR updated and tested against iOS 10.x and iOS 11. Seems to work! 🙏 Also improved the warn-message when calling location-services without permissions to print a well-formatted log of options. |
|
@jvandijk Ping? |
|
@hansemannn Sorry, was too busy with other things. In my situation I add an eventlistener to the resume event (coming back from the permission dialog) and in that eventlistener verify if the permission has been set to update the button state. That's the only difference from the described testcase on Jira. |
|
No worries. Did you test the latest version? |
|
Tested with 6.2.2 |
|
I mean, did you test the latest PR changes? |
|
@hansemannn I have just one more concern... and that's the hasLocationPermissions method. When checking |
|
Good question. I think it should / could show a warning but return |
|
It does not behave differently, just that |
|
That‘s correct, but if you want to check that permission, you would explicitly ask for it. In other words: I cannot see a use-use where this would fail. |
|
@ewieberappc Just resolved a linting-issue, so this is now definitely ready for QE 🙂. |
Generated by 🚫 dangerJS |
|
When following the steps from the ticket, I get an error that the NSLocationWhenInUseUsageDescription key must be defined. It seems that NSLocationAlwaysAndWhenInUseUsageDescription is not covering the "just when-in-use" request case. |
|
@ewieberappc Can you send me your test project? |
|
Closing this one in favor of the above linked one that handles this one + more improvements regarding the new auth-flow and the requirement of the when-in-use key. |
JIRA: https://jira.appcelerator.org/browse/TIMOB-25322
Unit-Tests not possible as it require's UI-interaction with the confirmation-dialog. Might be a good candidate for UI-tests in our Appium test-suite.