Merge pull request #14 from misaochan/gallery-fix-M

Fix gallery crash for Android M

app/src/main/java/fr/free/nrw/commons/contributions/ContributionController.java

@@ -85,7 +85,6 @@ public class ContributionController {
         try {
             activity.startActivity(shareIntent);
         } catch (SecurityException e) {
-            //FIXME: Add permission request here. Only startActivity if permission has been granted.
             Log.e("ContributionController", "Security Exception", e);
         }
     }

app/src/main/java/fr/free/nrw/commons/contributions/ContributionsListFragment.java

@@ -1,11 +1,16 @@
 package fr.free.nrw.commons.contributions;
 
+import android.Manifest;
 import android.app.Activity;
 import android.content.ActivityNotFoundException;
 import android.content.Context;
 import android.content.Intent;
 import android.content.SharedPreferences;
+import android.content.pm.PackageManager;
+import android.os.Build;
 import android.os.Bundle;
+import android.support.v4.app.ActivityCompat;
+import android.support.v4.content.ContextCompat;
 import android.util.Log;
 import android.view.LayoutInflater;
 import android.view.View;
@@ -94,8 +99,20 @@ public class ContributionsListFragment extends Fragment {
     public boolean onOptionsItemSelected(MenuItem item) {
         switch(item.getItemId()) {
             case R.id.menu_from_gallery:
-                controller.startGalleryPick();
+                //Gallery crashes before reach ShareActivity screen so must implement permissions check here
-                return true;
+                if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
+                    if (ContextCompat.checkSelfPermission(this.getActivity(), Manifest.permission.READ_EXTERNAL_STORAGE) != PackageManager.PERMISSION_GRANTED) {
+                        ActivityCompat.requestPermissions(this.getActivity(), new String[]{Manifest.permission.READ_EXTERNAL_STORAGE}, 1);
+                        return true;
+                    } else {
+                        controller.startGalleryPick();
+                        return true;
+                    }
+                }
+                else {
+                    controller.startGalleryPick();
+                    return true;
+                }
             case R.id.menu_from_camera:
                 controller.startCameraCapture();
                 return true;
@@ -129,6 +146,23 @@ public class ContributionsListFragment extends Fragment {
         }
     }
 
+    @Override
+    public void onRequestPermissionsResult(int requestCode,
+                                           String permissions[], int[] grantResults) {
+        switch (requestCode) {
+            // 1 = Storage allowed when gallery selected
+            case 1: {
+                if (grantResults.length > 0
+                        && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
+                    controller.startGalleryPick();
+                } else {
+                    return;
+                }
+                return;
+            }
+        }
+    }
+
     @Override
     public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) {
         menu.clear(); // See http://stackoverflow.com/a/8495697/17865

app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java

@@ -81,19 +81,45 @@ public  class       ShareActivity
         super(WikiAccountAuthenticator.COMMONS_ACCOUNT_TYPE);
     }
 
+    /**
+     * Called when user taps the submit button
+     */
     public void uploadActionInitiated(String title, String description) {
 
         this.title = title;
         this.description = description;
 
-        //Check for Storage permission that is required for upload. Do not allow user to proceed without permission, otherwise will crash
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
-        if (ContextCompat.checkSelfPermission(this, Manifest.permission.READ_EXTERNAL_STORAGE) != PackageManager.PERMISSION_GRANTED) {
+            //Check for Storage permission that is required for upload. Do not allow user to proceed without permission, otherwise will crash
-            ActivityCompat.requestPermissions(this, new String[]{Manifest.permission.READ_EXTERNAL_STORAGE}, 1);
+            if (ContextCompat.checkSelfPermission(this, Manifest.permission.READ_EXTERNAL_STORAGE) != PackageManager.PERMISSION_GRANTED) {
+                ActivityCompat.requestPermissions(this, new String[]{Manifest.permission.READ_EXTERNAL_STORAGE}, 4);
+            } else {
+                uploadBegins();
+            }
         } else {
             uploadBegins();
         }
     }
 
+    private void uploadBegins() {
+
+        Toast startingToast = Toast.makeText(getApplicationContext(), R.string.uploading_started, Toast.LENGTH_LONG);
+        startingToast.show();
+
+        if (cacheFound == false) {
+            //Has to be called after apiCall.request()
+            app.cacheData.cacheCategory();
+            Log.d(TAG, "Cache the categories found");
+        }
+
+        uploadController.startUpload(title, mediaUri, description, mimeType, source, new UploadController.ContributionUploadProgress() {
+            public void onUploadStarted(Contribution contribution) {
+                ShareActivity.this.contribution = contribution;
+                showPostUpload();
+            }
+        });
+    }
+
     private void showPostUpload() {
         if(categorizationFragment == null) {
             categorizationFragment = new CategorizationFragment();
@@ -270,37 +296,14 @@ public  class       ShareActivity
         }
     }
 
-    private void uploadBegins() {
-
-        Toast startingToast = Toast.makeText(getApplicationContext(), R.string.uploading_started, Toast.LENGTH_LONG);
-        startingToast.show();
-
-        if (cacheFound == false) {
-            //Has to be called after apiCall.request()
-            app.cacheData.cacheCategory();
-            Log.d(TAG, "Cache the categories found");
-        }
-
-        uploadController.startUpload(title, mediaUri, description, mimeType, source, new UploadController.ContributionUploadProgress() {
-            public void onUploadStarted(Contribution contribution) {
-                ShareActivity.this.contribution = contribution;
-                showPostUpload();
-            }
-        });
-    }
-
     @Override
     public void onRequestPermissionsResult(int requestCode,
                                            String permissions[], int[] grantResults) {
         switch (requestCode) {
-            // 1 = Storage
+            // 1 = Storage (from snackbar)
             case 1: {
                 if (grantResults.length > 0
                         && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
-
-                    //Uploading only begins if storage permission granted
-                    uploadBegins();
-                    snackbar.dismiss();
                     getFileMetadata();
                 }
                 return;
@@ -323,6 +326,22 @@ public  class       ShareActivity
                         && grantResults[1] == PackageManager.PERMISSION_GRANTED) {
                     getLocationData();
                 }
+                return;
+            }
+            // 4 = Storage (from submit button) - this needs to be separate from (1) because only the
+            // submit button should bring user to next screen
+            case 4: {
+                if (grantResults.length > 0
+                        && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
+                    //It is OK to call this at both (1) and (4) because if perm had been granted at
+                    //snackbar, user should not be prompted at submit button
+                    getFileMetadata();
+
+                    //Uploading only begins if storage permission granted from arrow icon
+                    uploadBegins();
+                    snackbar.dismiss();
+                }
+                return;
             }
         }
     }

app/src/main/res/values/strings.xml

@@ -150,8 +150,8 @@
     <string name="provider_campaigns">Campaigns</string>
     <string name="menu_refresh">Refresh</string>
 
-    <string name="storage_permission_rationale">Required: Read external storage. App cannot function without this.</string>
+    <string name="storage_permission_rationale">Required permission: Read external storage. App cannot function without this.</string>
-    <string name="location_permission_rationale">Optional: Current location for category suggestions</string>
+    <string name="location_permission_rationale">Optional permission: Get current location for category suggestions</string>
     <string name="ok">OK</string>
     <string name="back">Back</string>
 </resources>