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

remove read/write storage permissions from filepicker/mediapicker on Android #1968

Merged
merged 2 commits into from Apr 4, 2022

Conversation

jamesmontemagno
Copy link
Collaborator

Description of Change

Due to changes in targeting Android 11 the read/write permissions are super blocked. We dont' need them though as the API doesn't require them

Bugs Fixed

Provide links to issues here. Ensure that a GitHub issue was created for your feature or bug fix before sending PR.

API Changes

None.

Behavioral Changes

No longer require permissions.

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard
  • Updated documentation (see walkthrough)

@jamesmontemagno jamesmontemagno added android This issue impacts Android awaiting-review This PR needs to have a set of eyes on it blocker This is a blocker that must be fixed before shipping the next update. labels Feb 14, 2022
@jamesmontemagno
Copy link
Collaborator Author

Will need this one reviewed and then ported to .NET MAUI (i will open the PR). I think it is valid. Would be good to have some eyes on it.

Comment on lines -14 to -16
// we only need the permission when accessing the file, but it's more natural
// to ask the user first, then show the picker.
await Permissions.EnsureGrantedAsync<Permissions.StorageRead>();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too familiar with this area, but would this permission check still be needed for Android versions prior to 30? ie. should there be a version check here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what Google is reporting when people submit apps is that they will reject them even if they have the setting in there. I tested it out on older and newer devices and it seemed to work

TanayParikh
TanayParikh previously approved these changes Mar 9, 2022
@@ -6,11 +6,9 @@
<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />
<uses-permission android:name="android.permission.BATTERY_STATS" />
<uses-permission android:name="android.permission.CAMERA" />
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" />
<uses-permission android:name="android.permission.FLASHLIGHT" />
<uses-permission android:name="android.permission.FLASHLIGHT" />

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<uses-permission android:name="android.permission.FLASHLIGHT" />
<uses-permission android:name="android.permission.FLASHLIGHT" />

@TanayParikh
Copy link

Is this ready to merge? Would like to consume this in MAUI/Blazor Hybrid 😄

@TanayParikh
Copy link

@mattleibow any objections in merging this in?

@jamesmontemagno
Copy link
Collaborator Author

@Redth are we able to get this in and push somethign out @jfversluis as well cc

@jfversluis
Copy link
Member

I'll have a look (my) tomorrow. Let me know @jamesmontemagno if you have any more on the list that you think might be ready to go. I'll fix things up tomorrow and see if I can do a release early next week

@jfversluis
Copy link
Member

@rmarinho seems build is failing after the last changes you made to the pipeline any ideas?

@jfversluis
Copy link
Member

iOS build is broken and being worked on, but since this is Android only, this seems good to me!

@jfversluis jfversluis merged commit 162b072 into main Apr 4, 2022
@jfversluis jfversluis deleted the gh-1961 branch April 4, 2022 12:23
@TanayParikh
Copy link

@jfversluis how does this make its way into MAUI Essentials? I need to validate dotnet/maui#2678 after that's there.

@jfversluis
Copy link
Member

This should be already in there: dotnet/maui#4697

@MitchBomcanhao
Copy link

@jamesmontemagno @jfversluis isn't this still broken because the write permission is still asked for when you're taking a photo?
see #2037

on MediaPicker.android.cs
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android This issue impacts Android awaiting-review This PR needs to have a set of eyes on it blocker This is a blocker that must be fixed before shipping the next update.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants