Dbrant has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/377777 )

Change subject: Fix: Always ask for Write permission, not Read. (crash on Oreo)
......................................................................

Fix: Always ask for Write permission, not Read. (crash on Oreo)

When navigating to our LocalCompilations screen, we ask for permission to
READ external storage, so that it can scan the device for existing packs.

But the thing is, when the user wants to download a new pack, we actually
need the WRITE permission, so that the pack can be written to storage.
Older versions of Android don't really make the distinction between the
READ and WRITE permissions, but Oreo is actually starting to enforce this
distinction, and crashes if you try to write to the storage without the
explicit WRITE permission.

So, rather than asking for the WRITE permission separately (after having
already asked for the READ permission), let's just ask for the WRITE
permission in the first place, which will implicitly grant the READ
permission anyway, so that the user only has to deal with one permission
popup dialog.

Bug: T175580
Change-Id: Idc22de0c6c6208c70c05e9b26f1c62522c4aa436
---
M app/src/main/java/org/wikipedia/Constants.java
M app/src/main/java/org/wikipedia/activity/BaseActivity.java
M app/src/main/java/org/wikipedia/util/PermissionUtil.java
3 files changed, 15 insertions(+), 27 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/apps/android/wikipedia 
refs/changes/77/377777/1

diff --git a/app/src/main/java/org/wikipedia/Constants.java 
b/app/src/main/java/org/wikipedia/Constants.java
index 8428652..6e57055 100644
--- a/app/src/main/java/org/wikipedia/Constants.java
+++ b/app/src/main/java/org/wikipedia/Constants.java
@@ -14,7 +14,6 @@
             + "profile=\"https://www.mediawiki.org/wiki/Specs/";;
     public static final String ACCEPT_HEADER_SUMMARY = ACCEPT_HEADER_PREFIX + 
"Summary/1.1.2\"";
 
-    public static final int 
ACTIVITY_REQUEST_READ_EXTERNAL_STORAGE_PERMISSION_OFFLINE = 43;
     public static final int ACTIVITY_REQUEST_WRITE_EXTERNAL_STORAGE_PERMISSION 
= 44;
     public static final int ACTIVITY_REQUEST_VOICE_SEARCH = 45;
     public static final int ACTIVITY_REQUEST_LANGLINKS = 50;
diff --git a/app/src/main/java/org/wikipedia/activity/BaseActivity.java 
b/app/src/main/java/org/wikipedia/activity/BaseActivity.java
index ec0f070..d0bf688 100644
--- a/app/src/main/java/org/wikipedia/activity/BaseActivity.java
+++ b/app/src/main/java/org/wikipedia/activity/BaseActivity.java
@@ -73,14 +73,14 @@
     public void onRequestPermissionsResult(int requestCode, @NonNull String[] 
permissions,
                                            @NonNull int[] grantResults) {
         switch (requestCode) {
-            case 
Constants.ACTIVITY_REQUEST_READ_EXTERNAL_STORAGE_PERMISSION_OFFLINE:
+            case Constants.ACTIVITY_REQUEST_WRITE_EXTERNAL_STORAGE_PERMISSION:
                 if (PermissionUtil.isPermitted(grantResults)) {
                     searchOfflineCompilations(true);
                 } else {
-                    L.i("Read permission was denied by user");
+                    L.i("Write permission was denied by user");
                     onOfflineCompilationsError(new 
RuntimeException(getString(R.string.offline_read_permission_error)));
-                    if 
(PermissionUtil.shouldShowReadPermissionRationale(this)) {
-                        showReadPermissionSnackbar();
+                    if 
(PermissionUtil.shouldShowWritePermissionRationale(this)) {
+                        showStoragePermissionSnackbar();
                     }
                 }
                 break;
@@ -131,9 +131,9 @@
             // TODO: enable when ready for production.
             return;
         }
-        if (!PermissionUtil.hasReadExternalStoragePermission(this)) {
-            if (PermissionUtil.shouldShowReadPermissionRationale(this)) {
-                requestReadPermission();
+        if (!PermissionUtil.hasWriteExternalStoragePermission(this)) {
+            if (PermissionUtil.shouldShowWritePermissionRationale(this)) {
+                requestStoragePermission();
             } else {
                 onOfflineCompilationsError(new 
RuntimeException(getString(R.string.offline_read_permission_error)));
             }
@@ -163,18 +163,18 @@
         }
     }
 
-    private void requestReadPermission() {
-        PermissionUtil.requestReadStorageRuntimePermissions(BaseActivity.this,
-                
Constants.ACTIVITY_REQUEST_READ_EXTERNAL_STORAGE_PERMISSION_OFFLINE);
+    private void requestStoragePermission() {
+        PermissionUtil.requestWriteStorageRuntimePermissions(BaseActivity.this,
+                Constants.ACTIVITY_REQUEST_WRITE_EXTERNAL_STORAGE_PERMISSION);
     }
 
-    private void showReadPermissionSnackbar() {
+    private void showStoragePermissionSnackbar() {
         Snackbar snackbar = FeedbackUtil.makeSnackbar(this,
                 getString(R.string.offline_read_permission_rationale), 
FeedbackUtil.LENGTH_DEFAULT);
         snackbar.setAction(R.string.page_error_retry, new 
View.OnClickListener() {
             @Override
             public void onClick(View v) {
-                requestReadPermission();
+                requestStoragePermission();
             }
         });
         snackbar.show();
diff --git a/app/src/main/java/org/wikipedia/util/PermissionUtil.java 
b/app/src/main/java/org/wikipedia/util/PermissionUtil.java
index 986f329..5664af1 100644
--- a/app/src/main/java/org/wikipedia/util/PermissionUtil.java
+++ b/app/src/main/java/org/wikipedia/util/PermissionUtil.java
@@ -24,21 +24,10 @@
                 && grantResults[0] == PackageManager.PERMISSION_GRANTED;
     }
 
-    public static boolean hasReadExternalStoragePermission(@NonNull Context 
context) {
-        return ContextCompat.checkSelfPermission(context,
-                Manifest.permission.READ_EXTERNAL_STORAGE) == 
PackageManager.PERMISSION_GRANTED;
-    }
-
-    public static void requestReadStorageRuntimePermissions(AppCompatActivity 
activity, int requestCode) {
-        
Prefs.setAskedForPermissionOnce(Manifest.permission.READ_EXTERNAL_STORAGE);
-        ActivityCompat.requestPermissions(activity, new 
String[]{Manifest.permission.READ_EXTERNAL_STORAGE}, requestCode);
-        // once permission is granted/denied it will continue with 
onRequestPermissionsResult
-    }
-
     @TargetApi(Build.VERSION_CODES.M)
-    public static boolean shouldShowReadPermissionRationale(@NonNull 
AppCompatActivity activity) {
-        return 
!Prefs.askedForPermissionOnce(Manifest.permission.READ_EXTERNAL_STORAGE)
-                || 
activity.shouldShowRequestPermissionRationale(Manifest.permission.READ_EXTERNAL_STORAGE);
+    public static boolean shouldShowWritePermissionRationale(@NonNull 
AppCompatActivity activity) {
+        return 
!Prefs.askedForPermissionOnce(Manifest.permission.WRITE_EXTERNAL_STORAGE)
+                || 
activity.shouldShowRequestPermissionRationale(Manifest.permission.WRITE_EXTERNAL_STORAGE);
     }
 
     public static boolean hasWriteExternalStoragePermission(@NonNull Context 
context) {

-- 
To view, visit https://gerrit.wikimedia.org/r/377777
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idc22de0c6c6208c70c05e9b26f1c62522c4aa436
Gerrit-PatchSet: 1
Gerrit-Project: apps/android/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Dbrant <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to