Fixed for Android 8.0 #180
Conversation
Android 8.0 no longer assumes related permissions - so when the app gets a contact 'write' permission to save the contact, it no longer automatically gets the 'read' permission - which was causing the app to crash when it tries to read the contact it just saved. This modification simply ensures the app has both permissions before attempting to save or remove.
|
Uh @duncan-c, for us to be able to accept this PR you would stop adding unrelated fixes to this branch of your plugin - otherwise it will be really hard to merge these. Easiest way to achieve this is probably to go back to your "Fixed for Android 8.0" commit and create a branch from that, then create a new PR from that branch. Then you can go on and create new branches for your other commits and also create PRs from those (and mention that they build on the first one). Then we can merge those fixes into the plugin as well! |
|
Ah, apologies. I forgot this PR was open. I'll undo these two commits and create a new branch as you suggest. |
|
Cheers! |
|
@duncan-c Thanks for the work on this! Where do we stand on getting this merged? Looks like there are some failing CI checks. |
|
What Android Cordova Version and API version are you on @duncan-c? Did you try with 28? |
|
Sorry - Again I forgot about this open PR (and I probably should have made a new branch for both of these issues - its just that my project is pointing to master). |
I've now removed this commit to its own branch. |
Thanks. The best way to to do this is also move the commit for this PR into its own branch, then edit this PR to point at your new branch, like what Janpio said above. This will help protect this PR from unwanted changes in the future.
|
|
I'll try to see what is going on with Travis later tonight. The failures appears to be caused by using a mismatch of of cordova-android and the android sdk. |
So I didn't do this at the time as AFAIK you can't change the source branch for a pull request (correct me if I'm wrong) - so at the time I just moved my new commits to new branches (déjà vu). Perhaps I should close this PR and make a new one on it's own branch so this doesn't keep happening? |
Oops! Looks like you are right. Looks like you can only change the target branch, but not the source branch.
If you think that there is a good chance of you adding new work in your master branch, then I think it is worth closing this PR and recreating it from a different source branch. |
Ok I will do as I seem to keep making this mistake, and it would be good for all my fixes to be on the master branch so my project can easily use them. Sorry for the mess!! I'll also add PRs for the other fixes I made. |
Platforms affected
Android 8+
What does this PR do?
Android 8.0 no longer assumes related permissions - so when the app gets a contact 'write' permission to save the contact, it no longer automatically gets the 'read' permission - which was causing the app to crash when it tries to read the contact it just saved. This modification simply ensures the app has both permissions before attempting to save or remove.
What testing has been done on this change?
Only tested on Android 8 and 9, but no reason for it to not be backwards compatible.