fix(location): open map with approximate or one-time location permission - #6786
rapterjet2004 wants to merge 1 commit into
Conversation
Share location only opened the map when precise location was granted. The chat never requested the runtime permission, so "Ask every time" could never be granted, and both the chat check and the location picker required ACCESS_FINE_LOCATION. Now the permission is requested from the chat (fine and coarse, so the user can choose on API 31+) and either grant is accepted. The location services check uses LocationManagerCompat.isLocationEnabled, and getLastKnownLocation is guarded against SecurityException. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: rapterjet2004 <juliuslinus1@gmail.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughChatActivity now checks whether location services are enabled and requests fine and coarse location permissions when needed. It opens the location picker if either permission is granted. The picker also accepts either permission, and its last-known-location lookup catches SecurityException, logs a warning, and returns null. Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🟡 Moderate · up to Users who grant approximate or one-time location can now open the location-sharing map. Users who deny location permission, or who have device location turned off, still cannot open the map to pick a place by hand, which the linked issue asks for. Address these cases or explicitly accept them before merging. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Approximate or one-time location permission can now open the picker. The reviewed flow still requires a location grant and a separate action to share coordinates. No introduced security vulnerability was established, but permission-revocation behavior remains uncertain. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation The change meets the approximate, one-time, and coarse-permission cases in [ Resolution Update the no-grant path for [
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 471577c4-f450-4006-9dfe-e91c2cb432d5
📒 Files selected for processing (3)
app/src/main/java/com/nextcloud/talk/chat/ChatActivity.ktapp/src/main/java/com/nextcloud/talk/location/components/LocationPickerScreen.ktapp/src/main/java/com/nextcloud/talk/utils/permissions/PlatformPermissionUtilImpl.kt
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
| } else if (requestCode == REQUEST_LOCATION_PERMISSION) { | ||
| if (grantResults.any { it == PackageManager.PERMISSION_GRANTED }) { | ||
| openLocationPicker() | ||
| } else if (!shouldShowRequestPermissionRationale(Manifest.permission.ACCESS_FINE_LOCATION)) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Open the picker after the user denies location permission.
If the user selects “Don’t allow,” neither permission is granted. This branch shows denial UI and never calls openLocationPicker(). The linked issue requires manual map selection without location permission. Open the picker after denial and treat the permission as necessary only for the optional current-position feature. Android also recommends allowing users to continue without granting a permission when the feature can work without it. (developer.android.com)
| val isGpsEnabled = locationManager.isProviderEnabled(LocationManager.GPS_PROVIDER) | ||
|
|
||
| if (!isGpsEnabled) { | ||
| if (!LocationManagerCompat.isLocationEnabled(locationManager)) { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Open the picker when location services are disabled.
If the user disables device location services, this branch shows a settings dialog instead of opening the picker. The user cannot select a location manually, although the map does not require a device location fix. Open the picker in this case. Keep the settings action available for users who want their current position. LocationManagerCompat.isLocationEnabled reports whether device location is enabled. (developer.android.com)
📱 QA build
The QA build installs alongside a released Nextcloud app, so you can keep Downloading the file requires a GitHub account, so open this link on the |
1 participant
Share location only opened the map when precise location was granted. The chat never requested the runtime permission, so "Ask every time" could never be granted, and both the chat check and the location picker required
ACCESS_FINE_LOCATION.Now the permission is requested from the chat (fine and coarse, so the user can choose on API 31+) and either grant is accepted. The location services check uses
LocationManagerCompat.isLocationEnabled, andgetLastKnownLocationis guarded againstSecurityException.Assisted-by: Claude Code:claude-opus-5-5
🖼️ Screenshots
🏁 Checklist
/backport to stable-xx.x🤖 AI (if applicable)