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

Requesting unknown permission on Android fails, should default to indicate permission being granted #11782

Open
Robyt3 opened this issue Dec 30, 2024 · 2 comments
Milestone

Comments

@Robyt3
Copy link

Robyt3 commented Dec 30, 2024

SDL_AndroidRequestPermission/SDL_RequestAndroidPermission fails when requesting unknown permissions, e.g. when you request the notification permission on a platform before Android 13 where this permission did not exist. Best practice according to the documentation is to use ContextCompat and ActivityCompat to request permissions, e.g. in

Activity activity = (Activity)getContext();
if (activity.checkSelfPermission(permission) != PackageManager.PERMISSION_GRANTED) {
activity.requestPermissions(new String[]{permission}, requestCode);
} else {
nativePermissionResult(requestCode, true);
}

apply the patch

-        if (activity.checkSelfPermission(permission) != PackageManager.PERMISSION_GRANTED) {
-            activity.requestPermissions(new String[]{permission}, requestCode);
+        if (ContextCompat.checkSelfPermission(activity, permission) != PackageManager.PERMISSION_GRANTED) {
+            ActivityCompat.requestPermissions(activity, new String[]{permission}, requestCode);

That would result in SDL_AndroidRequestPermission returning a result indicating that the permission was granted for unknown permission IDs. This applies to both SDL2 and 3.

See also:

This would however also require adding the imports

import androidx.core.app.ActivityCompat;
import androidx.core.content.ContextCompat;

and Gradle dependency

android {
	dependencies {
		implementation 'androidx.core:core:1.13.1'
	}
}
@slouken
Copy link
Collaborator

slouken commented Dec 31, 2024

We don't currently have an androidx dependency. Is it possible to solve this another way?

@slouken slouken added this to the 3.x milestone Dec 31, 2024
@Robyt3
Copy link
Author

Robyt3 commented Dec 31, 2024

As a workaround we added a check in our project to not request the notification permission on API lower than 33, but that doesn't properly check if notifications are allowed by the notification manager.

It seems like using AndroidX is best practice for new Android projects to simplify supporting different API levels, but I understand if you don't want to add more dependencies to the existing project. I looked a bit through the project files and it doesn't seem like there are many use cases for AndroidX yet.

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

No branches or pull requests

2 participants