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

Change method to prompt for ignoring background optimization #980

Closed
shankari opened this issue Sep 18, 2023 · 14 comments
Closed

Change method to prompt for ignoring background optimization #980

shankari opened this issue Sep 18, 2023 · 14 comments

Comments

@shankari
Copy link
Contributor

shankari commented Sep 18, 2023

@louisg1337 During the onboarding process, we currently ask participants to give us a bunch of permissions.
On android, this includes the permission to remove background optimizations using
#934 (comment)

The user turns off battery optimizations for your app. You can help users find this option by sending them to your app's App info page in system settings. To do so, invoke an intent that contains the ACTION_IGNORE_BATTERY_OPTIMIZATION_SETTINGS intent action.

However, this is a bad user experience since ACTION_IGNORE_BATTERY_OPTIMIZATION_SETTINGS essentially launches a new screen with the currently unoptimized apps. The user has to select "optimized" and then search for the app name and then select "turn off optimization".

@JGreenlee found that other apps use a popup
#934 (comment)
by replacing the call with one to ACTION_REQUEST_IGNORE_BATTERY_OPTIMIZATIONS instead

I think if we request for ACTION_REQUEST_IGNORE_BATTERY_OPTIMIZATIONS, we'd get the popup:
https://developer.android.com/reference/android/provider/Settings#ACTION_REQUEST_IGNORE_BATTERY_OPTIMIZATIONS

However, this requires an additional permission
#934 (comment)

but there is some evidence that this may increase the chance of the app being rejected
#934 (comment)

We want to update the checkAndPromptIgnoreBatteryOptimizations in src/android/verification/SensorControlForegroundDelegate.java in the e-mission-data-collection plugin
https://github.com/e-mission/e-mission-data-collection to use ACTION_REQUEST_IGNORE_BATTERY_OPTIMIZATIONS instead of ACTION_IGNORE_BATTERY_OPTIMIZATION_SETTINGS

Note that https://github.com/e-mission/e-mission-data-collection is a cordova plugin, so after you make the change you will need to remove + re-add it (cordova plugin rm and cordova plugin add) so that you can test it with the UI.

@shankari
Copy link
Contributor Author

shankari commented Sep 22, 2023

In the android project, the permission needs to be added to AndroidManifest.xml
Note that for cordova projects, the android proejct is autogenerated when you build and is in platforms/android
you can look up how to "open that" and modify the AndroidManifest.xml

The changes to AndroidManifest or in the java files under platforms/android are not guaranteed to be persistent.
They are typically persistent as long as you are making changes over a short period of time but add/remove a plugin and they are all gone.

So after you have made the changes under platforms/android, you need to copy them to the e-mission-data-collection repo and then remove and add the plugin to test, and then bump up the plugin version in package.cordovabuild.json once the plugin is merged and a new plugin release has been created by me.

In the plugin, the permission changes will go into plugin.xml - see existing examples for location, motion activity etc in there.

think of this as a library

@shankari
Copy link
Contributor Author

  • develop in e-mission-phone by editing code in platforms/android directly
  • get it to work
  • copy changes to local copy of e-mission-data-collection
  • cordova plugin rm and cordova plugin add <local copy>
  • submit PR to e-mission-data-collection
  • I merge and create new release version
  • change plugin version on e-mission-phone
  • submit new PR
  • I merge, change is done

@louisg1337
Copy link

Implemented the new ACTION_REQUEST_IGNORE_BATTERY_OPTIMIZATIONS button and it now prompts the user to "Let app always run in the background."

There is a bug that arises, however, where when you click "Fix" and get the prompt for allowing the aforementioned, it won't let you "fix" any other permissions that bring you to the settings. It appears that the battery optimization prompts the user on the app, but also creates a pop up in the settings which is irresponsive that you can't click on.

Screenshot_1695585613

If you close that settings tab then you're allowed to "fix" other permissions as it lets you actually go to the settings now. It's definitely a weird bug that I'll have to look deeper into, just wanted to get it out for now.

@shankari
Copy link
Contributor Author

@louisg1337 this is the popup we want to get - see #934 (comment)

Can you clarify what you mean by " but also creates a pop up in the settings which is irresponsive that you can't click on.". Maybe a video that will show us the unreponsiveness.

@JGreenlee since he has seen this popup on other apps with background operation and has a sense of what is supposed to happen.

@JGreenlee
Copy link

Looks like great progress! It's the right popup but it's apparently launching over a Settings activity when it should just launch within our app activity.

I would double check the intent and make sure it is not still trying to launch the "battery optimizations" page.

@JGreenlee
Copy link

Apparently startActivityForResult is deprecated, so it's hard to find documentation as to whether you can just not give it a second argument.
But I'd try that first, and potentially consider whatever the recommended replacement for startActivityForResult is

@shankari
Copy link
Contributor Author

shankari commented Sep 25, 2023

It would also be helpful to show us your changes - maybe start a draft PR in e-mission-data-collection by copying over your changes, or at least attach a patch here. We do use startActivityForResult extensively in the existing permissions code, so I would suggest sticking with it for now. It should (and does) work for the other permissions.

@louisg1337
Copy link

Thank you guys for getting back to me with the feedback! Firstly, I created a draft pr here. I also added in a video below which should show the bug better I hope. In terms of your guys' suggestions, I used startActivity instead of startActivityForResult, but I can definitely play around with it some more and see if I can find out how to use it in this case. I'll let you guys know if I find anything different.

bug.webm

@shankari
Copy link
Contributor Author

@louisg1337 per our conversation today:

  • try to use the permission checker class that I have already written
  • note that older versions of android also use popups - which is what the permission checker was written for, so you should be able to use an android 10 emulator (BuildVersion.Q), upgrade the webview version using the google play store and compare existing popups versus your popup
  • in parallel, get an android 7 test phone, since the android 7 emulator does not have google play store included, so the webview cannot be updated, so the app will generate a WSOD

@louisg1337
Copy link

Slight update as I want to get my findings out before I go for the night. I tried using permission checker but I couldn't seem to get it to work with ACTION_REQUEST_IGNORE_BATTERY_OPTIMIZATIONS. I included the code commented out in my latest commit to my draft PR in case anyone knows how to fix it. I was really confused about the permissionStatusConstant, which is the first parameter of getPermissionChecker so maybe thats it.

If the permission checker method is not imperative, I did seem to fix the bug in the original code. If you use startActivityForResult instead of startActivity and pass in the original SensorControlConstants.OPEN_BATTERY_OPTIMIZATION_PAGE it appears to work correctly now without the weird not being able to change any other settings bug. I've tested it with android emulators api34 and 30 (>= R), and the popup seems to work fine, I just need to test >= Q to compare the popups as mentioned above to make sure its all good.

@shankari
Copy link
Contributor Author

shankari commented Sep 26, 2023

@louisg1337 I think it would be better to use the permission checker if possible since (i) it is a single location for popups, and (ii) it also deals with complex logic around people hitting "deny", instead of "allow", which you may not have tested. For example, you always call success, once you have launched the popup regardless of what the user actually picks. This is probably why (as we can see from the video) the category turns green even before the user selects anything.

I checked the PR and you have permission checker code largely working correctly. The status constant is correct (I assume you will put the hardcoded value into SensorControlConstants eventually). I think that the part you are missing is handling the callback (public void onActivityResult) correctly.

Note that this process is asynchronous - android generates a popup, waits for the user to respond, and then calls back with the status constant and that is when we need to return success or error to cordova.

@louisg1337
Copy link

louisg1337 commented Sep 27, 2023

09-27 14:34:24.305 524 524 W ActivityManager: Unable to start service Intent {act=android.service.contentcapture.ContentCaptureServicecmp=com.google.android.as/com.google.android.apps.miphone.aiai.app.AiAiContentCaptureService } U=0: not found

09-27 14:34:24.305 524 524 W RemoteContentCaptureService: could not bind to Intent {act=android.service.contentcapture.ContentCaptureServicecmp=com.google.android.as/com.google.android.apps.miphone.aiai.app.AiAiContentCaptureService } using flags 67112961

@louisg1337
Copy link

I managed to get the code working now using PermissionsPopupChecker by passing in a custom function that opens an intent specifically for this request. I attached a video below of it working as intended.

I pushed my changes to the draft PR. The commit does look confusing as it makes it seem like I rewrote the whole file, but it was just me trying to re-style the file back to its original state as I put in a bunch of spaces and comments to help me navigate the file. The main changes I made were in requestPermission, checkAndPromptIgnoreBatteryOptimizations and openRequestBatteryOptimization.

I do still have to test it on <= Q devices, although whenever I try to get onto the Google Play Store on a Q emulator to update Webview it won't let me and says authentication failed. Now whenever I try to load into it it refuses to take my phone number as a verification because it "has been used too many times."

Screen.Recording.2023-09-27.at.10.55.34.PM.mov

@shankari
Copy link
Contributor Author

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

3 participants