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

⬆️ More upgrade fun! Upgrade android to API 29 #557

Closed
shankari opened this issue Aug 4, 2020 · 7 comments
Closed

⬆️ More upgrade fun! Upgrade android to API 29 #557

shankari opened this issue Aug 4, 2020 · 7 comments

Comments

@shankari
Copy link
Contributor

shankari commented Aug 4, 2020

Since yesterday, Aug 3rd 2020, all new apps published to the app store require to support a minimum of API 29.
Now I wish I had prioritized uploading the app on Friday instead of fixing the server issues first.

https://developer.android.com/distribute/play-policies

When you upload an APK, it needs to meet Google Play’s target API level requirements. By August 3, 2020, new apps must target at least Android 10 (API level 29). By November 2, 2020, all apps that are being updated must target at least Android 10 (API level 29). Until then, new apps and app updates must target at least Android 9 (API level 28). Wear OS apps are not subject to the API level 29 requirement.

@shankari
Copy link
Contributor Author

shankari commented Aug 4, 2020

Here's a summary of the changes that apply to us:

  • Restricted access to location while the app is in the background, requiring ACCESS_BACKGROUND_LOCATION permission. We have a foreground service, so this is theoretically not required, but we may want to add it anyway.
  • Restricted access to physical activity information such as the user's step count, requiring ACTIVITY_RECOGNITION permission.

If we do want to use the Foreground Service exemption, we must expand it. For now, let's have both the foreground service and the background location permission. We can remove the foreground service in a later version once we have had more time for testing.

Unlike the ACCESS_FINE_LOCATION and ACCESS_COARSE_LOCATION permissions, the ACCESS_BACKGROUND_LOCATION permission only affects an app's access to location when it runs in the background. An app is considered to be accessing location in the background unless one of the following conditions is satisfied:
An activity belonging to the app is visible.
The app is running a foreground service that has declared a foreground service type of location.

Another restrictions is on launching an activity in the background

Android 10 (API level 29) and higher place restrictions on when apps can start activities when the app is running in the background. These restrictions help minimize interruptions for the user and keep the user more in control of what's shown on their screen.

Let's tackle each of these separately.

@shankari
Copy link
Contributor Author

shankari commented Aug 5, 2020

The first change was the most challenging. We don't only request the permission; we also periodically check if we have it, and we generate notifications if we don't. Trying to duplicate that code, only with BACKGROUND_LOC permission in addition to LOCATION.

Second change was a one-liner to plugin.xml

Third change may be the trickiest. Searching through the code indicates that startActivity is used in the following plugins:

  • local notification
  • cordova web view
  • push notification
  • IAB (but mainly to handle specialized URL such as tel)

So to finish testing this, we should try local and push notifications. the push notification will also test the IAB, so we will be done

@shankari
Copy link
Contributor Author

shankari commented Aug 5, 2020

Alas, the BACKGROUND_LOC versus LOCATION didn't work. If you ask for the permissions separately, then the user only has a choice between "when in use" and "deny". And then without the background permission, the Service went into an infinite loop caused by tracking_error in the start state calling setNewState(), which in turn checks the permission again...

Adding a parameter that controls whether or not to recheck the permissions fixed that while still allowing the foreground service to close properly.

Tested local notifications and remote notifications and they both work.
Uploading this apk right now and will deal with commits tomorrow afternoon.

@shankari
Copy link
Contributor Author

shankari commented Aug 5, 2020

Not so soon. It turns out we are still checking for the background permission on android 7 as well. Since the permission was only introduced in API 29, this check fails continuously and we can't track anything.

Remote notifications work, but the UUID is not getting filled in, need to investigate potential regression on the urap branch.
And we need to update the message for android 10 to indicate that the users need to choose "Always".

@shankari
Copy link
Contributor Author

shankari commented Aug 6, 2020

ok, so the UUID not getting filled in is because of the changes to support Google Forms.
And we will be using Office Forms so doesn't make much sense to revert back. Might actually be better to refactor for greater generality, although I don't think this is a blocker.

wrt greater generality:

  • qualtrics has a unique ID
  • looks like google forms has a unique class
  • office forms has a unique attribute named aria-labelledby - e.g.
<input class="office-form-question-textbox office-form-textfield-input form-control border-no-radius" aria-describedby="question1-subtitle" aria-labelledby="question1-title question1-required question1-questiontype" placeholder="Enter your answer" spellcheck="true" maxlength="4000" value="">

Note that Firefox supports getting the xpath of any element from the inspector (Right-click -> Copy -> xpath)
/html/body/div/div/div/div/div[1]/div/div[1]/div[2]/div[2]/div[1]/div[2]/div[3]/div/div/input which is messy but will also work.

The document object has the following methods:

  • getElementsByClassName
  • getElementsByTagName
  • getElementsByTagNameNS
  • getElementById

So we can't easily retrieve elements by other attributes. Should probably support id and xpath only.

  • Chrome also supports copying the xpath //*[@id="form-container"]/div/div/div[1]/div/div[1]/div[2]/div[2]/div[2]/div[2]/div[3]/div/div/div/input, or /html/body/div/div/div/div/div[1]/div/div[1]/div[2]/div[2]/div[2]/div[2]/div[3]/div/div/div/input
  • Safari also supports copying the xpath //*[@id="form-container"]/div/div/div[1]/div/div[1]/div[2]/div[2]/div[1]/div[2]/div[3]/div/div/div/input. Don't see a full xpath though.

We should only support full xpath for consistency. We can find elements that match an xpath by using Document.evaluate https://developer.mozilla.org/en-US/docs/Web/API/Document/evaluate

Seems like a nice addition even if it is not critical

shankari added a commit to shankari/e-mission-data-collection that referenced this issue Aug 6, 2020
Changes documented here:
e-mission/e-mission-docs#557

- Add the `BACKGROUND_LOCATION` permission
- Fix the motion activity permission

For the background location permissions, make sure we prompt for it correctly
(i.e. prompting along with location permission in general), check for it
correctly, and only use it for Q+.

The other changes were fairly straightforward
@shankari
Copy link
Contributor Author

shankari commented Aug 6, 2020

Testing done:

  • On both pre-Q and post-Q:
    • follow the instructions correctly: WORKS
    • turn off location after installing and hit force sync: generates prompt
    • select prompt: WORKS
  • On post-Q:
    • turn on "when in use": generates prompt to turn on background tracking

Everything seems to work! I'm not seeing any activity recognition events in the emulator though. Not sure if that is related to the emulator or activity recognition enablement in general. Will have to test on real phones to find out.

@shankari
Copy link
Contributor Author

shankari commented Aug 6, 2020

Closing this issue now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant