Skip to content
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.

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

Fix Crash in LocationPickerActivity when device configuration is changed #5500

Conversation

shashankiitbhu
Copy link
Contributor

Description (required)

Fixes #5474

What changes did you make and why?
Identified and fixed a critical issue in the open-source project related to the LocationPickerActivity.
The crash occurred when the device configuration changed, specifically during UI mode switches (dark to light or light to dark mode)

Investigated the crash root cause, which was tied to how the activity handled UI mode changes.

Tests performed (required)

Tested 4.2.1-debug on Xiaomi 11 Lite NE with API level 33

Screenshots (for UI changes only)

WhatsApp.Video.2024-01-31.at.12.10.27.PM.mp4

@shashankiitbhu
Copy link
Contributor Author

@nicolas-raoul Please Review this PR

@shashankiitbhu
Copy link
Contributor Author

Hi @nicolas-raoul , is there any change required in this PR?

@ShashwatKedia
Copy link
Contributor

ShashwatKedia commented Mar 4, 2024

Student Review

Screencast of Before:
https://drive.google.com/file/d/1w2wtnYh1ax3mIMfRStepET9Zl7efmLNl/view?usp=share_link

Screencast of After:
https://drive.google.com/file/d/1w9F14a22OPhHsAIgxmeHvvEa-XsV1KjL/view?usp=share_link

The crash does not happen after merging this PR, but some bugs still remain:

  1. If I move the marker to some other position and then change the configuration, then that change is lost (can be seen in the 'After Screencast' ).
  2. I started the process of uploading a new image and tried to add a location to it using the LocationPicker, but the app crashed Screencast when I pressed the tick icon. This does not happen if I change/update co-ordinates of an already uploaded image. Here's the Logcat for the same:

FATAL EXCEPTION: main Process: fr.free.nrw.commons, PID: 1201 java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String fr.free.nrw.commons.Media.getFilename()' on a null object reference at fr.free.nrw.commons.coordinates.CoordinateEditHelper.addCoordinates(CoordinateEditHelper.java:84) at fr.free.nrw.commons.coordinates.CoordinateEditHelper.makeCoordinatesEdit(CoordinateEditHelper.java:63) at fr.free.nrw.commons.LocationPicker.LocationPickerActivity.updateCoordinates(LocationPickerActivity.java:356) at fr.free.nrw.commons.LocationPicker.LocationPickerActivity.placeSelected(LocationPickerActivity.java:342) at fr.free.nrw.commons.LocationPicker.LocationPickerActivity.lambda$addPlaceSelectedButton$4$fr-free-nrw-commons-LocationPicker-LocationPickerActivity(LocationPickerActivity.java:328) at fr.free.nrw.commons.LocationPicker.LocationPickerActivity$$ExternalSyntheticLambda3.onClick(Unknown Source:2) at android.view.View.performClick(View.java:7488)

It seems to be happening due to media being null, while being passed in the newly created updateCoordinates function.

@shashankiitbhu shashankiitbhu marked this pull request as draft March 9, 2024 07:49
@shashankiitbhu
Copy link
Contributor Author

  1. If I move the marker to some other position and then change the configuration, then that change is lost (can be seen in the 'After Screencast' ).

This one is not a bug as you haven't selected any location , you are just hovering over it so it is not saved anywhere.

@shashankiitbhu
Copy link
Contributor Author

@ShashwatKedia Thanks for reporting the Null Pointer Exception, fixing it now

@shashankiitbhu shashankiitbhu marked this pull request as ready for review March 9, 2024 08:26
@shashankiitbhu
Copy link
Contributor Author

@nicolas-raoul @RitikaPahwa4444 can you review and merge this , thanks

Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

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

Tested, fixes the crash.
Thanks!

@nicolas-raoul nicolas-raoul merged commit fec6dba into commons-app:main Mar 17, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: App Crashes when Switched Between Dark/Light Mode in Location Picker Activity
3 participants