search

mozilla-mobile / reference-browser

A full-featured browser reference implementation using Mozilla Android Components.
Mozilla Public License 2.0
563 stars 160 forks source link

Remove deprecated requestPermissions. #2788

Closed mcarare closed 2 months ago

mcarare commented 2 months ago

Note that the onPermissionsResult method implementation does not even use the params passed and instead just checks again for the permission/ permissions being granted.

Pull Request checklist

rvandermeulen commented 2 months ago

I don't think I'm the best reviewer of this, but thanks for filing! For context to those coming in to look at this, this is addressing a build error we're seeing when testing out the Kotlin 2.0.0 RC1 build.

Relatedly, do we need to get a bug on file for doing similar on the AC side, @mcarare?

mcarare commented 2 months ago

For AC and Fenix/ Focus I would rather invest the effort in creating a new PermissionsFeature that adheres to the new API, instead of working on adapting the method calls to use the new API( like we did here, but required a smaller effort).

jonalmeida commented 2 months ago

For AC and Fenix/ Focus I would rather invest the effort in creating a new PermissionsFeature that adheres to the new API, instead of working on adapting the method calls to use the new API( like we did here, but required a smaller effort).

I would like to subscribe to this feature design. 🙂

mcarare commented 2 months ago

This seems fine, but I don't understand why we need to hold this register method in a property and not in-line it where we need it. It seems to be what the code docs show on the Android website though.

registerForActivityResult has to be called in the initialization part of the Fragment/ Activity. It will throw an IllegalStateException if called later.