-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fix camera permission issue #18290
Fix camera permission issue #18290
Conversation
📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
|
Generated by 🚫 dangerJS |
Found 1 violations: The PR caused the following dependency changes:-\--- org.wordpress:utils:{strictly 3.6.0} -> 3.6.0
+\--- org.wordpress:utils:{strictly 3.6.1} -> 3.6.1
Please review and act accordingly
|
@ravishanker, could you please review this for the |
We are not, but it'd be great to prioritize its review so we can ship the changes with a new beta as soon as we can. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @irfano 👍
I've tested the implementation using a CI build on a Pixel 5 (Android 13) and worked as expected for me. The code changes also look good 🎉
ps. I bumped into an issue with stories but I was able to reproduce it on 22.1
and should not be related with the changes introduced here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works on my machine 👍🏻 . Except in emulators, for some reason it only works on Android 13 for me there but I don't expect folks to use the app from an emulator 😅
Thank you! I want to mention that it works on my emulators. It must be an issue related to your emulator. |
This fixes recording video for a post. All places using Camera permission now use
PermissionUtils
class from WordPress-Utils library.The issue was fixed on the WordPress-Utils PR, and this PR updates the WordPress-Utils library for the fix.
Internal discussion: pcdRpT-2lO-p2#comment-4158
To test:
All these test cases should be done on Android 13, Android 11, Android 10, and a lower version than Android 10.
If you know any screen missing from these instructions, please test them as well.
Media Browser
Media Picker
Story
Regression Notes
Potential unintended areas of impact
All features use camera permissions.
What I did to test those areas of impact (or what existing automated tests I relied on)
Tested manually.
What automated tests I added (or what prevented me from doing so)
This does not introduce a new feature, and we don't cover permissions in UI tests.
PR submission checklist:
RELEASE-NOTES.txt
if necessary.UI Changes testing checklist: