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

Make MediaPicker capture methods work in Android 13+ #12766

Merged
merged 1 commit into from
Jan 26, 2023
Merged

Make MediaPicker capture methods work in Android 13+ #12766

merged 1 commit into from
Jan 26, 2023

Conversation

Ghostbird
Copy link
Contributor

@Ghostbird Ghostbird commented Jan 19, 2023

Description of Change

In Android 13+ android.permission.WRITE_EXTERNAL_STORAGE no longer exists, and requesting it always returns Denied. AFAIK it is not needed for the media capture in this Android version either, though I may be wrong.
This PR is created to make at least some progress towards solving #11275. Because currently there's no way to get the MediaPicker.Capture functions working on Android 13.

My apologies, this is a bit of a cruddy PR, but since the issue has been open since November, and it seems no-one has made progress on it, I'm trying. If this PR is unsuitable, I hope it at least gets some attention to the issue. It is really annoying that MAUI builds Android API 33 by default, but the current MediaPicker capture code can never work on Android API 33.

I looked at Permissions.android.cs and I think the storage read and write permission there are overdue a rewrite. The code there is 100% incompatible with Android 13 / API 33 which MAUI is meant to support. However, I don't know enough about the code base to make such rigorous changes, so I hope someone will look at that soon.

Issues Fixed

@ghost ghost added the community ✨ Community Contribution label Jan 19, 2023
@ghost
Copy link

ghost commented Jan 19, 2023

Hey there @Ghostbird! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@Ghostbird Ghostbird changed the title Make MediaPicker work in Android 13+ Make MediaPicker capture methods work in Android 13+ Jan 19, 2023
@Eilon Eilon added the area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info label Jan 19, 2023
@jfversluis
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@mattleibow
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@mattleibow
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@mattleibow
Copy link
Member

Thanks for this PR, just doing some tests now as permissions on android is ... interesting. And yeah, we need to investigate these permissions. We are doing several things different since we first wrote this code. I see now we are passing in locations that the media should be stored, and in my testing some OS versions did not even need any read/write permissions. But for now to fix this, this PR is geat!

@Ghostbird
Copy link
Contributor Author

@mattleibow What do I need to do to get this merged?

mattleibow
mattleibow previously approved these changes Jan 24, 2023
@mattleibow
Copy link
Member

Nothing more. Just want to get another review on this just in case. Android permissions are always a bit sketchy, but I will get this merged hopefully today.

@mattleibow mattleibow dismissed their stale review January 24, 2023 16:45

try again

@mattleibow mattleibow self-requested a review January 24, 2023 16:45
@Ghostbird
Copy link
Contributor Author

I still can't merge. I need 1 approving review.

@Ghostbird
Copy link
Contributor Author

@mattleibow Should I maybe rebase this on main, squash and force push it? I think you've added some merge commits that might mean that you're now an author of this PR and you're not allowed to approve this.

In Android 13+ android.permission.WRITE_EXTERNAL_STORAGE no longer exists, and requesting it always returns `Denied`.
AFAIK it is not needed for the media capture in this Android version either, though I may be wrong.
@Ghostbird
Copy link
Contributor Author

Ghostbird commented Jan 25, 2023

I think I messed something up, and forgot a using.

Never mind, it looks all right after all. I haven't actually managed to set-up my environment to successfully build MAUI, so I'm flying half-blind here.

@mattleibow
Copy link
Member

/azp run

@mattleibow
Copy link
Member

looks like this is more green :)

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@mattleibow mattleibow enabled auto-merge (squash) January 25, 2023 23:52
@mattleibow mattleibow merged commit 197f203 into dotnet:main Jan 26, 2023
@mattleibow
Copy link
Member

/backport to net7.0

@github-actions
Copy link
Contributor

Started backporting to net7.0: https://github.com/dotnet/maui/actions/runs/4011569003

@SamazoOo
Copy link

When will this fix be available via nuget packages?

@JacobAtchley
Copy link

@Ghostbird I am currently working on updating an old Xamarin Native app to MAUI and ran into this issue.

Should we instead check for version 29 when asking for WRITE_EXTERNAL_STORAGE permissions?

According to the android documentation, you do not need to request these permissions when writing to the application cache directories which the media picker does use.

https://developer.android.com/training/data-storage

@Ghostbird
Copy link
Contributor Author

I only did the very minimum fix required to make this work. Note that it just barely missed the latest service release, so it'll be in next month's. At the moment I simply build using API 32, which is a workaround detailed in the original issue #11275

@github-actions github-actions bot locked and limited conversation to collaborators Dec 13, 2023
@samhouts samhouts added the fixed-in-8.0.0-preview.1.7762 Look for this fix in 8.0.0-preview.1.7762! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info community ✨ Community Contribution fixed-in-8.0.0-preview.1.7762 Look for this fix in 8.0.0-preview.1.7762! platform/android 🤖
Projects
None yet
7 participants