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 of #1581 Crash after granting permission. #1933

Closed
wants to merge 2 commits into from

Conversation

hoplite390
Copy link
Contributor

Title:

location permission fix for 2.8-release

Fixes #1581 Crash after granting permission
Fixes bug of GPS enable dialog not dismissed

Description:

area: NearbyActivity, LocationServiceManager

Null check in LatLing getLKL method with replacing content of return with calling method that already exists "getLastLocation()".

  • App will not crash 1st time granting permission.

Fixing bug in NearbyActivity. Dialog (enable gps) was not dismissed in activity after pressing positive button and turning on gps.

  • Dialog (enable gps) will disappear and if gps is disabled again it will reappear.

{In case it is usable please assign me to #1581 and mark as resolved, so that i can show it to my teacher in class as proof. Thank you.}

Tested:

on API 27 & Nexus 5X (emulator), with {BetaDebug}

Hotfix for 2.8-release. Reintroduce lines from master to prevent crash: null chceck in LatLing getLKL method.
Fixing bug in NearbyActivity. Dialog was not dismissed in activity after pressing positive button and turning on GPS.
…od getLastLocation. Redundant variable Location lastKL replaced by lastLocation that was already defined.
@codecov-io
Copy link

Codecov Report

Merging #1933 into 2.8-release will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@              Coverage Diff               @@
##           2.8-release   #1933      +/-   ##
==============================================
- Coverage         3.72%   3.72%   -0.01%     
==============================================
  Files              190     190              
  Lines             9555    9556       +1     
  Branches           852     852              
==============================================
  Hits               356     356              
- Misses            9174    9175       +1     
  Partials            25      25
Impacted Files Coverage Δ
...e/nrw/commons/location/LocationServiceManager.java 0% <0%> (ø) ⬆️
...ava/fr/free/nrw/commons/nearby/NearbyActivity.java 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b708db...57f2314. Read the comment docs.

@hoplite390
Copy link
Contributor Author

Not sure why codacy could not re-analyse this pull req.

@domdomegg
Copy link
Member

I can't seem to reproduce #1581 with master so am unable to test whether this actually fixes the issue. Could you suggest an exact emulator configuration, ideally with super specific steps.

I've tried:

  • Galaxy Nexus API 15
  • Galaxy Nexus API 27
  • Galaxy Nexus API 28
  • Real device: Samsung Galaxy S6 API 25

@hoplite390
Copy link
Contributor Author

Sorry for your time spent. I left comment in #1917 and probably should have type there reference to my merge request as well. This was intended for release version as "hot fix" because app used to crash with this error and google store version. Problem was solved by someone else with null check in master. Meanwhile 2.8 and master was merged so this merge request lost its meaning. Other changes were more about 1 less declaration and reusing (small) existing method in another which is not mandatory for this fix. Thank you @domdomegg .

@domdomegg
Copy link
Member

No problem ;)

Glad the bug is fixed

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.

3 participants