App crashes on pressing the "My Location" icon in the upload wizard

[RitikaPahwa4444]

Summary

The app crashes on pressing the "My Location" button in the upload wizard. The same icon shows a dialog requesting for location access in the Nearby section and in the Map fragment of the Explore section.

Steps to reproduce

1. Open the Contribution tab
2. Click on the upload from gallery icon (either regular or the custom selector)
3. Choose an image and click on the map icon
4. Click on the "My Location" icon present just above the green coloured floating icon

Expected behaviour

A dialog must appear requesting for location access.

Actual behaviour

The app crashes on clicking the icon.

Device name

Redmi 5A

Android version

8.1.0

Commons app version

master and prodDebug

Device logs

java.lang.NullPointerException: Attempt to invoke virtual method 'double android.location.Location.getLatitude()' on a null object reference
        at fr.free.nrw.commons.LocationPicker.LocationPickerActivity.getCenter(LocationPickerActivity.java:457)
        at fr.free.nrw.commons.LocationPicker.LocationPickerActivity.lambda$addCenterOnGPSButton$4$LocationPickerActivity(LocationPickerActivity.java:451)
        at fr.free.nrw.commons.LocationPicker.-$$Lambda$LocationPickerActivity$adF53ASiF4sbH3p2XVSpYljRmC0.onClick(Unknown Source:2)
        at android.view.View.performClick(View.java:6304)
        at android.view.View$PerformClick.run(View.java:24803)
        at android.os.Handler.handleCallback(Handler.java:794)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:176)
        at android.app.ActivityThread.main(ActivityThread.java:6651)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:547)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:824)
E/ACRA: ACRA caught a NullPointerException for fr.free.nrw.commons
    java.lang.NullPointerException: Attempt to invoke virtual method 'double android.location.Location.getLatitude()' on a null object reference
        at fr.free.nrw.commons.LocationPicker.LocationPickerActivity.getCenter(LocationPickerActivity.java:457)
        at fr.free.nrw.commons.LocationPicker.LocationPickerActivity.lambda$addCenterOnGPSButton$4$LocationPickerActivity(LocationPickerActivity.java:451)
        at fr.free.nrw.commons.LocationPicker.-$$Lambda$LocationPickerActivity$adF53ASiF4sbH3p2XVSpYljRmC0.onClick(Unknown Source:2)
        at android.view.View.performClick(View.java:6304)
        at android.view.View$PerformClick.run(View.java:24803)
        at android.os.Handler.handleCallback(Handler.java:794)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:176)
        at android.app.ActivityThread.main(ActivityThread.java:6651)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:547)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:824)

Screen-shots

No response

Would you like to work on the issue?

Yes

[nicolas-raoul]

Interestingly, it does not crash for me on Android 13. Thanks for finding the issue!

[nicolas-raoul]

Now I am getting the crash, with the same steps, with a picture that has no EXIF location.

[RitikaPahwa4444]

Thank you for pointing this out. I may have overlooked it since most of the images I tested did not have an EXIF location and I was getting consistent results for all of them.

[RitikaPahwa4444]

@nicolas-raoul, the location icon is used in a different sense in the upload wizard. Since the aim of using the location icon is to retrieve the location from the EXIF data, we may have to explain this behaviour to the user in case it is not available. Should we set it as the current location of the user and explain this behaviour as a pop-up or simply display a toast and ask him/her to set it manually?

[nicolas-raoul]

It seems to me that the button has the same meaning in all activities... all center the map on the user's current location" I believe. If not, would you mind detailing what activity has what button meaning? Thanks!

[RitikaPahwa4444]

I am sorry, I may not have interpreted this correctly but for Nearby and Explore, it says:

Permission required to display a list of nearby places

This would display the places nearest to the user. However, in case of uploads, the location in the EXIF data may be different from the current location of the user. Are we not focusing on the place where the picture was taken in the upload wizard?

[RitikaPahwa4444]

I tried adding an image with EXIF data from the drive you had shared in the comments section of one of the issues and clicking the "My Location" icon successfully points to Japan, whereas the user's current location is not Japan:

So, clicking the icon has a different meaning here as it is not pointing to my current location (India)

[RitikaPahwa4444]

Hi @nicolas-raoul, I just wanted to confirm the expected behaviour once. Should tapping the icon centre the map on the user's current location even when the image has EXIF location?

[nicolas-raoul]

The "target" shaped icon, when tapped, should center the map on the user's current location; at least in all of the app's map activities with this icon that I can think of: Nearby, the Upload Wizard, the media details location editor, the Explore map. Thanks! :-)

[sivaraam]

Unfortunately, I'm still observing this in some cases (at the least). So, re-opening the issue.

The following is the log for the crash I observe:

Crash log

```stacktrace java.lang.RuntimeException: Failure delivering result ResultInfo{who=@android:requestPermissions:, request=42, result=-1, data=Intent { act=android.content.pm.action.REQUEST_PERMISSIONS (has extras) }} to activity {fr.free.nrw.commons/com.karumi.dexter.DexterActivity}: java.lang.NullPointerException: Attempt to invoke virtual method 'double fr.free.nrw.commons.location.LatLng.getLatitude()' on a null object reference at android.app.ActivityThread.deliverResults(ActivityThread.java:5516) at android.app.ActivityThread.handleSendResult(ActivityThread.java:5555) at android.app.servertransaction.ActivityResultItem.execute(ActivityResultItem.java:54) at android.app.servertransaction.ActivityTransactionItem.execute(ActivityTransactionItem.java:45) at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135) at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95) at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2345) at android.os.Handler.dispatchMessage(Handler.java:106) at android.os.Looper.loopOnce(Looper.java:233) at android.os.Looper.loop(Looper.java:344) at android.app.ActivityThread.main(ActivityThread.java:8212) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:584) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1034) Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'double fr.free.nrw.commons.location.LatLng.getLatitude()' on a null object reference at fr.free.nrw.commons.LocationPicker.LocationPickerActivity$2.onLocationPermissionGranted(LocationPickerActivity.java:488) at fr.free.nrw.commons.location.LocationPermissionsHelper.lambda$requestForLocationAccess$0$LocationPermissionsHelper(LocationPermissionsHelper.java:65) at fr.free.nrw.commons.location.-$$Lambda$LocationPermissionsHelper$d2wvPL4xiASUOzfwrYH5KkU9-aM.run(Unknown Source:4) at fr.free.nrw.commons.utils.PermissionUtils$1.onPermissionsChecked(PermissionUtils.java:137) at com.karumi.dexter.MultiplePermissionListenerThreadDecorator$1.run(Unknown Source:8) at com.karumi.dexter.MainThread.execute(Unknown Source:6) at com.karumi.dexter.MultiplePermissionListenerThreadDecorator.onPermissionsChecked(Unknown Source:7) at com.karumi.dexter.DexterInstance.onPermissionsChecked(Unknown Source:57) at com.karumi.dexter.DexterInstance.updatePermissionsAsGranted(Unknown Source:26) at com.karumi.dexter.DexterInstance.onPermissionRequestGranted(Unknown Source:0) at com.karumi.dexter.Dexter.onPermissionsRequested(Unknown Source:4) at com.karumi.dexter.DexterActivity.onRequestPermissionsResult(Unknown Source:51) at android.app.Activity.dispatchRequestPermissionsResult(Activity.java:8618) at android.app.Activity.dispatchActivityResult(Activity.java:8475) at android.app.ActivityThread.deliverResults(ActivityThread.java:5509) ... 13 more ```

I think this happens when we try to get to the location immediately after we got the permission. I suppose we at the least handle this gracefully rather than letting the app crash? Like skipping the getLatitude() call when the object is null?

[RitikaPahwa4444]

Oops! Seems like I missed some part while copy-pasting and merging with main. Thank you for the logs, I'll fix it today.