Skip to content

Commit 9a0f35c

Browse files
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
1 parent 4d71c30 commit 9a0f35c

File tree

6 files changed

+97
-20
lines changed

6 files changed

+97
-20
lines changed

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

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,9 @@
33
import static fr.free.nrw.commons.wikidata.WikidataConstants.PLACE_OBJECT;
44

55
import android.Manifest;
6-
import android.Manifest.permission;
76
import android.app.Activity;
87
import android.content.Context;
98
import android.content.Intent;
10-
import android.os.Build.VERSION;
11-
import android.os.Build.VERSION_CODES;
129
import androidx.annotation.NonNull;
1310
import fr.free.nrw.commons.R;
1411
import fr.free.nrw.commons.filepicker.DefaultCallback;
@@ -70,15 +67,6 @@ public void initiateCustomGalleryPickWithPermission(final Activity activity) {
7067
PermissionUtils.checkPermissionsAndPerformAction(activity,
7168
Manifest.permission.WRITE_EXTERNAL_STORAGE,
7269
() -> {
73-
if (VERSION.SDK_INT >= VERSION_CODES.Q) {
74-
PermissionUtils.checkPermissionsAndPerformAction(
75-
activity,
76-
permission.ACCESS_MEDIA_LOCATION,
77-
() -> {},
78-
R.string.media_location_permission_denied,
79-
R.string.add_location_manually
80-
);
81-
}
8270
FilePicker.openCustomSelector(activity, 0);
8371
},
8472
R.string.storage_permission_title,
@@ -91,7 +79,8 @@ public void initiateCustomGalleryPickWithPermission(final Activity activity) {
9179
*/
9280
private void initiateGalleryUpload(final Activity activity, final boolean allowMultipleUploads) {
9381
setPickerConfiguration(activity, allowMultipleUploads);
94-
FilePicker.openGallery(activity, 0);
82+
boolean isGetContentPickerPreferred = defaultKvStore.getBoolean("getContentPhotoPickerPref");
83+
FilePicker.openGallery(activity, 0, isGetContentPickerPreferred);
9584
}
9685

9786
/**

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

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import android.content.Context;
66
import android.content.Intent;
77
import android.content.SharedPreferences;
8-
import android.content.pm.PackageManager;
98
import android.os.Build.VERSION;
109
import android.os.Build.VERSION_CODES;
1110
import android.os.Bundle;
@@ -152,6 +151,20 @@ public void onCreate(Bundle savedInstanceState) {
152151
}
153152
}
154153
setUpPager();
154+
/**
155+
* Ask the user for media location access just after login
156+
* so that location in the EXIF metadata of the images shared by the user
157+
* is retained on devices running Android 10 or above
158+
*/
159+
if (VERSION.SDK_INT >= VERSION_CODES.Q) {
160+
PermissionUtils.checkPermissionsAndPerformAction(
161+
this,
162+
permission.ACCESS_MEDIA_LOCATION,
163+
() -> {},
164+
R.string.media_location_permission_denied,
165+
R.string.add_location_manually
166+
);
167+
}
155168
}
156169
}
157170

app/src/main/java/fr/free/nrw/commons/filepicker/FilePicker.java

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,11 @@ private static Uri createCameraPictureFile(@NonNull Context context) throws IOEx
4646
return uri;
4747
}
4848

49-
private static Intent createGalleryIntent(@NonNull Context context, int type) {
49+
private static Intent createGalleryIntent(@NonNull Context context, int type,
50+
boolean isGetContentPickerPreferred) {
5051
// storing picked image type to shared preferences
5152
storeType(context, type);
52-
return plainGalleryPickerIntent()
53+
return plainGalleryPickerIntent(isGetContentPickerPreferred)
5354
.putExtra(Intent.EXTRA_ALLOW_MULTIPLE, configuration(context).allowsMultiplePickingInGallery());
5455
}
5556

@@ -105,8 +106,8 @@ private static int restoreType(@NonNull Context context) {
105106
*
106107
* @param type Custom type of your choice, which will be returned with the images
107108
*/
108-
public static void openGallery(Activity activity, int type) {
109-
Intent intent = createGalleryIntent(activity, type);
109+
public static void openGallery(Activity activity, int type, boolean isGetContentPickerPreferred) {
110+
Intent intent = createGalleryIntent(activity, type, isGetContentPickerPreferred);
110111
activity.startActivityForResult(intent, RequestCodes.PICK_PICTURE_FROM_GALLERY);
111112
}
112113

@@ -200,8 +201,40 @@ private static boolean isPhoto(Intent data) {
200201
return data == null || (data.getData() == null && data.getClipData() == null);
201202
}
202203

203-
private static Intent plainGalleryPickerIntent() {
204-
Intent intent = new Intent(Intent.ACTION_GET_CONTENT);
204+
private static Intent plainGalleryPickerIntent(boolean isGetContentPickerPreferred) {
205+
/**
206+
* Asking for ACCESS_MEDIA_LOCATION at runtime solved the location-loss issue
207+
* in the custom selector in Contributions fragment.
208+
* Detailed discussion: https://github.com/commons-app/apps-android-commons/issues/5015
209+
*
210+
* This permission check, however, was insufficient to fix location-loss in
211+
* the regular selector in Contributions fragment and Nearby fragment,
212+
* especially on some devices running Android 13 that use the new Photo Picker by default.
213+
*
214+
* New Photo Picker: https://developer.android.com/training/data-storage/shared/photopicker
215+
*
216+
* The new Photo Picker introduced by Android redacts location tags from EXIF metadata.
217+
* Reported on the Google Issue Tracker: https://issuetracker.google.com/issues/243294058
218+
* Status: Won't fix (Intended behaviour)
219+
*
220+
* Switched intent from ACTION_GET_CONTENT to ACTION_OPEN_DOCUMENT
221+
* (based on user's preference) as:
222+
*
223+
* ACTION_GET_CONTENT opens the 'best application' for choosing that kind of data
224+
* The best application is the new Photo Picker that redacts the location tags
225+
*
226+
* ACTION_OPEN_DOCUMENT, however, displays the various DocumentsProvider instances
227+
* installed on the device, letting the user interactively navigate through them.
228+
*
229+
* So, this allows us to use the traditional file picker that does not redact location tags from EXIF.
230+
*
231+
*/
232+
Intent intent;
233+
if (isGetContentPickerPreferred) {
234+
intent = new Intent(Intent.ACTION_GET_CONTENT);
235+
} else {
236+
intent = new Intent(Intent.ACTION_OPEN_DOCUMENT);
237+
}
205238
intent.setType("image/*");
206239
return intent;
207240
}

app/src/main/java/fr/free/nrw/commons/settings/SettingsFragment.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import fr.free.nrw.commons.recentlanguages.RecentLanguagesAdapter;
4343
import fr.free.nrw.commons.recentlanguages.RecentLanguagesDao;
4444
import fr.free.nrw.commons.upload.LanguagesAdapter;
45+
import fr.free.nrw.commons.utils.DialogUtil;
4546
import fr.free.nrw.commons.utils.PermissionUtils;
4647
import fr.free.nrw.commons.utils.ViewUtil;
4748
import java.util.HashMap;
@@ -71,6 +72,7 @@ public class SettingsFragment extends PreferenceFragmentCompat {
7172
private TextView recentLanguagesTextView;
7273
private View separator;
7374
private ListView languageHistoryListView;
75+
private static final String GET_CONTENT_PICKER_HELP_URL = "https://commons-app.github.io/docs.html#get-content";
7476

7577
@Override
7678
public void onCreatePreferences(Bundle savedInstanceState, String rootKey) {
@@ -150,6 +152,17 @@ public boolean onPreferenceClick(Preference preference) {
150152
checkPermissionsAndSendLogs();
151153
return true;
152154
});
155+
156+
Preference getContentPickerPreference = findPreference("getContentPhotoPickerPref");
157+
getContentPickerPreference.setOnPreferenceChangeListener(
158+
(preference, newValue) -> {
159+
boolean isGetContentPickerTurnedOn = (boolean) newValue;
160+
if (isGetContentPickerTurnedOn) {
161+
showLocationLossWarning();
162+
}
163+
return true;
164+
}
165+
);
153166
// Disable some settings when not logged in.
154167
if (defaultKvStore.getBoolean("login_skipped", false)) {
155168
findPreference("useExternalStorage").setEnabled(false);
@@ -162,6 +175,26 @@ public boolean onPreferenceClick(Preference preference) {
162175
}
163176
}
164177

178+
/**
179+
* On some devices, the new Photo Picker with GET_CONTENT takeover
180+
* redacts location tags from EXIF metadata
181+
*
182+
* Show warning to the user when ACTION_GET_CONTENT intent is enabled
183+
*/
184+
private void showLocationLossWarning() {
185+
DialogUtil.showAlertDialog(
186+
getActivity(),
187+
null,
188+
getString(R.string.location_loss_warning),
189+
getString(R.string.ok),
190+
getString(R.string.read_help_link),
191+
() -> {},
192+
() -> Utils.handleWebUrl(requireContext(), Uri.parse(GET_CONTENT_PICKER_HELP_URL)),
193+
null,
194+
true
195+
);
196+
}
197+
165198
@Override
166199
protected Adapter onCreateAdapter(final PreferenceScreen preferenceScreen) {
167200
return new PreferenceGroupAdapter(preferenceScreen) {

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,9 @@ Upload your first media by tapping on the add button.</string>
440440
<string name="ends_on">Ends on:</string>
441441
<string name="display_campaigns">Display campaigns</string>
442442
<string name="display_campaigns_explanation">See the ongoing campaigns</string>
443+
<string name="get_content_photo_picker_title">Use GET_CONTENT photo picker</string>
444+
<string name="get_content_photo_picker_explanation">Disable if your pictures get uploaded without location</string>
445+
<string name="location_loss_warning">Please make sure that this new Android picker does not strip location from your pictures.</string>
443446

444447
<string name="nearby_campaign_dismiss_message">You won\'t see the campaigns anymore. However, you can re-enable this notification in Settings if you wish.</string>
445448
<string name="this_function_needs_network_connection">This function requires network connection, please check your connection settings.</string>

app/src/main/res/xml/preferences.xml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,12 @@
7070
app:singleLineTitle="false"
7171
android:summary="@string/display_campaigns_explanation"
7272
android:title="@string/display_campaigns" />
73+
74+
<SwitchPreference
75+
android:defaultValue="false"
76+
android:key="getContentPhotoPickerPref"
77+
android:summary="@string/get_content_photo_picker_explanation"
78+
android:title="@string/get_content_photo_picker_title"/>
7379
</PreferenceCategory>
7480

7581
<PreferenceCategory

0 commit comments

Comments
 (0)