D5876: Android 6 Runtime Permissions

[Albert Vaca Cintora]

albertvaka added a comment.


  I've just found time to compile the app with your patch and it looks great! Sorry for the delay. We will need to add a custom explanation for every plugin, but we can do that easily by overriding the getPermissionExplanationDialog(). Nicely done!
  
  I've seen that you added a function getOptionalPermissions() that you are not using, though. Is this work in progress and you plan to use it as part of this same code review? It would be nice to have a second list of "plugins that can load, but could do more stuff if you gave extra permissions". However, we can leave this out for a second iteration and merge the current patch, because it's already functional. Feel free to tell me if you want to do more changes to this patch or if it's ready to merge :)

INLINE COMMENTS

> SharePlugin.java:211
> +
> +                            // Nougat requires share:// URIs instead of file:// URIs
> +                            // TODO use FileProvider for >Nougat

Good catch :)

> TelephonyPlugin.java:291
> +    public String[] getRequiredPermissions() {
> +        return new String[]{Manifest.permission.READ_PHONE_STATE, Manifest.permission.READ_CONTACTS, Manifest.permission.SEND_SMS};
> +    }

This plugin only needs to RECEIVE_SMS, but not send them.

REPOSITORY
  R225 KDE Connect - Android application

REVISION DETAIL
  https://phabricator.kde.org/D5876

To: nicolasfella, #kde_connect
Cc: albertvaka, aboudhar, seebauer, progwolff, MayeulC, menasshock, ach, apol, hkaelberer
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20170530/e96d89ff/attachment.html>