-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 security exception (Nearby places not loading after permissions granted) #1440
Fix security exception (Nearby places not loading after permissions granted) #1440
Conversation
… into 2.7.x-release-fork
… into 2.7.x-release-fork
… into 2.7.x-release-fork
…s-android-commons into 2.7.x-release-fork
… into 2.7.x-release-fork
… into 2.7.x-release-fork
… into 2.7.x-release-fork
Codecov Report
@@ Coverage Diff @@
## 2.7.x-release #1440 +/- ##
================================================
- Coverage 3.3% 3.28% -0.02%
================================================
Files 128 128
Lines 6847 6870 +23
Branches 668 673 +5
================================================
Hits 226 226
- Misses 6606 6629 +23
Partials 15 15
Continue to review full report at Codecov.
|
The current code seems to fix the map problem, tested on API 24 real device. The problem is, I don't think it's a good solution, it's just a hack I made to figure out what the problem was. For some reason Ideally we should find out why it is null though. The hack:
Please don't merge yet, even if we do decide to go with this, I need to tidy up the code. :) |
@misaochan it would be great if you can share a screenshot to display what blank map means. If it only happens on emulator, some of my emulator devices have this problem too. But I am sure it is related with the emulator. |
The issue happened on my real device. :) But it should now be fixed by the hack - as I mentioned, it was caused by curLatLang defaulting to null in the bundle that was passed to NearbyMapFragment. I can go back to an earlier commit and take a SS there if needed, however it was literally a screen with "Nearby Places" on the top and nothing else. |
According to what I read and since I know the related code base, I can say that thing we are doing can be a little risky. But will investigate further to be sure. Update after testing: It worked on my both tests by the way. |
Thanks @neslihanturan . Could you please try and see if this fixes #1438 for you as well? If it does , then we can kill two birds with one stone here. Since this works for both of us, I'll tidy up my code and leave a TODO for finding a better fix. |
I am still able to reproduce the bug on this branch too:/ Maybe you are missing a step |
How odd. It happens to me sometimes but not at other times. When it happens, I can never access Directions from the map at ALL (although I can from the list). The steps don't matter - it happens regardless of whether I have opened the list beforehand, or changed activity, or not. When it doesn't happen, I can always access Directions from the map, and again the steps don't matter. Okay, so this PR clearly isn't a fix for that problem. I guess we can kill just one bird with this stone and then look for the other bird. ;) |
I think this PR is ready to merge? |
@neslihanturan I just need to do a bit more tidying and testing. Will ping you when it is, thanks. :) |
@neslihanturan Okay, this should be ready once Travis is done. :) |
Work in progress
I have been attempting to fix #1413 and have partially succeeded. Now, after granting permissions, the Nearby points load. I think the issue with the previous attempts was that when permissions were granted, we did not check for the last known location and so
refreshView()
returned prematurely with the below code:However, in this PR, the map is not created and a blank screen is shown, even though the points exist. If I tap on the list, the list displays fine, with the appropriate points. @neslihanturan do you think you could help with this?
Logs: