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

Request "Approximate" location along to "Precise" location will be required in Android 12 #207

Closed
mrsarm opened this issue Aug 18, 2021 · 10 comments · Fixed by #263
Closed
Assignees
Labels
Priority: 3 - Low Can be bumped from the release Type: Technical issue Improve something that users won't notice
Milestone

Comments

@mrsarm
Copy link
Contributor

mrsarm commented Aug 18, 2021

Android 12 will require apps to request "Approximate" location (ACCESS_COARSE_LOCATION) if they try to also access "Precise" location, check https://developer.android.com/about/versions/12/behavior-changes-12#approximate-location .

We have declared that permission in the app manifest, but at runtime we are only requesting the fine location. Not requesting the coarse granularity may cause Android to reject the access to the permission at runtime, and without any visual warning about it, only an error message in the device log is recorded (check https://developer.android.com/about/versions/12/approximate-location).

Also as mention in #189 , a refactor of how the location is requested is needed.

@mrsarm mrsarm changed the title Android 12: Request coerce" Request "Approximate" location along to "Precise" location will be required in Android 12 Aug 18, 2021
@mrsarm mrsarm added the Type: Technical issue Improve something that users won't notice label Aug 18, 2021
@garethbowen
Copy link
Member

As part of this we should track which users select which option (none, coarse, or fine), ie: show it on the About page, and store it in user telemetry. We could also go further and track all permissions that we request.

Ideally we would also include a way for users to change, so if they've picked coarse they can later launch the prompt again and change their permissions in the app.

Just my 2c... I'm not sure how hard this is, but it would be really useful to track in telemetry!

@mrsarm mrsarm self-assigned this Aug 19, 2021
@mrsarm
Copy link
Contributor Author

mrsarm commented Aug 19, 2021

Testing the App with Android 12 beta in an emulator, found the following results.

The geolocation didn't work well with the app as it is now (without changing the source code or the targeted SDK). The new UI that allows to select which precision of location is displayed as pointed out in the official documentation, and when selected fine precision, the geolocation data was collected as expected:

  "geolocation": {
    "latitude": 37.421999,
    "longitude": -122.0839994,
    "altitude": null,
    "accuracy": 20,
    "altitudeAccuracy": null,
    "heading": null,
    "speed": null
  }-

But, if the user picks approximate location instead of precise location, it failed to collect the geolocation, without any notice to the user. This is what we get in the record:

  "geolocation": {
    "code": -1,
    "message": "Geolocation timeout exceeded"
  },

This is bad because the user has the impression that it provided at least an aprox location but it didn't. The solution according with the documentation is to ask both permissions not just fine permission.

Targeting SDK 31 the problem is worst, because as it's pointed out in the documentation, the permission is rejected by Android, so after accepting the prominent disclosure the Android OS reject the permission automatically, and again the user has the impression that already accepted because in this case at least accepted the prominent disclosure.

Even though we don't plan in the near future to use any of the new features from Android 12 , it's likely that at some point the Play Store will force new updates of the app to target Android 12, as it does now requesting to target at least Android 11, but as mentioned above, the geolocation does not work properly in Android 12 either way, and this only affects Android 12, although targeting Android 12 without fixing the code makes the geolocation work worst.

As part of this we should track which users select which option (none, coarse, or fine), ie: show it on the About page, and store it in user telemetry. We could also go further and track all permissions that we request.

Currently if the user reject the location permission either from the prominent disclosure or from the Android UI after the disclosure, this is recorded:

  "geolocation": {
    "code": 2,
    "message": "application does not have sufficient geolocation permissions."
  },

If we implement the changes to request both aprox or precise permission yes there is a way to check which one the user has chosen.

Ideally we would also include a way for users to change, so if they've picked coarse they can later launch the prompt again and change their permissions in the app.
Just my 2c... I'm not sure how hard this is, but it would be really useful to track in telemetry!

Record in Telemetry shouldn't be so hard and can be implemented in the same PRs although will require CHT changes, but allow users to change its decision later is indeed harder and because it would require UI changes we should create a separated issue for that.

@mrsarm mrsarm added this to the 0.10.0 milestone Aug 24, 2021
@mrsarm mrsarm added the Priority: 1 - High Blocking the next release label Aug 24, 2021
@mrsarm mrsarm removed their assignment Aug 24, 2021
@mrsarm mrsarm modified the milestones: 0.10.0, 0.11.0 Sep 14, 2021
@mrsarm mrsarm modified the milestones: 0.11.0, 1.0.0 Nov 25, 2021
@craig-landry craig-landry added Priority: 2 - Medium Normal priority and removed Priority: 1 - High Blocking the next release labels Mar 11, 2022
@craig-landry
Copy link
Member

Temporarily making this "medium" priority as it pertains to the 1.0 release until we agree on if it must be included now or not.

@craig-landry craig-landry added Priority: 3 - Low Can be bumped from the release and removed Priority: 2 - Medium Normal priority labels Mar 11, 2022
@latin-panda
Copy link
Contributor

latin-panda commented Mar 25, 2022

We're already requesting for ACCESS_FINE_LOCATION and ACCESS_COARSE_LOCATION in the manifest and during run time.

What is left to do is:

  • Track the granted permissions in telemetry (webapp's service): ACCESS_FINE_LOCATION, ACCESS_COARSE_LOCATION and I guess READ_EXTERNAL_STORAGE too. It'll be resolved in this ticket.
  • Display them in the About page. It'll be resolved in this ticket.
  • Check if webapp is capturing approximate location (ACCESS_COARSE_LOCATION) data correctly.
  • Stop saving the flag in the SettingsStore when the permission is denied, so to display the prominent disclosure whenever the webapp requires location and one of these permissions is missing.

I recently upgraded my personal phone to Android 12 but I don't get the option to choose between accurate or approximate.
Doing this ticket after upgrading code to SDK 31 (Android 12) to appropriately test the feature.

@garethbowen I'm assuming that webapp code changes are going to be in v4.0.0.

@garethbowen
Copy link
Member

Stop saving the flag in the SettingsStore when the permission is denied, so to display the prominent disclosure whenever the webapp requires location and one of these permissions is missing.

I like this approach. I've always worried about what happens if someone changes their mind. However, what happens if someone declines? Will we prompt them again next time or does Android handle that?

I'm assuming that webapp code changes are going to be in v4.0.0.

Making sure coarse location works as expected (number 3 in your list) should definitely be done as part of this issue and included in either cht-android v1.0.0 or cht-core 4.0.0.

The two changes about telemetry and the about page is not a blocker for 4.0.0. Please raise an issue in cht-core for these two points.

@latin-panda
Copy link
Contributor

I've created a ticket for Number 1 and 2 of that check list.

what happens if someone declines? Will we prompt them again next time or does Android handle that?

We ask them again whenever the user need the permission and this one isn't granted yet. We get the grant automatically after they accept the prompt. But depending of the phone and Android version, if they refuse for a second time or click on the option Never Ask Again (option not available in some phones), we need to redirect them to the CHT Android's system settings page to manually grant the permission.
See implementation in this PR.

@garethbowen
Copy link
Member

@jkuester Is this ready for AT?

@jkuester
Copy link
Contributor

jkuester commented May 3, 2022

@garethbowen yes! This is ready. Sorry for the delay here. I was getting my ducks in a row on the Android 12 issue before pushing this one forwards.

This issue is ready for AT on https://github.com/medic/cht-android/tree/207-location-persmission-sdk31. (Note that the changes in this branch have been made on top of the Android 12 branch that is also under review in #262. We should be able to AT these simultaneously and the merge them both.)

The only behavior change here is that on Android 12, it is possible for the user to only grant COARSE location accuracy (instead of FINE location accuracy) even though we request both. So, on an Android 12 device, if a user only allows COARSE (when trying to create their first report), we should continue successfully and create the report, but the next time the user tries to create a report, we should prompt them again to grant the location permissions since we want the FINE permission. Once the user has granted the FINE permission we should stop prompting them for location permissions every time they fill out the report...

@tatilepizs tatilepizs self-assigned this May 3, 2022
@tatilepizs
Copy link

@mrjones-plip
Copy link
Collaborator

Testing this APK on Pixel 3a with Android 12 with GPS radio on and starting to fill a form:

  1. I first get prompted to turn on GPS

    turn on prompt screenshot

    turn on prompt

  2. I then get prompted to choose from fine or coarse. I choose Coarse (note circle on right icon)

    Coarse screenshot

    coarse access

  3. I'm able to fill out the form. I then go in and choose the same form again

  4. I then get prompted to choose from only fine. I choose allow

    fine screenshot

    fine access

  5. I'm able to fill out the form. I then go in and choose the same form again and I get no GPS prompts.

NB - I was using the standard config and the "New Pregnancy" form. For all the first two I got a permissions note in the JSON:

JSON
  "geolocation_log": [
    {
      "timestamp": 1651788892798,
      "recording": {
        "code": 2,
        "message": "application does not have sufficient geolocation permissions."
      }
    }
  ],
  "geolocation": {
    "code": 2,
    "message": "application does not have sufficient geolocation permissions."
  },

And for the last one I got entries in both:

JSON
  "geolocation_log": [
    {
      "timestamp": 1651788913378,
      "recording": {
        "latitude": 22.9950483,
        "longitude": -108.0982527,
        "altitude": 717.1888427734375,
        "accuracy": 15.486000061035156,
        "altitudeAccuracy": null,
        "heading": null,
        "speed": 0
      }
    }
  ],
  "geolocation": {
    "latitude": 22.9950483,
    "longitude": -108.0982527,
    "altitude": 717.1888427734375,
    "accuracy": 15.486000061035156,
    "altitudeAccuracy": null,
    "heading": null,
    "speed": 0
  },

I kinda expected the second form would have had it and not a permission error? However, subsequent forms had it, so that seems not a real problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: 3 - Low Can be bumped from the release Type: Technical issue Improve something that users won't notice
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants