5196: Fix in-app camera location loss #5249

RitikaPahwa4444 · 2023-06-24T13:22:07Z

Description (required)

What changes did you make and why?
Some default camera applications strip location tags off. Enabled location recording before and after image capture to deal with in-app camera location loss.

Side note: Uploads do not proceed as reported in issues #5233 and #5072. Testing beyond upload activity is subject to these upload issues getting resolved first.

Tests performed (required)

Tested prodDebug on Redmi 5A with API level 27 and Pixel 4 emulator with API level 33.

…tionDialog to have null location

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

app/src/main/java/fr/free/nrw/commons/upload/FileProcessor.kt

app/src/main/java/fr/free/nrw/commons/upload/UploadActivity.java

nicolas-raoul

@RitikaPahwa4444 Looks good to me! Would you mind removing the Draft/WIP status if you consider it good to merge?

@sivaraam This is working on my device whose in-app camera provides EXIF. How about on yours whose in-app camera does not?

@kartikaykaushik14 Any comment? :-)

sivaraam

I tested on a few devices. In the Android 8 and 12 devices, the location fails to be identified during the first upload. It's recognized in next upload attempts and attached properly, though.

I left a few other comments. Do check them.

sivaraam · 2023-06-29T18:03:57Z

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

+     */
+
+    private void openLocationSettings(Activity activity) {
+        final Intent intent = new Intent(Settings.ACTION_LOCATION_SOURCE_SETTINGS);


Strangely, this is opening the app's setting screen instead of the location settings screen on my device (Nord running Android 12). Any thoughts on possible reasons for this ? 🤔

I observed a similar behavior.

This is used at many places in the app already and works pretty well on my device, so I assumed it to be correct. Will check this out. Thank you for mentioning this!

sivaraam · 2023-06-29T18:10:22Z

app/src/main/java/fr/free/nrw/commons/upload/mediaDetails/UploadMediaDetailFragment.java

@@ -117,6 +119,7 @@ public class UploadMediaDetailFragment extends UploadBaseFragment implements
     */
    private Place nearbyPlace;
    private UploadItem uploadItem;
+    private LatLng location;


location

Better name this one inAppPictureLocation for better understanding? Otherwise it is easy to misunderstand this as the value representing actual location in all image upload cases.

Also, a comment clarifying the need to store this separately would also be useful.

A similar adjustment would be needed in other such instances too.

it is easy to misunderstand this as the value representing actual location in all image upload cases

I intentionally removed this distinction. It's either EXIF location or the current user location. We're treating both of them as actual. For all types of uploads, it is anyway the EXIF location that is being used (if available). I thought having this in the initial stages would be enough, it's "picture location" once we decide if it has be used or not.

I'm sorry for generalising it to this extent, will change it. I have added a detailed comment before askUserToAllowLocationAccess(), should I mention it in all the files?

sivaraam · 2023-06-29T18:57:02Z

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

+                    if (!(PermissionUtils.hasPermission(activity, Manifest.permission.ACCESS_FINE_LOCATION)
+                         && isLocationAccessToAppsTurnedOn())) {


Having a check like this to trigger the consent dialog could be a bit incorrect. We attach the location to the upload when the EXIF location is unavailable. This could also occur in cases where the app has location access and location is turned on the device but saving location is turned off in the device camera. The user could've done this intentionally. The app obtained location would be uploaded in this case without the user having any idea about it. I believe it is ideal to get the consent from the user before doing so.

There's also the case of lower Android version devices (such as API 21) for which there is no notion of location permission. So, this dialog is shown to them only when location is turned off.

I also observed this dialog appears each time when trying to use the in-app camera and location is either off / location permissions is unavailable. This might become a bit annoying if the user has intentionally not granted those.

To better handle this, I would suggested showcasing the consent dialog first time (unconditinally) when user users in-app camera and store their preference. Given this would be a one-time dialog, we should also have a corresponding preference in settings in case the user wants to toggle their choice.

When we have user's consent to attach the location, we could then show dialog about lack of location permission / location enabled when in-app camera is triggered.

This seems like a better and reasonable flow to me. Thoughts ?

@sivaraam Just a clarification -

Given this would be a one-time dialog, we should also have a corresponding preference in settings in case the user wants to toggle their choice.

Is this referring to displaying the consent dialog?
If yes, I'm a little skeptical about having a setting to toggle the choice to display the dialog. In most apps, we usually have an option like "Do not show this message again" and once the user selects the option, we do not see it again. A user who's curious about enabling location could probably go through the information we have already associated while enabling/disabling permissions for location and would not want a pop up enabled again.

I may be wrong but just wanted to understand the perspective behind it.

Having a check like this to trigger the consent dialog could be a bit incorrect.

Appeared incorrect to me as well, I felt I was missing out some case but did not know which one. Thank you for elaborating about it, I'll make this dialog compulsory on the first run.

Also, I'd suggested this "Do not show this message again" in my comment above. If this sounds okay, I'll add it for the Dismiss case (it does not appear again if the user allows).

sivaraam · 2023-06-29T19:02:09Z

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

+     */
+    private void askUserToAllowLocationAccess(Activity activity) {
+        DialogUtil.showAlertDialog(activity,
+            activity.getString(R.string.location_permission_title),


The title for this dialog could be something more meaningful than "Requesting location permission"

May be try "Record location for in-app shots" ?

kartikaykaushik14 · 2023-06-29T19:42:04Z

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

@@ -443,6 +443,11 @@ Upload your first media by tapping on the add button.</string>
  <string name="get_content_photo_picker_title">Use GET_CONTENT photo picker</string>
  <string name="get_content_photo_picker_explanation">Disable if your pictures get uploaded without location</string>
  <string name="location_loss_warning">Please make sure that this new Android picker does not strip location from your pictures.</string>
+  <string name="in_app_camera_location_access_explanation">Allow the app to fetch location in case the camera does not record it. Some device cameras do not record location. In such cases, letting the app fetch and attach location to it makes your contribution more useful.</string>


It looks alright but a suggestion to change the tone of message -

Allow the application to retrieve location data in the event that the camera fails to capture it. Certain device cameras may not have the capability to record location information. In such circumstances, enabling the application to fetch and append location data enhances the value of your contribution. Please remember that you can modify this setting at any time.

Please remember that you can modify this setting at any time.

Sounds good. Will add this once we're sure if we're using a "Do not ask again" checkbox or a toggle button in the Settings.

kartikaykaushik14

Added a couple of comments

RitikaPahwa4444 · 2023-06-30T05:54:17Z

In the Android 8 and 12 devices, the location fails to be identified during the first upload. It's recognized in next upload attempts and attached properly, though.

Not very surprising. This happens even on (many) default camera applications. When we allow location access for the first time, apps, including the camera apps, search for GPS. The time taken to search for GPS appears significantly less (usually) in subsequent runs because location is turned on already and the user spends enough time before tapping the click button. So, even default applications do not recognise locations in the first click.

This "searching for GPS" operation usually takes 5-7 seconds on my device but sometimes it may take longer (I mean too long sometimes). I can add a delay for the first upload for the app to search for GPS first and only then allow users to use the camera. I hope this will not give an impression that the in-app camera does not work at all to some users because of the long delay. Adding a threshold limit may help too. Any suggestions?

kartikaykaushik14 · 2023-07-03T07:01:06Z

Adding a few more alternatives to resolve the above problem.

We can consider using a progress bar or animated icon that indicates the progress of GPS initialization. This visual feedback can make the waiting process more engaging and provide users with a sense of progress and the users know that the in-app camera is working.
Overlay a transparent educational message or tip on the camera screen during the first upload attempt. This message can explain the delay and encourage users to hold steady or wait a few seconds for accurate location tagging. We need to ensure that the overlay is unobtrusive and doesn't interfere with the camera's functionality.
Display contextual tips or suggestions on the camera screen while the app is searching for GPS. These tips can include photography advice, composition tips, or information about nearby points of interest. This way, users have something informative and entertaining to engage with while they wait.

We would have to check the feasibility to implement these alternatives.

nicolas-raoul · 2023-07-03T07:06:00Z

@kartikaykaushik14 Great ideas! As we are already at the half of this GSoC, they can probably be considered for future improvements, or implemented at the end of the GSoC if there is time left. :-)

…ocation

RitikaPahwa4444 · 2023-07-04T14:35:52Z

Thank you for reviewing the pull request and suggesting improvements :) I've covered almost all of them. Only the first upload related problem needs to be dealt with.

nicolas-raoul · 2023-07-04T14:49:49Z

If first upload is too difficult to deal with, feel free to create an issue and keep it for the end of the GSoC.
Getting the second upload and the next ones right is an overwhelming improvement over the previous behavior of the app.

sivaraam

Added a few review comments based on my look at the code.

BTW, any luck on the incorrect settings page opening up ? I suspect that we're somehow showing the incorrect dialog in this particular case. I'm being shown the permission request dialog when the location turned on dialog should be shown

Consistently reproducible with following steps:

Ensure app does not have location permission and device's location is turned off.
Ensure "Record location for in-app shots" is turned off
Go to Settings.
Turn on the "Record location for in-app shots"
Grant the location permission for the app
Skip the dialog for turning on device location
Turn off the "Record location for in-app shots"
Turn on the "Record location for in-app shots"
A dialog is shown with a Settings button to turn on location
Clicking the "Settings" button takes me to the app's setting instead of location setting.

Note that the app already has location permission. It seems to me like the onPermissionDenied method of PermissionUtils.checkPermissionsAndPerformAction is being run for some reason regardless. @RitikaPahwa4444 could you look into this ?

If first upload is too difficult to deal with, feel free to create an issue and keep it for the end of the GSoC.
Getting the second upload and the next ones right is an overwhelming improvement over the previous behavior of the app.

I would suggest a bare minimum of at least warning the user that the location may not be recorded the first time. This could be done via the dialog / or by some other way if there's a better suit.

sivaraam · 2023-07-04T18:35:35Z

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

@@ -190,6 +190,8 @@
  <string name="read_storage_permission_rationale">Required permission: Read external storage. App cannot access your gallery without this.</string>
  <string name="write_storage_permission_rationale">Required permission: Write external storage. App cannot access your camera/gallery without this.</string>
  <string name="location_permission_title">Requesting Location Permission</string>
+  <string name="in_app_camera_location_permission_title">Record location for in-app shots</string>
+  <string name="in_app_camera_location_switch_pref_summary">Enable this to record location while using in-app camera</string>


We might need to concisely clarify that this is only considered when camera fails to record location.

I think we're explaining this in the in-app camera pop-up already.

The user has the liberty to toggle the setting anytime they want. So, it's better to be unambiguous in the setting description.

It's better not to presume that the user would remember (or) fully grasp what they saw in a one-time dialog.

I added this as a follow-up for the dialog (for privacy reasons) as discussed above. As a privacy setting, this summary sounded enough to me but in case we're adding this as an in-app camera related setting, should I move it to the General section instead of the Privacy section?

As a privacy setting, this summary sounded enough to me

If this setting were really controlling whether the app records location for in-app shots, the description is totally fine. The problem is that this setting only controls whether the app appends the location when the in-built camera doesn't provide us an location.

... in case we're adding this as an in-app camera related setting ...

Isn't this a in-app camera related setting already? What am I missing ? 🤔

should I move it to the General section instead of the Privacy section?

Moving it out of "Privacy" is a nice idea. Moving it into "General" isn't. That section is already too cluttered. I think we could introduce a seprarate section title named "Uploads" and move the following preferences into that section:

Save In-app shots

Use custom author name

Custom author name

Photo picker one

Record location with in-app shots (with a tweaked description)

This might be a better situation help de-clutter the settings and position this setting in a better place 🙂

This could also occur in cases where the app has location access and location is turned on the device but saving location is turned off in the device camera. The user could've done this intentionally. The app obtained location would be uploaded in this case without the user having any idea about it.

I'd added it as a privacy setting for this reason. Adding a separate section sounds better. Thank you for this great suggestion!

The problem is that this setting only controls whether the app appends the location when the in-built camera doesn't provide us an location.

I'm sorry if I made it too abstract, seems like I still need to understand a lot about user behaviour. Here's what I took into consideration:

As a user, I wouldn't have thought if the camera is recording the location or the app is doing it. As a new user, if I open the Settings first and like my pictures to be geotagged, I'll simply turn it on. And if I open the in-app camera first, I'll still turn it on to be on the safer side. I don't think naive users will test if the default camera app records location, for them, it's always our app that's not working properly (even if that's their camera's issue). In fact, the notion of default camera app exists only for us because we know we're triggering it using an intent. For users, it's just the app's camera.

Thank you for explaining it in detail, will add a separate section with modified summary 👍

I'll add a new section for the in-app shots location. Created a new issue (#5258) for the rest of the options as these are not a part of this issue. Will handle the rest of them there, I hope it sounds okay.

I'll add a new section for the in-app shots location. Created a new issue (#5258) for the rest of the options as these are not a part of this issue. Will handle the rest of them there, I hope it sounds okay.

To be honest, having a separate section with just the setting for in-app shots location is what sounds awkward to me. While it might seem a bit unrelated, doesn't it seem too trivial to be handled in a separate issue ? If handled in this PR, you'll only be doing the change as a part of a related change 🤔

The primary thing of concern is the section itself. A section for in-app shot location alone doesn't seem useful. The kind of categorization I suggest helps gather related settings and sounds reasonable. Or am I missing something ?

I'm sorry if this was not intended😔

You don't have to apologize for everything. All these are part of the contribution cycle. You just need to be sure to learn along the way 🙂

In case it helps, give the following article a read: How to Become A Hacker

sivaraam · 2023-07-04T19:02:29Z

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

+            permission.ACCESS_FINE_LOCATION,
+            () -> onLocationPermissionGranted(activity),
+            () -> {},
+            R.string.ask_to_turn_location_on,


IIUC, this should be R.string.location_permission_title since this is title for the location permission dialog.

sivaraam · 2023-07-04T19:03:01Z

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

+    private void showLocationOffDialog(Activity activity) {
+        DialogUtil
+            .showAlertDialog(activity,
+                activity.getString(R.string.location_permission_title),


This should be R.string.ask_to_turn_location_on since this is a dialog for requesting to turn on location.

sivaraam · 2023-07-04T19:03:51Z

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

+            permission.ACCESS_FINE_LOCATION,
+            () -> onLocationPermissionGranted(getActivity()),
+            () -> {},
+            R.string.ask_to_turn_location_on,


DItto on the title to be used here.

Speaking about dittos, could you look into avoiding duplication of these code in SettingsFragment and the ContributionController ? Duplication of the code is bad for maintenance. I believe it should be easy to make the code reusable. Let us know in case you have need any clarifications about the same.

Duplication of the code is bad for maintenance.

I usually do not write redundant code. I'm sorry but this is how the app is handling location related stuff currently. In fact, this needs to be standardised with utmost priority as this is leading to inconsistent behaviour at many places. This is the link to my pull request where I'd started standardising it (it had the same permissions related code, and the same map behaviour was required in all the fragments but still duplication was high).

Is it okay if I handle this separately?

It's a pity that the app already has much duplicated instances. Let that not be a motivator to add more tech debt, though. Kindly look into avoiding the duplication in this case. Do share if you face any difficulties with it.

The standardization you mention seems like a good thing that could certainly be pursued later on. Feel free to open an issue to keep track of it 👍🏼

sivaraam · 2023-07-04T19:19:25Z

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

@@ -99,6 +215,9 @@ private void setPickerConfiguration(Activity activity,
     */
    private void initiateCameraUpload(Activity activity) {
        setPickerConfiguration(activity, false);
+        if (defaultKvStore.getBoolean("inAppCameraLocationPref", false)) {
+            locationBeforeImageCapture = locationManager.getLastLocation();


We're presuming the device location would be turned on whenever the setting is turned on. This is incorrect since the user has the freedom to turn on location of their device even after the setting is turned on.

Ditto on the location permission.

It is prudent to verify that location permission is available and location is turned on whenever the setting is true.

We're presuming the device location would be turned on whenever the setting is turned on.

We've already prompted the user to turn it on, redirected to the Settings too.

The world is never ideal when it comes to Android. The user can have the setting enabled and then later on turn off location on their device. This is a normal case which we should be wary of. That's precisely why the Nearby feature asks user to turn on location when it is turned off.

The same applies for the location permission (especially on Android 11 and above where permissions are auto-removed for unused apps). So, it is the ideal case to ensure we have location permission and if its unavailable request for the same.

A user who wants pictures to be geo-tagged will be turning location on for the camera too but I see it was a bad assumption. Thank you for explaining it! :)

RitikaPahwa4444 · 2023-07-05T06:53:07Z

I'm being shown the permission request dialog when the location turned on dialog should be shown

I don't think there's anything wrong with this flow. You're not turning on device location in step 6, which means the app won't be able to retrieve location despite having the required permission granted. This also goes for:

Note that the app already has location permission

About the first upload location loss problem now:

I could have automated the flow after returning from the Settings as well. But it seemed okay to make the user try tapping the camera icon again on returning. This allowed me to introduce some delay. Since this also shows a notification in the status bar, it is quite possible for the user to wait for the GPS to turn on before tapping again. This did not appear very intuitive to me in case location is turned on in the Settings already and user just has to tap on "Allow" for ACCESS_FINE_LOCATION permission within the app.

Showing the dialog for the first time will not guarantee that it won't happen in subsequent uploads too. I have explained how it usually does not happen here.

sivaraam · 2023-07-05T19:00:23Z

I don't think there's anything wrong with this flow. You're not turning on device location in step 6, which means the app won't be able to retrieve location despite having the required permission granted. This also goes for:

When the location is turned off, the app should be taking me to the location setting screen. The problem is it's taking me to the app's setting screen instead which is incorrect. Try repeating the steps yourself manually. You'll likely understand the problem easily.

Showing the dialog for the first time will not guarantee that it won't happen in subsequent uploads too. I have explained how it usually does not happen #5249 (comment).

In that case, can we not educate the users that the location may not get recorded in some cases since GPS wouldn't have initialized when the app records the location? We could suggest them to retry in case they observe the location to be missing.

We would need a more reliable solution that that. That could be explored and worked upon separately too. Kindly open an issue for the same 👍🏼

RitikaPahwa4444 · 2023-07-06T05:47:57Z

Try repeating the steps yourself manually. You'll likely understand the problem easily.

I'm sorry, I'm still not able to relate🥲

location_settings.mp4

This is how the app is handling it in Explore too. Sivaraam, are you facing the same problem there as well?

RitikaPahwa4444 · 2023-07-06T05:59:01Z

We would need a more reliable solution that that. That could be explored and worked upon separately too.

This happens with the default camera apps as well. They do not even mention about this. I think the only way out is to educate the users.

nicolas-raoul

Thanks!
Using this branch:

With Save location enabled in the stock Pixel 6 Android camera's settings, in-app pictures have location, and everything works fine (like before).

With Save location disabled in the stock Pixel 6 Android camera's settings, in-app pictures do not have location, and I get the No location found popup, always. Is that expected?

RitikaPahwa4444 · 2023-07-27T13:55:55Z

Did you choose 'Allow' in the first run? If yes, then it could be due to the GPS initialisation delay.

nicolas-raoul · 2023-07-27T14:07:35Z

Somehow I did not see that popup when I first used this branch. I might have missed it, not 100% sure.
So I reset the app's storage, got the popup, allowed, and now it is getting the right location even with camera's Save location disabled. :-)

The code looks correct too (Javadoc could be improved in some places but that can be done later), so I would be OK with merging if you are sure the popup will appear for all users.

RitikaPahwa4444 · 2023-07-27T14:16:52Z

Javadoc could be improved in some places but that can be done later

Noted✅

if you are sure the popup will appear for all users.

It is a one-time dialog, and will always appear on the first use.

nicolas-raoul

It works fine for me under different circumstances, apart from the final "File not found" which I believe is a different issue. Happy to merge if @sivaraam is OK.

RitikaPahwa4444 · 2023-07-28T05:41:30Z

@nicolas-raoul, since there's already a pull request for issue #5247, is it okay if I remove the code I'd commented out earlier?

nicolas-raoul · 2023-07-28T06:10:41Z

Yes, thanks! :-)

nicolas-raoul

I might have found a bug:

Using the custom picker, select a picture that has no EXIF location
The upload wizard seems to think that this picture was taken at the park I am in right now. Note: I have never used in-app camera at that park.

I am pretty sure this does not happen in master, I can try if you want.

RitikaPahwa4444 · 2023-07-29T07:43:11Z

Seems like a major blunder. I'm sorry for breaking the code😔 Looking into this.

…cation in normal and custom selector

nicolas-raoul

Thanks for fixing the above, it works! 🙂

Unfortunately it seems that now all pictures uploaded from normal and custom picker have their location removed...

RitikaPahwa4444 · 2023-07-31T05:03:57Z

Location is still getting recognised for me if present in the EXIF. Moreover, it is always the EXIF that's checked first. Is there anything that I'm missing out?

sivaraam · 2023-08-01T07:38:04Z

Just wanted to share what I observe. Uploading using the normal / custom picker seems to work as usual for me. While using the in-app camera, the usual flow seems to work fine. I just noticed two incorrect flows.

Flow 1: When the "Record location ..." preference is turned on and the location permission is denied for the app, it is not possible to capture an image using the in-app camera option itself. Tapping on it just results in nothing happening.

Flow 2: The same thing happens when "Record location ..." is turned on and location (GPS) is turned off in the device.

Possible resolution: In the above cases, it would be better to show a toast that location wouldn't be recorded due to lack of location permission and let the user continue with usual flow anyway.

sivaraam

Thanks for the change, Ritika! I've checked that the toast shows up properly when the GPS (i.e., location) is turned off on the device. OTOH, when the location permission is revoked for the app, I'm still unable to open the in-app camera.

Check the following screen recording:

Commons_location_permission_denied.mp4

Location captured when device location turned off

I noticed a rather curious case. When I have granted the location permission and location is turned on, the in-app shot is able to record location. Once that's done for an in-app shot and I then turn off device location, the app still seems to remember the previously recorded location and attach that to the upload. Any idea why this is happening?

If it seems like a rather edge case, let me know and we could handle it later. I'll open an issue to capture this in that case.

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

sivaraam · 2023-08-08T08:04:51Z

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

        <SwitchPreference
            android:defaultValue="true"
            android:key="openDocumentPhotoPickerPref"
            android:summary="@string/open_document_photo_picker_explanation"
            android:title="@string/open_document_photo_picker_title"/>
+
+        <SwitchPreference
+          android:key="inAppCameraLocationPref"
+          android:title="@string/in_app_camera_location_permission_title"
+          android:summary="@string/in_app_camera_location_switch_pref_summary"/>
+


We need to ensure that these two preferences aren't enabled for users who skipped login. For this the code in SettingsFragment needs to be tweaked. Link to corresponding part of the code: https://github.com/commons-app/apps-android-commons/blob/master/app/src/main/java/fr/free/nrw/commons/settings/SettingsFragment.java#L167-L175

Also, these could be moved above the customer author name settings such that they appear below "Save In-app shots" so that in-app shot related settings appear close together.

sivaraam · 2023-08-08T08:08:18Z

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

+  <string name="option_allow">Allow</string>
+  <string name="option_dismiss">Dismiss</string>
+  <string name="in_app_camera_needs_location">Please turn on location access from the Settings and try again. \n\nNote: The first upload may not have location due to limitations of device GPS.</string>
+  <string name="in_app_camera_location_permission_rationale">In-app camera needs location permission to attach it to your images in case location is not available in EXIF. Please allow the app to access your location and try again.\n\nNote: The first upload may not have location due to limitations of device GPS.</string>


We specify "first upload" but I believe that isn't specificially true. The location won't be captured whenever GPS initialization fails. So, it might be worth tweaking the phrase to something like:

Note: The upload may not have location if app is unable to retrieve location from device within a short interval.

Also, I believe it would be worth copying the note to in_app_camera_needs_location string too since the location request dialog won't be shown on devices that don't have permissions.

We specify "first upload" but I believe that isn't specificially true.

Was it not supposed to be done to keep things simple for the users? I'm sorry, I think I misinterpreted something discussed over Zulip. Thank you for suggesting a better phrase, I'll change it :)

RitikaPahwa4444 · 2023-08-08T08:38:59Z

Strange! Denying permission on the first run opens the camera, but this does not happen on subsequent runs. I reused the same code🤔 Looking into this, thank you for reviewing the PR again.

…gged-out users

nicolas-raoul

@RitikaPahwa4444 Would you mind creating GitHub issues for any remaining bugs/improvements? I am OK to have this merged as it is a huge improvement compared to the master branch.

RitikaPahwa4444 · 2023-08-30T10:32:31Z

I've created a new issue for the remaining bug/improvement.

nicolas-raoul · 2023-08-30T13:26:17Z

@sivaraam Can you merge if it looks good to you?

fix in-app camera location loss

02acabe

RitikaPahwa4444 marked this pull request as draft June 24, 2023 13:22

RitikaPahwa4444 added 5 commits June 24, 2023 19:28

fix failing unit tests

d6ac060

UploadMediaDetailFragmentUnitTest: modify testOnActivityResultAddLoca…

655ff4c

…tionDialog to have null location

reintroduce removed variable

a8e0c54

enable prePopulateCategoriesAndDepictionsBy for current user location

7208018

add relevant comment and fix failing test

3c0339b

nicolas-raoul requested changes Jun 25, 2023

View reviewed changes

modify dialog and disable location tag redaction from EXIF

5f9ff36

nicolas-raoul approved these changes Jun 29, 2023

View reviewed changes

RitikaPahwa4444 marked this pull request as ready for review June 29, 2023 04:40

RitikaPahwa4444 changed the title ~~[WIP] 5196: Fix in-app camera location loss~~ 5196: Fix in-app camera location loss Jun 29, 2023

sivaraam reviewed Jun 29, 2023

View reviewed changes

kartikaykaushik14 reviewed Jun 29, 2023

View reviewed changes

RitikaPahwa4444 added 2 commits July 4, 2023 19:40

modify in-app camera dialog flow and change location to inAppPictureL…

e72be00

…ocation

change location to inAppPictureLocation

74f8c13

sivaraam requested changes Jul 4, 2023

View reviewed changes

This was referenced Jul 7, 2023

In-app camera fails to record location information sometimes #5254

Open

Requesting location permission popup opens app's settings instead of location settings #5255

Closed

RitikaPahwa4444 and others added 2 commits July 26, 2023 22:33

preferences.xml: remove redundant default value

5f873d1

inform users about location loss happening for first upload

f0dd7d2

nicolas-raoul reviewed Jul 27, 2023

View reviewed changes

nicolas-raoul approved these changes Jul 28, 2023

View reviewed changes

RitikaPahwa4444 and others added 2 commits July 28, 2023 13:40

Merge branch 'commons-app:master' into in_app_camera_location_loss

5837b8b

FileProcessor.kt: remove commented-out code

a8783f6

nicolas-raoul requested changes Jul 29, 2023

View reviewed changes

prevent user location from getting attached to images with no EXIF lo…

96c7b4d

…cation in normal and custom selector

RitikaPahwa4444 requested a review from nicolas-raoul July 30, 2023 13:09

nicolas-raoul requested changes Jul 31, 2023

View reviewed changes

handle onPermissionDenied for location permission

4417140

sivaraam reviewed Aug 8, 2023

View reviewed changes

RitikaPahwa4444 added 3 commits August 16, 2023 22:36

remove last location when the user turns the GPS off

28618ce

disable photo picker and in app camera preferences in settings for lo…

efbd199

…gged-out users

remove debug statements and add toast inside runnables

0a2a10b

nicolas-raoul approved these changes Aug 30, 2023

View reviewed changes

RitikaPahwa4444 mentioned this pull request Aug 30, 2023

In-app camera does not open when location permission is revoked #5273

Closed

nicolas-raoul merged commit 5073ca0 into commons-app:master Sep 1, 2023
1 check passed

mnalis mentioned this pull request Apr 14, 2024

Mention location permission doesn't affect picture location #4741

Closed

		if (!(PermissionUtils.hasPermission(activity, Manifest.permission.ACCESS_FINE_LOCATION)
		&& isLocationAccessToAppsTurnedOn())) {

5196: Fix in-app camera location loss #5249

5196: Fix in-app camera location loss #5249

Conversation

RitikaPahwa4444 commented Jun 24, 2023

nicolas-raoul left a comment

Choose a reason for hiding this comment

sivaraam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RitikaPahwa4444 Jun 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kartikaykaushik14 Jun 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kartikaykaushik14 left a comment

Choose a reason for hiding this comment

RitikaPahwa4444 commented Jun 30, 2023

kartikaykaushik14 commented Jul 3, 2023 • edited Loading

nicolas-raoul commented Jul 3, 2023

RitikaPahwa4444 commented Jul 4, 2023

nicolas-raoul commented Jul 4, 2023

sivaraam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sivaraam Jul 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RitikaPahwa4444 commented Jul 5, 2023

sivaraam commented Jul 5, 2023

RitikaPahwa4444 commented Jul 6, 2023

RitikaPahwa4444 commented Jul 6, 2023

nicolas-raoul left a comment • edited Loading

Choose a reason for hiding this comment

RitikaPahwa4444 commented Jul 27, 2023

nicolas-raoul commented Jul 27, 2023

RitikaPahwa4444 commented Jul 27, 2023 • edited Loading

nicolas-raoul left a comment

Choose a reason for hiding this comment

RitikaPahwa4444 commented Jul 28, 2023

nicolas-raoul commented Jul 28, 2023

nicolas-raoul left a comment

Choose a reason for hiding this comment

RitikaPahwa4444 commented Jul 29, 2023

nicolas-raoul left a comment

Choose a reason for hiding this comment

RitikaPahwa4444 commented Jul 31, 2023

sivaraam commented Aug 1, 2023 • edited Loading

sivaraam left a comment • edited Loading

Choose a reason for hiding this comment

Location captured when device location turned off

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RitikaPahwa4444 Aug 8, 2023 • edited Loading

Choose a reason for hiding this comment

RitikaPahwa4444 commented Aug 8, 2023

nicolas-raoul left a comment

Choose a reason for hiding this comment

RitikaPahwa4444 Jun 30, 2023 •

edited

Loading

kartikaykaushik14 Jun 29, 2023 •

edited

Loading

kartikaykaushik14 commented Jul 3, 2023 •

edited

Loading

sivaraam Jul 9, 2023 •

edited

Loading

nicolas-raoul left a comment •

edited

Loading

RitikaPahwa4444 commented Jul 27, 2023 •

edited

Loading

sivaraam commented Aug 1, 2023 •

edited

Loading

sivaraam left a comment •

edited

Loading

RitikaPahwa4444 Aug 8, 2023 •

edited

Loading