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.

Sign up for GitHub

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

Jump to bottom

Android 13 fix media permissions #731

Merged
merged 3 commits into from
Apr 6, 2023
Merged

Android 13 fix media permissions #731

merged 3 commits into from
Apr 6, 2023

Conversation

ravishanker
Copy link
Contributor

@ravishanker ravishanker commented Mar 30, 2023 •
edited
Loading

Updates media permissions for Android 13

Internal project thread: pbArwn-5IN-p2

Fixes: #18096

To test

Warning
This probably needs to wait for #18183 on media permission to be merged first.

  • Checkout this branch
  • Use this branch through WP (uncomment localStoriesAndroidPath in local-builds.gradle)
  • Try out adding a Story post (FAB > Story post) with an image and/or video from WP
  • Ensure, no issues or crashes

Note
Adding an image or video doesn't do anything contrary to expectations from demo app Loop in this repo

@ravishanker ravishanker added the [Type] Task Something that needs be done, but is not necessarily a feature/bugfix itself. label Mar 30, 2023
@ravishanker ravishanker requested a review from irfano March 30, 2023 06:43
@ravishanker ravishanker mentioned this pull request Mar 30, 2023
Closed
5 tasks
@ravishanker ravishanker added the [Status] Nor Ready for Merge PR not ready for merge yet label Mar 31, 2023
@irfano irfano mentioned this pull request Apr 3, 2023
Merged
3 tasks
@irfano
Copy link
Contributor

irfano commented Apr 3, 2023

👋🏻 @ravishanker ! I tested the PR on wordpress-mobile/WordPress-Android#18183, and it failed to post a story.
I am adding a patch that should fix the issue with story posting. Can you please test WPAndroid using the patch?
I think we should merge this PR first and then merge wordpress-mobile/WordPress-Android#18183.

Update_WRITE_EXTERNAL_STORAGE_permission_for_newer_APIs.patch

@ravishanker
Update permissions
e0862ba 
Updates storage permissions for Version R (30)
@ravishanker
Copy link
Contributor Author

👋🏻 @ravishanker ! I tested the PR on wordpress-mobile/WordPress-Android#18183, and it failed to post a story. I am adding a patch that should fix the issue with story posting. Can you please test WPAndroid using the patch? I think we should merge this PR first and then merge wordpress-mobile/WordPress-Android#18183.

Applied patch and tested with #18183, works like a charm 👍

@ravishanker ravishanker removed the [Status] Nor Ready for Merge PR not ready for merge yet label Apr 6, 2023
@ravishanker ravishanker self-assigned this Apr 6, 2023
irfano
irfano approved these changes Apr 6, 2023
View reviewed changes
Copy link
Contributor

@irfano irfano left a comment

Choose a reason for hiding this comment

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

Thank you for handling this and testing on WPAndroid. LGTM! 👍🏻

@irfano irfano merged commit 949ea6b into trunk Apr 6, 2023
@irfano irfano deleted the Android-13-Fix-media-permissions branch April 6, 2023 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@irfano irfano irfano approved these changes

Assignees

@ravishanker ravishanker

Labels
[Type] Task Something that needs be done, but is not necessarily a feature/bugfix itself.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Upgrade Stories-Android to Android 13
2 participants
@ravishanker @irfano