Skip to content

Fixes #5015 - custom image selector not identifying photo location #5190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 31, 2023
Merged

Fixes #5015 - custom image selector not identifying photo location #5190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/src/main/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<uses-permission android:name="com.google.android.apps.photos.permission.GOOGLE_PHOTOS" />
<uses-permission android:name="android.permission.SET_WALLPAPER"/>
<uses-permission android:name="android.permission.FOREGROUND_SERVICE" />
<uses-permission android:name="android.permission.ACCESS_MEDIA_LOCATION"/>

<queries>
<!-- Browser -->
Expand Down
16 changes: 15 additions & 1 deletion app/src/main/java/fr/free/nrw/commons/contributions/ContributionController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@
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;
Expand Down Expand Up @@ -66,7 +69,18 @@ public void initiateCustomGalleryPickWithPermission(final Activity activity) {

Copy link
Collaborator

@RitikaPahwa4444 RitikaPahwa4444 Mar 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting! This function is only for the custom selector, right? As @sivaraam stated here and suggested by @nicolas-raoul too, this may have to be implemented for the regular selector too.

Copy link
Contributor Author

@sivasubramaniamv sivasubramaniamv Mar 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is executed on pressing the custom selector. I suggest @sivaraam run the app with these changes & observe if the case is still the same. If so we have two options:

  1. Request permissions on clicking the + floating action button in ContributionsListFragment, so it is granted before the user could use any of the image pickers

  2. Copy the ACCESS_MEDIA_LOCATION permission request code block to the function that gets executed when the regular picker is pressed, ie: initiateGalleryPick() at line 60 in ContributionController.java

I guess the second option is the most appropriate as there is no need to obtain WRITE_EXTERNAL_STORAGE permission if the user never uses the custom picker.

Copy link
Member

@sivaraam sivaraam Apr 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to to share an image (which has location information) to the app while it does not have the location permission. When clicking on the map icon in the upload wizard, the app shows me the location drop pin shows an approx. location that is a bit far from the actual location. The actual location is not indicated at all.

When I repeat the same when the app has been granted the location permission, the location drop pin is situated a bit far away. Though, the actual location is at least indicated correctly.

PS: This seems to be the case for me when trying to upload via custom selector too (I'm not shown any location request dialog).

Copy link
Collaborator

@RitikaPahwa4444 RitikaPahwa4444 Apr 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sivaraam, this is happening with me as well. Is this not because the EXIF location may be different from user's current location (in case the user uploads the picture later from somewhere else)? The location drop pin would display the EXIF location and the actual location is indicated by the circular mark. As far as the request dialog is concerned, adding it in the beginning (as I'd demonstrated here; we can definitely look for a better way of accessing the dialog after the if block you had mentioned) or at some other appropriate stage will indicate the actual location too. The circular mark indicating the actual location will then not be visible only in case the user denies to provide location access.

PermissionUtils.checkPermissionsAndPerformAction(activity,
Manifest.permission.WRITE_EXTERNAL_STORAGE,
() -> FilePicker.openCustomSelector(activity, 0),
() -> {
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,
R.string.write_storage_permission_rationale);
}
Expand Down
3 changes: 3 additions & 0 deletions app/src/main/res/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,9 @@ Upload your first media by tapping on the add button.</string>
<string name="exif_tag_name_serialNumbers">Serial Numbers</string>
<string name="exif_tag_name_software">Software</string>

<string name="media_location_permission_denied">Media location access denied</string>
<string name="add_location_manually">We may not be able to automatically obtain location data from pictures you upload. Please add the appropriate location for each picture before submitting</string>

<string name="share_text">Upload photos to Wikimedia Commons directly from your phone. Download the Commons App now: %1$s</string>
<string name="share_via">Share app via...</string>
<string name="image_info">Image Info</string>
Expand Down