From 62b0346c397c68b8f9cea4460c8568a8fed3bc7b Mon Sep 17 00:00:00 2001 From: Ritika Date: Tue, 30 May 2023 21:05:15 +0530 Subject: [PATCH 1/7] MainActivity: add ACCESS_MEDIA_LOCATION permission check to retain location info in EXIF metadata --- .../nrw/commons/contributions/MainActivity.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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 46ecc1bb8b..59a6ca88b6 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 @@ -152,6 +152,20 @@ public void onCreate(Bundle savedInstanceState) { } } 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 + ); + } } } From 064d814bfdccaf81a1827ccb4eec44eff5107330 Mon Sep 17 00:00:00 2001 From: Ritika Date: Tue, 30 May 2023 21:28:34 +0530 Subject: [PATCH 2/7] remove redundant permission check and optimise imports --- .../contributions/ContributionController.java | 12 ------------ .../free/nrw/commons/contributions/MainActivity.java | 1 - 2 files changed, 13 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 57f77053c8..f53d49430c 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 @@ 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 void initiateCustomGalleryPickWithPermission(final Activity activity) { 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, 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 59a6ca88b6..a96f1f37bd 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.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; From ad13fe52120e66bbd402281aa604fb8c8462fdc9 Mon Sep 17 00:00:00 2001 From: Ritika Date: Thu, 8 Jun 2023 11:29:33 +0530 Subject: [PATCH 3/7] FilePicker: switch to ACTION_OPEN_DOCUMENT intent for opening image files --- .../main/java/fr/free/nrw/commons/filepicker/FilePicker.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 bc43cb1543..68b8a5a752 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 @@ -201,7 +201,7 @@ private static boolean isPhoto(Intent data) { } private static Intent plainGalleryPickerIntent() { - Intent intent = new Intent(Intent.ACTION_GET_CONTENT); + Intent intent = new Intent(Intent.ACTION_OPEN_DOCUMENT); intent.setType("image/*"); return intent; } From 1b9a668b16eaed2ec5a0cf66c70ccccd336b981f Mon Sep 17 00:00:00 2001 From: Ritika Date: Fri, 9 Jun 2023 10:45:52 +0530 Subject: [PATCH 4/7] add a comment explaining the change --- .../nrw/commons/filepicker/FilePicker.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) 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 68b8a5a752..d6398fe09e 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 @@ -201,6 +201,33 @@ private static boolean isPhoto(Intent data) { } private static Intent plainGalleryPickerIntent() { + /** + * 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 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. + * + * Note: The traditional file picker might get deprecated in future. + */ Intent intent = new Intent(Intent.ACTION_OPEN_DOCUMENT); intent.setType("image/*"); return intent; From e5945effa9693fad033218814be7634495701a85 Mon Sep 17 00:00:00 2001 From: Ritika Date: Wed, 14 Jun 2023 11:29:04 +0530 Subject: [PATCH 5/7] implement GET_CONTENT photo picker toggle switch --- .../contributions/ContributionController.java | 3 ++- .../nrw/commons/filepicker/FilePicker.java | 22 ++++++++++++------- app/src/main/res/values/strings.xml | 2 ++ app/src/main/res/xml/preferences.xml | 6 +++++ 4 files changed, 24 insertions(+), 9 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 f53d49430c..0a01ef70c3 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 @@ -79,7 +79,8 @@ public void initiateCustomGalleryPickWithPermission(final Activity activity) { */ 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/filepicker/FilePicker.java b/app/src/main/java/fr/free/nrw/commons/filepicker/FilePicker.java index d6398fe09e..f05f6a7e76 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 @@ private static Uri createCameraPictureFile(@NonNull Context context) throws IOEx 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 @@ private static int restoreType(@NonNull Context context) { * * @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,7 +201,7 @@ private static boolean isPhoto(Intent data) { return data == null || (data.getData() == null && data.getClipData() == null); } - private static Intent plainGalleryPickerIntent() { + 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. @@ -216,7 +217,8 @@ private static Intent plainGalleryPickerIntent() { * 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 as: + * 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 @@ -226,9 +228,13 @@ private static Intent plainGalleryPickerIntent() { * * So, this allows us to use the traditional file picker that does not redact location tags from EXIF. * - * Note: The traditional file picker might get deprecated in future. */ - Intent intent = new Intent(Intent.ACTION_OPEN_DOCUMENT); + 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/res/values/strings.xml b/app/src/main/res/values/strings.xml index 3d3ad7a635..315260c15b 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -440,6 +440,8 @@ 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 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 e2f98d8f06..17360bd2e9 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" /> + + Date: Wed, 14 Jun 2023 12:29:17 +0530 Subject: [PATCH 6/7] add location loss warning pop up --- .../commons/settings/SettingsFragment.java | 33 +++++++++++++++++++ app/src/main/res/values/strings.xml | 1 + 2 files changed, 34 insertions(+) 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 3df477f55c..23ab80610a 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.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 boolean onPreferenceClick(Preference preference) { 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 boolean onPreferenceClick(Preference preference) { } } + /** + * 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 315260c15b..b285b273e1 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -442,6 +442,7 @@ Upload your first media by tapping on the add button. 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. From 27dcc77524d3b86671d406dbb16acd61315efdcd Mon Sep 17 00:00:00 2001 From: Ritika Date: Wed, 14 Jun 2023 19:33:39 +0530 Subject: [PATCH 7/7] SettingsFragment: modify the comment about GET_CONTENT takeover for more clarity --- .../java/fr/free/nrw/commons/settings/SettingsFragment.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 23ab80610a..0846fa9dc4 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 @@ -176,8 +176,8 @@ public boolean onPreferenceClick(Preference preference) { } /** - * The new Photo Picker with GET_CONTENT takeover redacts location tags - * from EXIF metadata. + * 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 */