From 9a0f35c681990a6767cf7a8c442690a16702c67c Mon Sep 17 00:00:00 2001 From: Ritika Pahwa <83745993+RitikaPahwa4444@users.noreply.github.com> Date: Thu, 15 Jun 2023 06:35:55 +0530 Subject: [PATCH] 5196: Fix location stripped from EXIF metadata (#5227) * MainActivity: add ACCESS_MEDIA_LOCATION permission check to retain location info in EXIF metadata * remove redundant permission check and optimise imports * FilePicker: switch to ACTION_OPEN_DOCUMENT intent for opening image files * add a comment explaining the change * implement GET_CONTENT photo picker toggle switch * add location loss warning pop up * SettingsFragment: modify the comment about GET_CONTENT takeover for more clarity --- .../contributions/ContributionController.java | 15 +------ .../commons/contributions/MainActivity.java | 15 ++++++- .../nrw/commons/filepicker/FilePicker.java | 45 ++++++++++++++++--- .../commons/settings/SettingsFragment.java | 33 ++++++++++++++ app/src/main/res/values/strings.xml | 3 ++ app/src/main/res/xml/preferences.xml | 6 +++ 6 files changed, 97 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionController.java b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionController.java index 57f77053c..0a01ef70c 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/ContributionController.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/ContributionController.java @@ -3,12 +3,9 @@ package fr.free.nrw.commons.contributions; import static fr.free.nrw.commons.wikidata.WikidataConstants.PLACE_OBJECT; import android.Manifest; -import android.Manifest.permission; import android.app.Activity; import android.content.Context; import android.content.Intent; -import android.os.Build.VERSION; -import android.os.Build.VERSION_CODES; import androidx.annotation.NonNull; import fr.free.nrw.commons.R; import fr.free.nrw.commons.filepicker.DefaultCallback; @@ -70,15 +67,6 @@ public class ContributionController { PermissionUtils.checkPermissionsAndPerformAction(activity, Manifest.permission.WRITE_EXTERNAL_STORAGE, () -> { - if (VERSION.SDK_INT >= VERSION_CODES.Q) { - PermissionUtils.checkPermissionsAndPerformAction( - activity, - permission.ACCESS_MEDIA_LOCATION, - () -> {}, - R.string.media_location_permission_denied, - R.string.add_location_manually - ); - } FilePicker.openCustomSelector(activity, 0); }, R.string.storage_permission_title, @@ -91,7 +79,8 @@ public class ContributionController { */ private void initiateGalleryUpload(final Activity activity, final boolean allowMultipleUploads) { setPickerConfiguration(activity, allowMultipleUploads); - FilePicker.openGallery(activity, 0); + boolean isGetContentPickerPreferred = defaultKvStore.getBoolean("getContentPhotoPickerPref"); + FilePicker.openGallery(activity, 0, isGetContentPickerPreferred); } /** diff --git a/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java b/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java index 46ecc1bb8..a96f1f37b 100644 --- a/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java +++ b/app/src/main/java/fr/free/nrw/commons/contributions/MainActivity.java @@ -5,7 +5,6 @@ import android.app.Activity; import android.content.Context; import android.content.Intent; import android.content.SharedPreferences; -import android.content.pm.PackageManager; import android.os.Build.VERSION; import android.os.Build.VERSION_CODES; import android.os.Bundle; @@ -152,6 +151,20 @@ public class MainActivity extends BaseActivity } } setUpPager(); + /** + * Ask the user for media location access just after login + * so that location in the EXIF metadata of the images shared by the user + * is retained on devices running Android 10 or above + */ + if (VERSION.SDK_INT >= VERSION_CODES.Q) { + PermissionUtils.checkPermissionsAndPerformAction( + this, + permission.ACCESS_MEDIA_LOCATION, + () -> {}, + R.string.media_location_permission_denied, + R.string.add_location_manually + ); + } } } diff --git a/app/src/main/java/fr/free/nrw/commons/filepicker/FilePicker.java b/app/src/main/java/fr/free/nrw/commons/filepicker/FilePicker.java index bc43cb154..f05f6a7e7 100644 --- a/app/src/main/java/fr/free/nrw/commons/filepicker/FilePicker.java +++ b/app/src/main/java/fr/free/nrw/commons/filepicker/FilePicker.java @@ -46,10 +46,11 @@ public class FilePicker implements Constants { return uri; } - private static Intent createGalleryIntent(@NonNull Context context, int type) { + private static Intent createGalleryIntent(@NonNull Context context, int type, + boolean isGetContentPickerPreferred) { // storing picked image type to shared preferences storeType(context, type); - return plainGalleryPickerIntent() + return plainGalleryPickerIntent(isGetContentPickerPreferred) .putExtra(Intent.EXTRA_ALLOW_MULTIPLE, configuration(context).allowsMultiplePickingInGallery()); } @@ -105,8 +106,8 @@ public class FilePicker implements Constants { * * @param type Custom type of your choice, which will be returned with the images */ - public static void openGallery(Activity activity, int type) { - Intent intent = createGalleryIntent(activity, type); + public static void openGallery(Activity activity, int type, boolean isGetContentPickerPreferred) { + Intent intent = createGalleryIntent(activity, type, isGetContentPickerPreferred); activity.startActivityForResult(intent, RequestCodes.PICK_PICTURE_FROM_GALLERY); } @@ -200,8 +201,40 @@ public class FilePicker implements Constants { return data == null || (data.getData() == null && data.getClipData() == null); } - private static Intent plainGalleryPickerIntent() { - Intent intent = new Intent(Intent.ACTION_GET_CONTENT); + private static Intent plainGalleryPickerIntent(boolean isGetContentPickerPreferred) { + /** + * Asking for ACCESS_MEDIA_LOCATION at runtime solved the location-loss issue + * in the custom selector in Contributions fragment. + * Detailed discussion: https://github.com/commons-app/apps-android-commons/issues/5015 + * + * This permission check, however, was insufficient to fix location-loss in + * the regular selector in Contributions fragment and Nearby fragment, + * especially on some devices running Android 13 that use the new Photo Picker by default. + * + * New Photo Picker: https://developer.android.com/training/data-storage/shared/photopicker + * + * The new Photo Picker introduced by Android redacts location tags from EXIF metadata. + * Reported on the Google Issue Tracker: https://issuetracker.google.com/issues/243294058 + * Status: Won't fix (Intended behaviour) + * + * Switched intent from ACTION_GET_CONTENT to ACTION_OPEN_DOCUMENT + * (based on user's preference) as: + * + * ACTION_GET_CONTENT opens the 'best application' for choosing that kind of data + * The best application is the new Photo Picker that redacts the location tags + * + * ACTION_OPEN_DOCUMENT, however, displays the various DocumentsProvider instances + * installed on the device, letting the user interactively navigate through them. + * + * So, this allows us to use the traditional file picker that does not redact location tags from EXIF. + * + */ + Intent intent; + if (isGetContentPickerPreferred) { + intent = new Intent(Intent.ACTION_GET_CONTENT); + } else { + intent = new Intent(Intent.ACTION_OPEN_DOCUMENT); + } intent.setType("image/*"); return intent; } diff --git a/app/src/main/java/fr/free/nrw/commons/settings/SettingsFragment.java b/app/src/main/java/fr/free/nrw/commons/settings/SettingsFragment.java index 3df477f55..0846fa9dc 100644 --- a/app/src/main/java/fr/free/nrw/commons/settings/SettingsFragment.java +++ b/app/src/main/java/fr/free/nrw/commons/settings/SettingsFragment.java @@ -42,6 +42,7 @@ import fr.free.nrw.commons.recentlanguages.Language; import fr.free.nrw.commons.recentlanguages.RecentLanguagesAdapter; import fr.free.nrw.commons.recentlanguages.RecentLanguagesDao; import fr.free.nrw.commons.upload.LanguagesAdapter; +import fr.free.nrw.commons.utils.DialogUtil; import fr.free.nrw.commons.utils.PermissionUtils; import fr.free.nrw.commons.utils.ViewUtil; import java.util.HashMap; @@ -71,6 +72,7 @@ public class SettingsFragment extends PreferenceFragmentCompat { private TextView recentLanguagesTextView; private View separator; private ListView languageHistoryListView; + private static final String GET_CONTENT_PICKER_HELP_URL = "https://commons-app.github.io/docs.html#get-content"; @Override public void onCreatePreferences(Bundle savedInstanceState, String rootKey) { @@ -150,6 +152,17 @@ public class SettingsFragment extends PreferenceFragmentCompat { checkPermissionsAndSendLogs(); return true; }); + + Preference getContentPickerPreference = findPreference("getContentPhotoPickerPref"); + getContentPickerPreference.setOnPreferenceChangeListener( + (preference, newValue) -> { + boolean isGetContentPickerTurnedOn = (boolean) newValue; + if (isGetContentPickerTurnedOn) { + showLocationLossWarning(); + } + return true; + } + ); // Disable some settings when not logged in. if (defaultKvStore.getBoolean("login_skipped", false)) { findPreference("useExternalStorage").setEnabled(false); @@ -162,6 +175,26 @@ public class SettingsFragment extends PreferenceFragmentCompat { } } + /** + * On some devices, the new Photo Picker with GET_CONTENT takeover + * redacts location tags from EXIF metadata + * + * Show warning to the user when ACTION_GET_CONTENT intent is enabled + */ + private void showLocationLossWarning() { + DialogUtil.showAlertDialog( + getActivity(), + null, + getString(R.string.location_loss_warning), + getString(R.string.ok), + getString(R.string.read_help_link), + () -> {}, + () -> Utils.handleWebUrl(requireContext(), Uri.parse(GET_CONTENT_PICKER_HELP_URL)), + null, + true + ); + } + @Override protected Adapter onCreateAdapter(final PreferenceScreen preferenceScreen) { return new PreferenceGroupAdapter(preferenceScreen) { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 3d3ad7a63..b285b273e 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -440,6 +440,9 @@ Upload your first media by tapping on the add button. Ends on: Display campaigns See the ongoing campaigns + Use GET_CONTENT photo picker + Disable if your pictures get uploaded without location + Please make sure that this new Android picker does not strip location from your pictures. You won\'t see the campaigns anymore. However, you can re-enable this notification in Settings if you wish. This function requires network connection, please check your connection settings. diff --git a/app/src/main/res/xml/preferences.xml b/app/src/main/res/xml/preferences.xml index e2f98d8f0..17360bd2e 100644 --- a/app/src/main/res/xml/preferences.xml +++ b/app/src/main/res/xml/preferences.xml @@ -70,6 +70,12 @@ app:singleLineTitle="false" android:summary="@string/display_campaigns_explanation" android:title="@string/display_campaigns" /> + +