From 7955cfe71b85f457b10d454313531424b441e679 Mon Sep 17 00:00:00 2001 From: Adam Jones Date: Wed, 8 Jun 2016 00:30:13 +0100 Subject: [PATCH 1/5] Fix storage permission Part of the way there to fixing #117, however location and contacts still need to be fixed. --- commons/app/build.gradle | 10 +-- .../nrw/commons/upload/ShareActivity.java | 72 +++++++++++++------ commons/app/src/main/res/values/strings.xml | 3 + commons/build.gradle | 2 +- 4 files changed, 61 insertions(+), 26 deletions(-) diff --git a/commons/app/build.gradle b/commons/app/build.gradle index 8bf7f2583..5a6428a72 100644 --- a/commons/app/build.gradle +++ b/commons/app/build.gradle @@ -1,8 +1,7 @@ apply plugin: 'com.android.application' dependencies { - compile fileTree(dir: 'libs', include: '*.jar') - compile 'com.google.code.gson:gson:1.4' + compile fileTree(include: '*.jar', dir: 'libs') compile 'fr.avianey.com.viewpagerindicator:library:2.4.1.1@aar' compile 'in.yuvi:http.fluent:1.3' compile 'com.android.volley:volley:1.0.0' @@ -10,14 +9,17 @@ dependencies { compile 'ch.acra:acra:4.5.0' compile 'org.mediawiki:api:1.3' compile 'de.keyboardsurfer.android.widget:crouton:1.8.5@aar' - compile group: 'commons-codec', name: 'commons-codec', version: '1.10' + compile 'commons-codec:commons-codec:1.10' compile 'com.android.support:support-v4:23.4.0' compile 'com.android.support:appcompat-v7:23.4.0' + compile 'com.android.support:design:23.4.0' + //noinspectio GradleDependency - old version has required feature + compile 'com.google.code.gson:gson:1.4' } android { compileSdkVersion 23 - buildToolsVersion "23.0.2" + buildToolsVersion "23.0.3" useLibrary 'org.apache.http.legacy' diff --git a/commons/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java b/commons/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java index d01f4d194..c92c90f6c 100644 --- a/commons/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java +++ b/commons/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java @@ -1,13 +1,19 @@ package fr.free.nrw.commons.upload; +import android.Manifest; import android.content.ContentResolver; import android.content.Intent; +import android.content.pm.PackageManager; import android.net.Uri; import android.os.Bundle; import android.os.Environment; +import android.support.v4.app.ActivityCompat; import android.support.v4.app.NavUtils; +import android.support.design.widget.Snackbar; +import android.support.v4.content.ContextCompat; import android.util.Log; import android.view.MenuItem; +import android.view.View; import android.widget.ImageView; import android.widget.Toast; @@ -19,9 +25,9 @@ import java.util.List; import fr.free.nrw.commons.CommonsApplication; import fr.free.nrw.commons.EventLog; import fr.free.nrw.commons.R; -import fr.free.nrw.commons.auth.*; +import fr.free.nrw.commons.auth.AuthenticatedActivity; +import fr.free.nrw.commons.auth.WikiAccountAuthenticator; import fr.free.nrw.commons.category.CategorizationFragment; -import fr.free.nrw.commons.contributions.*; import fr.free.nrw.commons.modifications.CategoryModifier; import fr.free.nrw.commons.modifications.ModificationsContentProvider; import fr.free.nrw.commons.modifications.ModifierSequence; @@ -33,12 +39,12 @@ import fr.free.nrw.commons.modifications.TemplateRemoveModifier; */ public class ShareActivity extends AuthenticatedActivity - implements SingleUploadFragment.OnUploadActionInitiated, + implements fr.free.nrw.commons.upload.SingleUploadFragment.OnUploadActionInitiated, CategorizationFragment.OnCategoriesSaveHandler { private static final String TAG = ShareActivity.class.getName(); - private SingleUploadFragment shareView; + private fr.free.nrw.commons.upload.SingleUploadFragment shareView; private CategorizationFragment categorizationFragment; private CommonsApplication app; @@ -48,14 +54,14 @@ public class ShareActivity private String mediaUriString; private Uri mediaUri; - private Contribution contribution; + private fr.free.nrw.commons.contributions.Contribution contribution; private ImageView backgroundImageView; - private UploadController uploadController; + private fr.free.nrw.commons.upload.UploadController uploadController; private CommonsApplication cacheObj; private boolean cacheFound; - private GPSExtractor imageObj; + private fr.free.nrw.commons.upload.GPSExtractor imageObj; public ShareActivity() { super(WikiAccountAuthenticator.COMMONS_ACCOUNT_TYPE); @@ -71,8 +77,8 @@ public class ShareActivity Log.d(TAG, "Cache the categories found"); } - uploadController.startUpload(title, mediaUri, description, mimeType, source, new UploadController.ContributionUploadProgress() { - public void onUploadStarted(Contribution contribution) { + uploadController.startUpload(title, mediaUri, description, mimeType, source, new fr.free.nrw.commons.upload.UploadController.ContributionUploadProgress() { + public void onUploadStarted(fr.free.nrw.commons.contributions.Contribution contribution) { ShareActivity.this.contribution = contribution; showPostUpload(); } @@ -134,7 +140,7 @@ public class ShareActivity } else { EventLog.schema(CommonsApplication.EVENT_UPLOAD_ATTEMPT) .param("username", app.getCurrentAccount().name) - .param("source", getIntent().getStringExtra(UploadService.EXTRA_SOURCE)) + .param("source", getIntent().getStringExtra(fr.free.nrw.commons.upload.UploadService.EXTRA_SOURCE)) .param("multiple", true) .param("result", "cancelled") .log(); @@ -147,10 +153,10 @@ public class ShareActivity app.getApi().setAuthCookie(authCookie); - shareView = (SingleUploadFragment) getSupportFragmentManager().findFragmentByTag("shareView"); + shareView = (fr.free.nrw.commons.upload.SingleUploadFragment) getSupportFragmentManager().findFragmentByTag("shareView"); categorizationFragment = (CategorizationFragment) getSupportFragmentManager().findFragmentByTag("categorization"); if(shareView == null && categorizationFragment == null) { - shareView = new SingleUploadFragment(); + shareView = new fr.free.nrw.commons.upload.SingleUploadFragment(); this.getSupportFragmentManager() .beginTransaction() .add(R.id.single_upload_fragment_container, shareView, "shareView") @@ -170,9 +176,9 @@ public class ShareActivity @Override public void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); - uploadController = new UploadController(this); + uploadController = new fr.free.nrw.commons.upload.UploadController(this); setContentView(R.layout.activity_share); - + app = (CommonsApplication)this.getApplicationContext(); backgroundImageView = (ImageView)findViewById(R.id.backgroundImage); @@ -180,10 +186,10 @@ public class ShareActivity if(intent.getAction().equals(Intent.ACTION_SEND)) { mediaUri = (Uri) intent.getParcelableExtra(Intent.EXTRA_STREAM); - if(intent.hasExtra(UploadService.EXTRA_SOURCE)) { - source = intent.getStringExtra(UploadService.EXTRA_SOURCE); + if(intent.hasExtra(fr.free.nrw.commons.upload.UploadService.EXTRA_SOURCE)) { + source = intent.getStringExtra(fr.free.nrw.commons.upload.UploadService.EXTRA_SOURCE); } else { - source = Contribution.SOURCE_EXTERNAL; + source = fr.free.nrw.commons.contributions.Contribution.SOURCE_EXTERNAL; } mimeType = intent.getType(); } @@ -210,12 +216,36 @@ public class ShareActivity Log.d(TAG, "Uri: " + mediaUriString); Log.d(TAG, "Ext storage dir: " + Environment.getExternalStorageDirectory()); + + + // Check permissions + if (ContextCompat.checkSelfPermission(this.getApplicationContext(), + Manifest.permission.READ_EXTERNAL_STORAGE) + != PackageManager.PERMISSION_GRANTED) { + + if (ActivityCompat.shouldShowRequestPermissionRationale(this, + Manifest.permission.READ_EXTERNAL_STORAGE)) { + + Log.i(TAG, + "Displaying camera permission rationale to provide additional context."); + Snackbar.make(this.findViewById(android.R.id.content), R.string.storage_permission_rationale, Snackbar.LENGTH_INDEFINITE).setAction(R.string.ok, new View.OnClickListener() { + @Override + public void onClick(View view) { + ActivityCompat.requestPermissions(ShareActivity.this, new String[]{Manifest.permission.READ_EXTERNAL_STORAGE}, 1); + } + }).show(); + + } else { + ActivityCompat.requestPermissions(this, new String[]{Manifest.permission.READ_EXTERNAL_STORAGE}, 1); + } + } + //convert image Uri to file path - String filePath = FileUtils.getPath(this, mediaUri); + String filePath = fr.free.nrw.commons.upload.FileUtils.getPath(this, mediaUri); Log.d(TAG, "Filepath: " + filePath); Log.d(TAG, "Calling GPSExtractor"); - imageObj = new GPSExtractor(filePath, this); + imageObj = new fr.free.nrw.commons.upload.GPSExtractor(filePath, this); imageObj.registerLocationManager(); if (filePath != null && !filePath.equals("")) { @@ -232,7 +262,7 @@ public class ShareActivity app.cacheData.setQtPoint(decLongitude, decLatitude); } - MwVolleyApi apiCall = new MwVolleyApi(this); + fr.free.nrw.commons.upload.MwVolleyApi apiCall = new fr.free.nrw.commons.upload.MwVolleyApi(this); List displayCatList = app.cacheData.findCategory(); boolean catListEmpty = displayCatList.isEmpty(); @@ -246,7 +276,7 @@ public class ShareActivity } else { cacheFound = true; Log.d(TAG, "Cache found, setting categoryList in MwVolleyApi to " + displayCatList.toString()); - MwVolleyApi.setGpsCat(displayCatList); + fr.free.nrw.commons.upload.MwVolleyApi.setGpsCat(displayCatList); } } } diff --git a/commons/app/src/main/res/values/strings.xml b/commons/app/src/main/res/values/strings.xml index 5b53614a7..f0bd2f760 100644 --- a/commons/app/src/main/res/values/strings.xml +++ b/commons/app/src/main/res/values/strings.xml @@ -148,4 +148,7 @@ Unknown license Campaigns Refresh + + Storage permission is needed to access photos + OK diff --git a/commons/build.gradle b/commons/build.gradle index 0a11ca96a..691c8e8d0 100644 --- a/commons/build.gradle +++ b/commons/build.gradle @@ -5,7 +5,7 @@ buildscript { mavenCentral() } dependencies { - classpath 'com.android.tools.build:gradle:2.1.0' + classpath 'com.android.tools.build:gradle:2.1.2' } } From 8bf9808cfc5b3f1c84525f44201e81376581f21b Mon Sep 17 00:00:00 2001 From: Adam Jones Date: Wed, 8 Jun 2016 23:03:17 +0100 Subject: [PATCH 2/5] Fix permission problems on API 23+ --- commons/app/build.gradle | 4 +- commons/app/src/main/AndroidManifest.xml | 4 - .../free/nrw/commons/auth/LoginActivity.java | 54 ++++-- .../nrw/commons/upload/ShareActivity.java | 171 ++++++++++-------- commons/app/src/main/res/values/strings.xml | 4 +- 5 files changed, 137 insertions(+), 100 deletions(-) diff --git a/commons/app/build.gradle b/commons/app/build.gradle index 5a6428a72..adf1229fe 100644 --- a/commons/app/build.gradle +++ b/commons/app/build.gradle @@ -8,12 +8,12 @@ dependencies { compile 'com.nostra13.universalimageloader:universal-image-loader:1.8.4' compile 'ch.acra:acra:4.5.0' compile 'org.mediawiki:api:1.3' - compile 'de.keyboardsurfer.android.widget:crouton:1.8.5@aar' compile 'commons-codec:commons-codec:1.10' compile 'com.android.support:support-v4:23.4.0' compile 'com.android.support:appcompat-v7:23.4.0' compile 'com.android.support:design:23.4.0' - //noinspectio GradleDependency - old version has required feature + + //noinspection GradleDependency - old version has required feature compile 'com.google.code.gson:gson:1.4' } diff --git a/commons/app/src/main/AndroidManifest.xml b/commons/app/src/main/AndroidManifest.xml index 50be5c2b1..ccf0998ed 100644 --- a/commons/app/src/main/AndroidManifest.xml +++ b/commons/app/src/main/AndroidManifest.xml @@ -8,10 +8,6 @@ android:targetSdkVersion="23" /> - - - - diff --git a/commons/app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java b/commons/app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java index 78f06012f..48ef8236e 100644 --- a/commons/app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java +++ b/commons/app/src/main/java/fr/free/nrw/commons/auth/LoginActivity.java @@ -1,26 +1,39 @@ package fr.free.nrw.commons.auth; +import android.accounts.Account; +import android.accounts.AccountAuthenticatorActivity; +import android.accounts.AccountAuthenticatorResponse; +import android.accounts.AccountManager; +import android.app.Activity; +import android.app.ProgressDialog; +import android.content.ContentResolver; +import android.content.Intent; +import android.net.Uri; +import android.os.AsyncTask; +import android.os.Bundle; +import android.support.design.widget.Snackbar; +import android.support.v4.app.NavUtils; +import android.text.Editable; +import android.text.TextWatcher; +import android.util.Log; +import android.view.KeyEvent; +import android.view.Menu; +import android.view.MenuItem; +import android.view.View; +import android.view.inputmethod.EditorInfo; +import android.widget.Button; +import android.widget.EditText; +import android.widget.TextView; +import android.widget.Toast; + import java.io.IOException; -import android.content.*; -import android.net.Uri; -import android.text.*; -import android.view.inputmethod.EditorInfo; -import de.keyboardsurfer.android.widget.crouton.*; -import android.os.*; -import android.accounts.*; -import android.app.*; -import android.util.*; -import android.view.*; -import android.widget.*; -import android.support.v4.app.NavUtils; - -import fr.free.nrw.commons.*; -import fr.free.nrw.commons.WelcomeActivity; -import fr.free.nrw.commons.modifications.ModificationsContentProvider; import fr.free.nrw.commons.CommonsApplication; import fr.free.nrw.commons.EventLog; -import fr.free.nrw.commons.contributions.*; +import fr.free.nrw.commons.R; +import fr.free.nrw.commons.WelcomeActivity; +import fr.free.nrw.commons.contributions.ContributionsContentProvider; +import fr.free.nrw.commons.modifications.ModificationsContentProvider; public class LoginActivity extends AccountAuthenticatorActivity { @@ -79,7 +92,7 @@ public class LoginActivity extends AccountAuthenticatorActivity { } else if(result.equals("NotExists") || result.equals("Illegal") || result.equals("NotExists")) { response = R.string.login_failed_username; passwordEdit.setText(""); - } else if(result.equals("EmptyPass") || result.equals("WrongPass")) { + } else if(result.equals("EmptyPass") || result.equals("WrongPass") || result.equals("WrongPluginPass")) { response = R.string.login_failed_password; passwordEdit.setText(""); } else if(result.equals("Throttled")) { @@ -88,10 +101,11 @@ public class LoginActivity extends AccountAuthenticatorActivity { response = R.string.login_failed_blocked; } else { // Should never really happen + Log.d("Commons", "Login failed with reason: " + result); response = R.string.login_failed_generic; } - Crouton.makeText(context, response, Style.ALERT, R.id.loginErrors).show(); - dialog.dismiss(); + Toast.makeText(getApplicationContext(), response, Toast.LENGTH_LONG).show(); + dialog.cancel(); } } diff --git a/commons/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java b/commons/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java index c92c90f6c..4fae74a9f 100644 --- a/commons/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java +++ b/commons/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java @@ -5,16 +5,18 @@ import android.content.ContentResolver; import android.content.Intent; import android.content.pm.PackageManager; import android.net.Uri; +import android.os.Build; import android.os.Bundle; import android.os.Environment; +import android.support.design.widget.Snackbar; import android.support.v4.app.ActivityCompat; import android.support.v4.app.NavUtils; -import android.support.design.widget.Snackbar; import android.support.v4.content.ContextCompat; import android.util.Log; import android.view.MenuItem; import android.view.View; import android.widget.ImageView; +import android.widget.TextView; import android.widget.Toast; import com.nostra13.universalimageloader.core.ImageLoader; @@ -28,6 +30,7 @@ import fr.free.nrw.commons.R; import fr.free.nrw.commons.auth.AuthenticatedActivity; import fr.free.nrw.commons.auth.WikiAccountAuthenticator; import fr.free.nrw.commons.category.CategorizationFragment; +import fr.free.nrw.commons.contributions.Contribution; import fr.free.nrw.commons.modifications.CategoryModifier; import fr.free.nrw.commons.modifications.ModificationsContentProvider; import fr.free.nrw.commons.modifications.ModifierSequence; @@ -39,12 +42,12 @@ import fr.free.nrw.commons.modifications.TemplateRemoveModifier; */ public class ShareActivity extends AuthenticatedActivity - implements fr.free.nrw.commons.upload.SingleUploadFragment.OnUploadActionInitiated, + implements SingleUploadFragment.OnUploadActionInitiated, CategorizationFragment.OnCategoriesSaveHandler { private static final String TAG = ShareActivity.class.getName(); - private fr.free.nrw.commons.upload.SingleUploadFragment shareView; + private SingleUploadFragment shareView; private CategorizationFragment categorizationFragment; private CommonsApplication app; @@ -54,14 +57,14 @@ public class ShareActivity private String mediaUriString; private Uri mediaUri; - private fr.free.nrw.commons.contributions.Contribution contribution; + private Contribution contribution; private ImageView backgroundImageView; - private fr.free.nrw.commons.upload.UploadController uploadController; + private UploadController uploadController; private CommonsApplication cacheObj; private boolean cacheFound; - private fr.free.nrw.commons.upload.GPSExtractor imageObj; + private GPSExtractor imageObj; public ShareActivity() { super(WikiAccountAuthenticator.COMMONS_ACCOUNT_TYPE); @@ -77,8 +80,8 @@ public class ShareActivity Log.d(TAG, "Cache the categories found"); } - uploadController.startUpload(title, mediaUri, description, mimeType, source, new fr.free.nrw.commons.upload.UploadController.ContributionUploadProgress() { - public void onUploadStarted(fr.free.nrw.commons.contributions.Contribution contribution) { + uploadController.startUpload(title, mediaUri, description, mimeType, source, new UploadController.ContributionUploadProgress() { + public void onUploadStarted(Contribution contribution) { ShareActivity.this.contribution = contribution; showPostUpload(); } @@ -140,7 +143,7 @@ public class ShareActivity } else { EventLog.schema(CommonsApplication.EVENT_UPLOAD_ATTEMPT) .param("username", app.getCurrentAccount().name) - .param("source", getIntent().getStringExtra(fr.free.nrw.commons.upload.UploadService.EXTRA_SOURCE)) + .param("source", getIntent().getStringExtra(UploadService.EXTRA_SOURCE)) .param("multiple", true) .param("result", "cancelled") .log(); @@ -153,10 +156,10 @@ public class ShareActivity app.getApi().setAuthCookie(authCookie); - shareView = (fr.free.nrw.commons.upload.SingleUploadFragment) getSupportFragmentManager().findFragmentByTag("shareView"); + shareView = (SingleUploadFragment) getSupportFragmentManager().findFragmentByTag("shareView"); categorizationFragment = (CategorizationFragment) getSupportFragmentManager().findFragmentByTag("categorization"); if(shareView == null && categorizationFragment == null) { - shareView = new fr.free.nrw.commons.upload.SingleUploadFragment(); + shareView = new SingleUploadFragment(); this.getSupportFragmentManager() .beginTransaction() .add(R.id.single_upload_fragment_container, shareView, "shareView") @@ -173,10 +176,15 @@ public class ShareActivity finish(); } + /** + * Initiates retrieval of image coordinates or user coordinates, and caching of coordinates. + * Then initiates the calls to MediaWiki API through an instance of MwVolleyApi. + */ + @Override public void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); - uploadController = new fr.free.nrw.commons.upload.UploadController(this); + uploadController = new UploadController(this); setContentView(R.layout.activity_share); app = (CommonsApplication)this.getApplicationContext(); @@ -186,10 +194,10 @@ public class ShareActivity if(intent.getAction().equals(Intent.ACTION_SEND)) { mediaUri = (Uri) intent.getParcelableExtra(Intent.EXTRA_STREAM); - if(intent.hasExtra(fr.free.nrw.commons.upload.UploadService.EXTRA_SOURCE)) { - source = intent.getStringExtra(fr.free.nrw.commons.upload.UploadService.EXTRA_SOURCE); + if(intent.hasExtra(UploadService.EXTRA_SOURCE)) { + source = intent.getStringExtra(UploadService.EXTRA_SOURCE); } else { - source = fr.free.nrw.commons.contributions.Contribution.SOURCE_EXTERNAL; + source = Contribution.SOURCE_EXTERNAL; } mimeType = intent.getType(); } @@ -203,80 +211,94 @@ public class ShareActivity contribution = savedInstanceState.getParcelable("contribution"); } requestAuthToken(); - } - /** - * Initiates retrieval of image coordinates or user coordinates, and caching of coordinates. - * Then initiates the calls to MediaWiki API through an instance of MwVolleyApi. - */ - @Override - public void onResume() { - super.onResume(); Log.d(TAG, "Uri: " + mediaUriString); Log.d(TAG, "Ext storage dir: " + Environment.getExternalStorageDirectory()); - - - // Check permissions - if (ContextCompat.checkSelfPermission(this.getApplicationContext(), - Manifest.permission.READ_EXTERNAL_STORAGE) - != PackageManager.PERMISSION_GRANTED) { - - if (ActivityCompat.shouldShowRequestPermissionRationale(this, - Manifest.permission.READ_EXTERNAL_STORAGE)) { - - Log.i(TAG, - "Displaying camera permission rationale to provide additional context."); - Snackbar.make(this.findViewById(android.R.id.content), R.string.storage_permission_rationale, Snackbar.LENGTH_INDEFINITE).setAction(R.string.ok, new View.OnClickListener() { + // Check storage permissions if marshmallow or newer + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M && + (ContextCompat.checkSelfPermission(this, + Manifest.permission.READ_EXTERNAL_STORAGE) != PackageManager.PERMISSION_GRANTED || + ContextCompat.checkSelfPermission(this, Manifest.permission.ACCESS_FINE_LOCATION) != PackageManager.PERMISSION_GRANTED)) { + if (!(ActivityCompat.shouldShowRequestPermissionRationale(this, Manifest.permission.READ_EXTERNAL_STORAGE) && (ActivityCompat.shouldShowRequestPermissionRationale(this, Manifest.permission.ACCESS_FINE_LOCATION)))) { + String permissionRationales = getResources().getString(R.string.storage_permission_rationale) + "\n" + getResources().getString(R.string.location_permission_rationale); + Snackbar snackbar = Snackbar.make(findViewById(android.R.id.content), permissionRationales, + Snackbar.LENGTH_INDEFINITE) + .setAction(R.string.ok, new View.OnClickListener() { @Override public void onClick(View view) { - ActivityCompat.requestPermissions(ShareActivity.this, new String[]{Manifest.permission.READ_EXTERNAL_STORAGE}, 1); + ActivityCompat.requestPermissions(ShareActivity.this, + new String[]{Manifest.permission.READ_EXTERNAL_STORAGE, Manifest.permission.ACCESS_FINE_LOCATION}, 1); + } + }); + snackbar.show(); + View snackbarView = snackbar.getView(); + TextView textView = (TextView) snackbarView.findViewById(android.support.design.R.id.snackbar_text); + textView.setMaxLines(3); + } else if (!ActivityCompat.shouldShowRequestPermissionRationale(this, Manifest.permission.READ_EXTERNAL_STORAGE)) { + Snackbar.make(findViewById(android.R.id.content), R.string.storage_permission_rationale, + Snackbar.LENGTH_INDEFINITE) + .setAction(R.string.ok, new View.OnClickListener() { + @Override + public void onClick(View view) { + ActivityCompat.requestPermissions(ShareActivity.this, + new String[]{Manifest.permission.READ_EXTERNAL_STORAGE}, 1); + } + }).show(); + } else if (!ActivityCompat.shouldShowRequestPermissionRationale(this, Manifest.permission.ACCESS_FINE_LOCATION)) { + Snackbar.make(findViewById(android.R.id.content), R.string.location_permission_rationale, + Snackbar.LENGTH_INDEFINITE) + .setAction(R.string.ok, new View.OnClickListener() { + @Override + public void onClick(View view) { + ActivityCompat.requestPermissions(ShareActivity.this, + new String[]{Manifest.permission.READ_EXTERNAL_STORAGE}, 1); } }).show(); - - } else { - ActivityCompat.requestPermissions(this, new String[]{Manifest.permission.READ_EXTERNAL_STORAGE}, 1); } - } + } else { + // Convert image Uri to file path + String filePath = FileUtils.getPath(this, mediaUri); + Log.d(TAG, "Filepath: " + filePath); - //convert image Uri to file path - String filePath = fr.free.nrw.commons.upload.FileUtils.getPath(this, mediaUri); - Log.d(TAG, "Filepath: " + filePath); + // Check location permissions if marshmallow or newer + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M || ContextCompat.checkSelfPermission(this, + Manifest.permission.ACCESS_FINE_LOCATION) == PackageManager.PERMISSION_GRANTED) { + Log.d(TAG, "Calling GPSExtractor"); + imageObj = new GPSExtractor(filePath, this); + imageObj.registerLocationManager(); - Log.d(TAG, "Calling GPSExtractor"); - imageObj = new fr.free.nrw.commons.upload.GPSExtractor(filePath, this); - imageObj.registerLocationManager(); + if (filePath != null && !filePath.equals("")) { + // Gets image coords if exist, otherwise gets last known coords + String decimalCoords = imageObj.getCoords(); - if (filePath != null && !filePath.equals("")) { - //Gets image coords if exist, otherwise gets last known coords - String decimalCoords = imageObj.getCoords(); + if (decimalCoords != null) { + Log.d(TAG, "Decimal coords of image: " + decimalCoords); - if (decimalCoords != null) { - Log.d(TAG, "Decimal coords of image: " + decimalCoords); + // Only set cache for this point if image has coords + if (imageObj.imageCoordsExists) { + double decLongitude = imageObj.getDecLongitude(); + double decLatitude = imageObj.getDecLatitude(); + app.cacheData.setQtPoint(decLongitude, decLatitude); + } - //Only set cache for this point if image has coords - if (imageObj.imageCoordsExists) { - double decLongitude = imageObj.getDecLongitude(); - double decLatitude = imageObj.getDecLatitude(); - app.cacheData.setQtPoint(decLongitude, decLatitude); - } + MwVolleyApi apiCall = new MwVolleyApi(this); - fr.free.nrw.commons.upload.MwVolleyApi apiCall = new fr.free.nrw.commons.upload.MwVolleyApi(this); + List displayCatList = app.cacheData.findCategory(); + boolean catListEmpty = displayCatList.isEmpty(); - List displayCatList = app.cacheData.findCategory(); - boolean catListEmpty = displayCatList.isEmpty(); - - //if no categories found in cache, call MW API to match image coords with nearby Commons categories - if (catListEmpty) { - cacheFound = false; - apiCall.request(decimalCoords); - Log.d(TAG, "displayCatList size 0, calling MWAPI" + displayCatList.toString()); - - } else { - cacheFound = true; - Log.d(TAG, "Cache found, setting categoryList in MwVolleyApi to " + displayCatList.toString()); - fr.free.nrw.commons.upload.MwVolleyApi.setGpsCat(displayCatList); + // If no categories found in cache, call MediaWiki API to match image coords with nearby Commons categories + if (catListEmpty) { + cacheFound = false; + apiCall.request(decimalCoords); + Log.d(TAG, "displayCatList size 0, calling MWAPI" + displayCatList.toString()); + } else { + cacheFound = true; + Log.d(TAG, "Cache found, setting categoryList in MwVolleyApi to " + displayCatList.toString()); + MwVolleyApi.setGpsCat(displayCatList); + } + } } } } @@ -285,7 +307,10 @@ public class ShareActivity @Override public void onPause() { super.onPause(); - imageObj.unregisterLocationManager(); + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M || ContextCompat.checkSelfPermission(this, + Manifest.permission.ACCESS_FINE_LOCATION) == PackageManager.PERMISSION_GRANTED) { + imageObj.unregisterLocationManager(); + } } @Override diff --git a/commons/app/src/main/res/values/strings.xml b/commons/app/src/main/res/values/strings.xml index f0bd2f760..82623916e 100644 --- a/commons/app/src/main/res/values/strings.xml +++ b/commons/app/src/main/res/values/strings.xml @@ -149,6 +149,8 @@ Campaigns Refresh - Storage permission is needed to access photos + Recommended: Storage for photo metadata + Optional: Location for geo-tag OK + Back From 6adf78bec91294450a83f74425ad9230e3d1bed5 Mon Sep 17 00:00:00 2001 From: Adam Jones Date: Thu, 9 Jun 2016 13:21:25 +0100 Subject: [PATCH 3/5] Catch NullPointerException when unregistering locationManager --- .../java/fr/free/nrw/commons/upload/ShareActivity.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/commons/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java b/commons/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java index 4fae74a9f..16d0d429f 100644 --- a/commons/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java +++ b/commons/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java @@ -307,9 +307,13 @@ public class ShareActivity @Override public void onPause() { super.onPause(); - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M || ContextCompat.checkSelfPermission(this, - Manifest.permission.ACCESS_FINE_LOCATION) == PackageManager.PERMISSION_GRANTED) { + + try { imageObj.unregisterLocationManager(); + Log.d(TAG, "Unregistered locationManager"); + } + catch (NullPointerException e) { + Log.d(TAG, "locationManager does not exist, not unregistered"); } } From d5278f0c85d4493ba98c2b1b13b872d64cee88fa Mon Sep 17 00:00:00 2001 From: Adam Jones Date: Thu, 9 Jun 2016 14:53:14 +0100 Subject: [PATCH 4/5] Fix location permission requirement Add feature to extract location rom EXIF data which works when location permission has not been granted yet. Also does not find location when uploading an image which already has a location in EXIF. --- .../free/nrw/commons/upload/GPSExtractor.java | 19 ++- .../nrw/commons/upload/ShareActivity.java | 160 ++++++++++++------ commons/app/src/main/res/values/strings.xml | 2 +- 3 files changed, 121 insertions(+), 60 deletions(-) diff --git a/commons/app/src/main/java/fr/free/nrw/commons/upload/GPSExtractor.java b/commons/app/src/main/java/fr/free/nrw/commons/upload/GPSExtractor.java index c45952606..0486b00e5 100644 --- a/commons/app/src/main/java/fr/free/nrw/commons/upload/GPSExtractor.java +++ b/commons/app/src/main/java/fr/free/nrw/commons/upload/GPSExtractor.java @@ -9,6 +9,7 @@ import android.location.LocationManager; import android.media.ExifInterface; import android.os.Bundle; import android.preference.PreferenceManager; +import android.support.design.widget.Snackbar; import android.util.Log; import java.io.IOException; @@ -71,7 +72,7 @@ public class GPSExtractor { * Extracts geolocation of image from EXIF data. * @return coordinates of image as string (needs to be passed as a String in API query) */ - public String getCoords() { + public String getCoords(boolean useGPS) { ExifInterface exif; String latitude = ""; @@ -87,25 +88,27 @@ public class GPSExtractor { return null; } - if (exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE) == null) { + if (exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE) == null && useGPS) { + registerLocationManager(); + imageCoordsExists = false; - Log.d(TAG, "Picture has no GPS info"); + Log.d(TAG, "EXIF data has no location info"); //Check what user's preference is for automatic location detection boolean gpsPrefEnabled = gpsPreferenceEnabled(); if (gpsPrefEnabled) { Log.d(TAG, "Current location values: Lat = " + currentLatitude + " Long = " + currentLongitude); - String currentCoords = String.valueOf(currentLatitude) + "|" + String.valueOf(currentLongitude); - return currentCoords; + return String.valueOf(currentLatitude) + "|" + String.valueOf(currentLongitude); } else { - //Otherwise treat as if no coords found + // No coords found return null; } - + } else if(exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE) == null) { + return null; } else { imageCoordsExists = true; - Log.d(TAG, "Picture has GPS info"); + Log.d(TAG, "EXIF data has location info"); latitude = exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE); latitude_ref = exif.getAttribute(ExifInterface.TAG_GPS_LATITUDE_REF); diff --git a/commons/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java b/commons/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java index 16d0d429f..a89310b28 100644 --- a/commons/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java +++ b/commons/app/src/main/java/fr/free/nrw/commons/upload/ShareActivity.java @@ -65,6 +65,12 @@ public class ShareActivity private boolean cacheFound; private GPSExtractor imageObj; + private String filePath; + private String decimalCoords; + + private boolean useNewPermissions = false; + private boolean storagePermission = false; + private boolean locationPermission = false; public ShareActivity() { super(WikiAccountAuthenticator.COMMONS_ACCOUNT_TYPE); @@ -216,12 +222,19 @@ public class ShareActivity Log.d(TAG, "Uri: " + mediaUriString); Log.d(TAG, "Ext storage dir: " + Environment.getExternalStorageDirectory()); + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { + useNewPermissions = true; + if(ContextCompat.checkSelfPermission(this, Manifest.permission.READ_EXTERNAL_STORAGE) == PackageManager.PERMISSION_GRANTED) { + storagePermission = true; + } + if (ContextCompat.checkSelfPermission(this, Manifest.permission.ACCESS_FINE_LOCATION) == PackageManager.PERMISSION_GRANTED) { + locationPermission = true; + } + } + // Check storage permissions if marshmallow or newer - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M && - (ContextCompat.checkSelfPermission(this, - Manifest.permission.READ_EXTERNAL_STORAGE) != PackageManager.PERMISSION_GRANTED || - ContextCompat.checkSelfPermission(this, Manifest.permission.ACCESS_FINE_LOCATION) != PackageManager.PERMISSION_GRANTED)) { - if (!(ActivityCompat.shouldShowRequestPermissionRationale(this, Manifest.permission.READ_EXTERNAL_STORAGE) && (ActivityCompat.shouldShowRequestPermissionRationale(this, Manifest.permission.ACCESS_FINE_LOCATION)))) { + if (useNewPermissions && (!storagePermission || !locationPermission)) { + if (!storagePermission && !locationPermission) { String permissionRationales = getResources().getString(R.string.storage_permission_rationale) + "\n" + getResources().getString(R.string.location_permission_rationale); Snackbar snackbar = Snackbar.make(findViewById(android.R.id.content), permissionRationales, Snackbar.LENGTH_INDEFINITE) @@ -229,14 +242,14 @@ public class ShareActivity @Override public void onClick(View view) { ActivityCompat.requestPermissions(ShareActivity.this, - new String[]{Manifest.permission.READ_EXTERNAL_STORAGE, Manifest.permission.ACCESS_FINE_LOCATION}, 1); + new String[]{Manifest.permission.READ_EXTERNAL_STORAGE, Manifest.permission.ACCESS_FINE_LOCATION}, 3); } }); snackbar.show(); View snackbarView = snackbar.getView(); TextView textView = (TextView) snackbarView.findViewById(android.support.design.R.id.snackbar_text); textView.setMaxLines(3); - } else if (!ActivityCompat.shouldShowRequestPermissionRationale(this, Manifest.permission.READ_EXTERNAL_STORAGE)) { + } else if (!storagePermission) { Snackbar.make(findViewById(android.R.id.content), R.string.storage_permission_rationale, Snackbar.LENGTH_INDEFINITE) .setAction(R.string.ok, new View.OnClickListener() { @@ -246,60 +259,106 @@ public class ShareActivity new String[]{Manifest.permission.READ_EXTERNAL_STORAGE}, 1); } }).show(); - } else if (!ActivityCompat.shouldShowRequestPermissionRationale(this, Manifest.permission.ACCESS_FINE_LOCATION)) { + } else if (!locationPermission) { Snackbar.make(findViewById(android.R.id.content), R.string.location_permission_rationale, Snackbar.LENGTH_INDEFINITE) .setAction(R.string.ok, new View.OnClickListener() { @Override public void onClick(View view) { ActivityCompat.requestPermissions(ShareActivity.this, - new String[]{Manifest.permission.READ_EXTERNAL_STORAGE}, 1); + new String[]{Manifest.permission.ACCESS_FINE_LOCATION}, 2); } }).show(); } - } else { - // Convert image Uri to file path - String filePath = FileUtils.getPath(this, mediaUri); - Log.d(TAG, "Filepath: " + filePath); + } else if (useNewPermissions && storagePermission && !locationPermission) { + getFileMetadata(); + } else if(!useNewPermissions || (storagePermission && locationPermission)) { + getFileMetadata(); + getLocationData(); + } + } - // Check location permissions if marshmallow or newer - if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M || ContextCompat.checkSelfPermission(this, - Manifest.permission.ACCESS_FINE_LOCATION) == PackageManager.PERMISSION_GRANTED) { - Log.d(TAG, "Calling GPSExtractor"); - imageObj = new GPSExtractor(filePath, this); - imageObj.registerLocationManager(); - - if (filePath != null && !filePath.equals("")) { - // Gets image coords if exist, otherwise gets last known coords - String decimalCoords = imageObj.getCoords(); - - if (decimalCoords != null) { - Log.d(TAG, "Decimal coords of image: " + decimalCoords); - - // Only set cache for this point if image has coords - if (imageObj.imageCoordsExists) { - double decLongitude = imageObj.getDecLongitude(); - double decLatitude = imageObj.getDecLatitude(); - app.cacheData.setQtPoint(decLongitude, decLatitude); - } - - MwVolleyApi apiCall = new MwVolleyApi(this); - - List displayCatList = app.cacheData.findCategory(); - boolean catListEmpty = displayCatList.isEmpty(); - - // If no categories found in cache, call MediaWiki API to match image coords with nearby Commons categories - if (catListEmpty) { - cacheFound = false; - apiCall.request(decimalCoords); - Log.d(TAG, "displayCatList size 0, calling MWAPI" + displayCatList.toString()); - } else { - cacheFound = true; - Log.d(TAG, "Cache found, setting categoryList in MwVolleyApi to " + displayCatList.toString()); - MwVolleyApi.setGpsCat(displayCatList); - } - } + @Override + public void onRequestPermissionsResult(int requestCode, + String permissions[], int[] grantResults) { + switch (requestCode) { + // 1 = Storage + case 1: { + if (grantResults.length > 0 + && grantResults[0] == PackageManager.PERMISSION_GRANTED) { + getFileMetadata(); } + return; + } + // 2 = Location + case 2: { + if (grantResults.length > 0 + && grantResults[0] == PackageManager.PERMISSION_GRANTED) { + getLocationData(); + } + return; + } + // 3 = Storage + Location + case 3: { + if (grantResults.length > 1 + && grantResults[0] == PackageManager.PERMISSION_GRANTED) { + getFileMetadata(); + } + if (grantResults.length > 1 + && grantResults[1] == PackageManager.PERMISSION_GRANTED) { + getLocationData(); + } + } + } + } + + public void getFileMetadata() { + filePath = FileUtils.getPath(this, mediaUri); + Log.d(TAG, "Filepath: " + filePath); + Log.d(TAG, "Calling GPSExtractor"); + imageObj = new GPSExtractor(filePath, this); + + if (filePath != null && !filePath.equals("")) { + // Gets image coords from exif data + decimalCoords = imageObj.getCoords(false); + useImageCoords(); + } + } + + public void getLocationData() { + if(imageObj == null) { + imageObj = new GPSExtractor(filePath, this); + } + + decimalCoords = imageObj.getCoords(true); + useImageCoords(); + } + + public void useImageCoords() { + if(decimalCoords != null) { + Log.d(TAG, "Decimal coords of image: " + decimalCoords); + + // Only set cache for this point if image has coords + if (imageObj.imageCoordsExists) { + double decLongitude = imageObj.getDecLongitude(); + double decLatitude = imageObj.getDecLatitude(); + app.cacheData.setQtPoint(decLongitude, decLatitude); + } + + MwVolleyApi apiCall = new MwVolleyApi(this); + + List displayCatList = app.cacheData.findCategory(); + boolean catListEmpty = displayCatList.isEmpty(); + + // If no categories found in cache, call MediaWiki API to match image coords with nearby Commons categories + if (catListEmpty) { + cacheFound = false; + apiCall.request(decimalCoords); + Log.d(TAG, "displayCatList size 0, calling MWAPI" + displayCatList.toString()); + } else { + cacheFound = true; + Log.d(TAG, "Cache found, setting categoryList in MwVolleyApi to " + displayCatList.toString()); + MwVolleyApi.setGpsCat(displayCatList); } } } @@ -307,7 +366,6 @@ public class ShareActivity @Override public void onPause() { super.onPause(); - try { imageObj.unregisterLocationManager(); Log.d(TAG, "Unregistered locationManager"); diff --git a/commons/app/src/main/res/values/strings.xml b/commons/app/src/main/res/values/strings.xml index 82623916e..a2468b57b 100644 --- a/commons/app/src/main/res/values/strings.xml +++ b/commons/app/src/main/res/values/strings.xml @@ -150,7 +150,7 @@ Refresh Recommended: Storage for photo metadata - Optional: Location for geo-tag + Optional: Current location for category suggestions OK Back From 20a3295f29e0feb99fba9000a4e41628d85bfadc Mon Sep 17 00:00:00 2001 From: Adam Jones Date: Mon, 13 Jun 2016 12:47:29 +0100 Subject: [PATCH 5/5] Add back accounts related permissions in manifest for APIs <23 --- commons/app/src/main/AndroidManifest.xml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/commons/app/src/main/AndroidManifest.xml b/commons/app/src/main/AndroidManifest.xml index ccf0998ed..485fabc24 100644 --- a/commons/app/src/main/AndroidManifest.xml +++ b/commons/app/src/main/AndroidManifest.xml @@ -14,6 +14,10 @@ + + + +