-
-
Notifications
You must be signed in to change notification settings - Fork 869
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 android bluetooth permission request above api version 30 #993
Conversation
The BLUETOOTH permission was removed in api version 30 in favor of BLUETOOTH_SCAN, BLUETOOTH_ADVERTISE and BLUETOOTH_CONNECT. This means that manifests targetting api version above 30 will not have this permission set. For these versions the manifest should be checked for the new permissions. This change ensures that the documented behavior of the bluetooth permission always returning `true` is true. Fixes issues Baseflow#884
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only one of these should remain. If both are kept this results in an error
Is there an update on this? |
List<String> names = PermissionUtils.getManifestNames(context, PermissionConstants.PERMISSION_GROUP_BLUETOOTH); | ||
boolean missingInManifest = names == null || names.isEmpty(); | ||
if (missingInManifest) { | ||
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have just spent some time with this library and getting to know its code and testing it on various Android devices, and here is the result of my research: this change (and PR) seems to be unnecessary, at least with version 10.4.2.
As you can see here https://github.com/Baseflow/flutter-permission-handler/blob/permission_handler_v10.4.2/permission_handler_android/android/src/main/java/com/baseflow/permissionhandler/PermissionManager.java#L322-L324 the checkBluetoothPermissionStatus
method edited here is used if the new permissions are checked on a device with API level < S (31) - in this case the library will simply fall back to checking for the old Permissions.BLUETOOTH
(code before this PR). If the current device is 31 or higher, the method will not be called and the permissions will be checked by the code starting here: https://github.com/Baseflow/flutter-permission-handler/blob/permission_handler_v10.4.2/permission_handler_android/android/src/main/java/com/baseflow/permissionhandler/PermissionManager.java#L327.
Similarly, requestPermissions
(not touched in this PR) works correctly - on pre-31 devices it will first determine the status of the permissions (by using checkBluetoothPermissionStatus
and the fallback logic as described above) and because Permissions.BLUETOOTH
is a 'normal' permission, as long as it is in the manifest it will be 'granted' (not having it in the manifest is an error), so requesting it is a no-op - it will not put the permission to permissionsToRequest
list. On 31 and later devices, it will correctly request the new permissions.
Which means - as long as the Flutter part of you application uses new permissions, it will all work even on pre-31 devices as the library is smart enough to fallback to the old permission if new ones are used on old devices. Tested with both a 33 and 23 device.
As a side note, irrelevant to this PR, the same logic applies to the new Permissions.POST_NOTIFICATIONS
introduced in API level 33 - if used on a pre-33 device, this library falls back to using the NotificationManagerCompat
for the check, and 33 and later check the permission - see https://github.com/Baseflow/flutter-permission-handler/blob/permission_handler_v10.4.2/permission_handler_android/android/src/main/java/com/baseflow/permissionhandler/PermissionManager.java#L479-L492.
I have a test app running on devices with API level 23 and 33 and I don't need any workarounds. I always use the new permissions, it works even on old devices (see my other rather lengthy comment) and the lib implements fallbacks. I think this PR should be closed as it is unnecessary. |
Agreed with @wujek-srujek, closing this PR |
The BLUETOOTH permission was removed in api version 30 in favor of BLUETOOTH_SCAN, BLUETOOTH_ADVERTISE and BLUETOOTH_CONNECT.
This means that manifests targetting api version above 30 will not have this permission set. For these versions the manifest should be checked for the new permissions.
This change ensures that the documented behavior of the bluetooth permission always returning
true
is true.Fixes issues #884
✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)
Bug fix for Android api versions above 30 requesting Bluetooth permissions.
Permission.bluetooth
return denied instead of the documented always allowed (https://pub.dev/documentation/permission_handler_platform_interface/latest/permission_handler_platform_interface/Permission/bluetooth-constant.html)🆕 What is the new behavior (if this is a feature change)?
This commit brings the code in line with the documented behavior
💥 Does this PR introduce a breaking change?
No.
🐛 Recommendations for testing
Test on api versions after 30 with a manifest that only includes one of the new permissions, and either no BLUETOOTH permission, or `
📝 Links to relevant issues/docs
🤔 Checklist before submitting
master
.