You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2020/09/30 15:41:26 UTC
[GitHub] [cordova-plugin-camera] PieterVanPoyer opened a new pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
PieterVanPoyer opened a new pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669
<!--
Please make sure the checklist boxes are all checked before submitting the PR. The checklist is intended as a quick reference, for complete details please see our Contributor Guidelines:
http://cordova.apache.org/contribute/contribute_guidelines.html
Thanks!
-->
### Platforms affected
- Android
### Motivation and Context
<!-- Why is this change required? What problem does it solve? -->
<!-- If it fixes an open issue, please link to the issue here. -->
- Fixes #341
- Fixes #577
The saveToPhotoAlbum setting throws an exception while using the camera. (sourceType = CAMERA)
This results in a crash of the application.
The first bug was reported in september 2018.
This PR only tries to solve the issue for the sourceType Camera.PictureSourceType.CAMERA, not for when the sourceType is PHOTOLIBRARY.
A reproduction of the issue can be done with https://github.com/PieterVanPoyer/cordova-camera-plugin-testing-app repo.
### Description
<!-- Describe your changes in detail -->
- saveToPhotoAlbum feature fixed by taken into count content - uri. (writeUncompressedImage method) ( see: apache#611 (comment) )
- make saveToPhotoAlbum future proof by using the MediaStore to insert the taken image
- made a method to calculate the compressFormat based on the encodingType (JPEG or PNG)
- layout of the performCrop method is adjusted
- removed unused rotate variable inside processResultFromGallery method
- added a GalleryPathVO (ValueObject) for more easy working with the path and imagename.
- add extra VO class to the plugin.xml
### Testing
<!-- Please describe in detail how you tested your changes. -->
I did run npm run test.
Tested on Android emulators from Android 5.1, 7, 10 and 11.
Tested on a real Samsung J7 device (Android 9)
### Checklist
- [x] I've run the tests to see all new and existing tests pass
- [x] I added automated test coverage as appropriate for this change
- [x] Commit is prefixed with `(platform)` if this change only applies to one platform (e.g. `(android)`)
- [x] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct [keyword to close issues using keywords](https://help.github.com/articles/closing-issues-using-keywords/))
- [x] I've updated the documentation if necessary
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] breautek commented on a change in pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
breautek commented on a change in pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#discussion_r504985709
##########
File path: src/android/CameraLauncher.java
##########
@@ -494,16 +494,18 @@ private void processResultFromCamera(int destType, Intent intent) throws IOExcep
// in the gallery and the modified image is saved in the temporary
// directory
if (this.saveToPhotoAlbum) {
- galleryUri = Uri.fromFile(new File(getPicturesPath()));
+ GalleryPathVO galleryPathVO = getPicturesPath();
+ galleryUri = Uri.fromFile(new File(galleryPathVO.getGalleryPath()));
if (this.allowEdit && this.croppedUri != null) {
writeUncompressedImage(croppedUri, galleryUri);
} else {
- Uri imageUri = this.imageUri;
- writeUncompressedImage(imageUri, galleryUri);
+ if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) { // Between LOLLIPOP_MR1 and P
Review comment:
> But is it really necessary to be able to compile with the sdk version equal to 22?
My personal opinion is no.... but I think that's what our min requirement is going to be. I'm about to write a new thread in the dev mail to see if the community can come to a consensus and so that we can document what Cordova supports for enterprise users. I'm assuming the consensus will be "whatever what the min sdk version is", which is currently 22.
Obviously this only applies to people who aren't publishing to the google play store.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] PieterVanPoyer commented on a change in pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on a change in pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#discussion_r504950465
##########
File path: src/android/CameraLauncher.java
##########
@@ -494,16 +494,18 @@ private void processResultFromCamera(int destType, Intent intent) throws IOExcep
// in the gallery and the modified image is saved in the temporary
// directory
if (this.saveToPhotoAlbum) {
- galleryUri = Uri.fromFile(new File(getPicturesPath()));
+ GalleryPathVO galleryPathVO = getPicturesPath();
+ galleryUri = Uri.fromFile(new File(galleryPathVO.getGalleryPath()));
if (this.allowEdit && this.croppedUri != null) {
writeUncompressedImage(croppedUri, galleryUri);
} else {
- Uri imageUri = this.imageUri;
- writeUncompressedImage(imageUri, galleryUri);
+ if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) { // Between LOLLIPOP_MR1 and P
Review comment:
@breautek
I digged a little deeper.
When I change the Build.VERSION_CODES_P to the numerical 28 , I am able to compile and run it with the defaultCompileSdkVersion starting from 22 and higher.
So I changed the code to the 28 number.
But is it really necessary to be able to compile with the sdk version equal to 22?
Kind regards
Pieter
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] PieterVanPoyer edited a comment on pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer edited a comment on pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#issuecomment-701212434
@erisu or @breautek can anybody review this PR, please?
The Travic build failed, but nothing was changed for the failed tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] PieterVanPoyer edited a comment on pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer edited a comment on pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#issuecomment-701212434
@erisu or @breautek can anybody review this PR, please?
The Travis build failed, but nothing was changed for the failed tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] breautek closed pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
breautek closed pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] breautek commented on pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
breautek commented on pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#issuecomment-705865814
I'll give a 1 week call out.
If there are no objections by October 16 I'll merge.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] PieterVanPoyer commented on pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#issuecomment-705793122
@breautek Hey it seems like erisu can not find the time to review this PR, maybe just merge it to the master? Or maybe we should request another reviewer?
Kind regards.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] breautek commented on pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
breautek commented on pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#issuecomment-710733001
Travis failures are unrelated to this PR. Merging now. Thank you @PieterVanPoyer for your time and effort in preparing this PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] breautek merged pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
breautek merged pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] PieterVanPoyer commented on a change in pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on a change in pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#discussion_r504950465
##########
File path: src/android/CameraLauncher.java
##########
@@ -494,16 +494,18 @@ private void processResultFromCamera(int destType, Intent intent) throws IOExcep
// in the gallery and the modified image is saved in the temporary
// directory
if (this.saveToPhotoAlbum) {
- galleryUri = Uri.fromFile(new File(getPicturesPath()));
+ GalleryPathVO galleryPathVO = getPicturesPath();
+ galleryUri = Uri.fromFile(new File(galleryPathVO.getGalleryPath()));
if (this.allowEdit && this.croppedUri != null) {
writeUncompressedImage(croppedUri, galleryUri);
} else {
- Uri imageUri = this.imageUri;
- writeUncompressedImage(imageUri, galleryUri);
+ if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) { // Between LOLLIPOP_MR1 and P
Review comment:
@breautek
I digged a little deeper.
When I change the Build.VERSION_CODES_P to the numerical 28 , I am able to compile and run it with the defaultCompileSdkVersion starting from 22 and higher.
So I changed the code to the 28 number. ( https://github.com/apache/cordova-plugin-camera/pull/669/commits/a30375fafbe78c467c2818fba7b85220ade5d89b )
I repeated some successfull testing (android 22, 29).
But is it really necessary to be able to compile with the sdk version equal to 22?
Kind regards
Pieter
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] PieterVanPoyer commented on a change in pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on a change in pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#discussion_r504950465
##########
File path: src/android/CameraLauncher.java
##########
@@ -494,16 +494,18 @@ private void processResultFromCamera(int destType, Intent intent) throws IOExcep
// in the gallery and the modified image is saved in the temporary
// directory
if (this.saveToPhotoAlbum) {
- galleryUri = Uri.fromFile(new File(getPicturesPath()));
+ GalleryPathVO galleryPathVO = getPicturesPath();
+ galleryUri = Uri.fromFile(new File(galleryPathVO.getGalleryPath()));
if (this.allowEdit && this.croppedUri != null) {
writeUncompressedImage(croppedUri, galleryUri);
} else {
- Uri imageUri = this.imageUri;
- writeUncompressedImage(imageUri, galleryUri);
+ if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) { // Between LOLLIPOP_MR1 and P
Review comment:
@breautek
I digged a little deeper.
When I change the Build.VERSION_CODES_P to the numerical 28 , I am able to compile and run it with the defaultCompileSdkVersion starting from 22 and higher.
So I changed the code to the 28 number. ( https://github.com/apache/cordova-plugin-camera/pull/669/commits/a30375fafbe78c467c2818fba7b85220ade5d89b )
But is it really necessary to be able to compile with the sdk version equal to 22?
Kind regards
Pieter
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] PieterVanPoyer commented on a change in pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on a change in pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#discussion_r504806816
##########
File path: src/android/CameraLauncher.java
##########
@@ -494,16 +494,18 @@ private void processResultFromCamera(int destType, Intent intent) throws IOExcep
// in the gallery and the modified image is saved in the temporary
// directory
if (this.saveToPhotoAlbum) {
- galleryUri = Uri.fromFile(new File(getPicturesPath()));
+ GalleryPathVO galleryPathVO = getPicturesPath();
+ galleryUri = Uri.fromFile(new File(galleryPathVO.getGalleryPath()));
if (this.allowEdit && this.croppedUri != null) {
writeUncompressedImage(croppedUri, galleryUri);
} else {
- Uri imageUri = this.imageUri;
- writeUncompressedImage(imageUri, galleryUri);
+ if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) { // Between LOLLIPOP_MR1 and P
Review comment:
**Changing the defaultTargetSdkVersion to 22 keeps working with the code.**
Only when it is necessary to change the defaultCompileSdkVersion below 28, there is a problem.
When I change to code to the numerical ``28`` I can go to defaultCompileSdkVersion 26.
With the defaultCompileSdkVersion set to 25. I've got resource linking errors.
Just to be sure: Does Cordova require the Cameraplugin (and any other official plugin) to be able to work with the `defaultCompileSdkVersion` set to 22?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] PieterVanPoyer commented on pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#issuecomment-701212434
@erisu or @breautek can anybody review this PR, please?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] breautek commented on a change in pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
breautek commented on a change in pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#discussion_r504370675
##########
File path: src/android/CameraLauncher.java
##########
@@ -494,16 +494,18 @@ private void processResultFromCamera(int destType, Intent intent) throws IOExcep
// in the gallery and the modified image is saved in the temporary
// directory
if (this.saveToPhotoAlbum) {
- galleryUri = Uri.fromFile(new File(getPicturesPath()));
+ GalleryPathVO galleryPathVO = getPicturesPath();
+ galleryUri = Uri.fromFile(new File(galleryPathVO.getGalleryPath()));
if (this.allowEdit && this.croppedUri != null) {
writeUncompressedImage(croppedUri, galleryUri);
} else {
- Uri imageUri = this.imageUri;
- writeUncompressedImage(imageUri, galleryUri);
+ if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) { // Between LOLLIPOP_MR1 and P
Review comment:
I kinda forget android behaviour, can we double check that `Build.VERSION_CODES.P` is safe to use on devices that are API < 28?
Ie: Android 5.1 simulator, with an app built with target API 29.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] PieterVanPoyer commented on pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#issuecomment-703140017
@erisu Hey, I hate to disturb you, but can you find the time to review this PR. Kind regards.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] breautek commented on pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
breautek commented on pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#issuecomment-705865814
I'll give a 1 week call out.
If there are no objections by October 16 I'll merge.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] breautek commented on a change in pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
breautek commented on a change in pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#discussion_r504632953
##########
File path: src/android/CameraLauncher.java
##########
@@ -494,16 +494,18 @@ private void processResultFromCamera(int destType, Intent intent) throws IOExcep
// in the gallery and the modified image is saved in the temporary
// directory
if (this.saveToPhotoAlbum) {
- galleryUri = Uri.fromFile(new File(getPicturesPath()));
+ GalleryPathVO galleryPathVO = getPicturesPath();
+ galleryUri = Uri.fromFile(new File(galleryPathVO.getGalleryPath()));
if (this.allowEdit && this.croppedUri != null) {
writeUncompressedImage(croppedUri, galleryUri);
} else {
- Uri imageUri = this.imageUri;
- writeUncompressedImage(imageUri, galleryUri);
+ if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) { // Between LOLLIPOP_MR1 and P
Review comment:
Ok, even though this normally would be the acceptable approach because Google Play enforces apps to be built with target SDK 28, (and soon to be forced for all apps to use API target 29)
Cordova is suppose to support enterprise environments where the play store is not involved in their process, and therefore they may use a lower target SDK than what Google demands. This means we can't actually use the `VERSION_CODES` constants unless if they are introduced in/before our min sdk level. So I guess for now the solution to this is to change `Build.VERSION_CODES.P` to a hard-code numerical `28`.
This isn't documented in the Cordova docs, I'm requesting the actual minimum target API we support to be defined so that this can actually be documented.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] PieterVanPoyer commented on pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#issuecomment-705793122
@breautek Hey it seems like erisu can not find the time to review this PR, maybe just merge it to the master? Or maybe we should request another reviewer?
Kind regards.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] PieterVanPoyer commented on pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#issuecomment-706563564
> I'll give a 1 week call out.
>
> If there are no objections by October 16 I'll merge.
Sound like a great plan! Thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] breautek commented on pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
breautek commented on pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#issuecomment-701473070
Travis is failling on browser tests, and considering this PR changes nothing in the browser platform, I don't think it's a blocker. It seems to be a timeout issue, so I'll close/reopen this PR to retrigger travis. Maybe that's all it is.
I had a glance at this PR and I don't see anything wrong, but I would like to spend some time to actually test on a physical device, but I don't think I can do that this week.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] PieterVanPoyer commented on a change in pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on a change in pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#discussion_r504514583
##########
File path: src/android/CameraLauncher.java
##########
@@ -494,16 +494,18 @@ private void processResultFromCamera(int destType, Intent intent) throws IOExcep
// in the gallery and the modified image is saved in the temporary
// directory
if (this.saveToPhotoAlbum) {
- galleryUri = Uri.fromFile(new File(getPicturesPath()));
+ GalleryPathVO galleryPathVO = getPicturesPath();
+ galleryUri = Uri.fromFile(new File(galleryPathVO.getGalleryPath()));
if (this.allowEdit && this.croppedUri != null) {
writeUncompressedImage(croppedUri, galleryUri);
} else {
- Uri imageUri = this.imageUri;
- writeUncompressedImage(imageUri, galleryUri);
+ if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) { // Between LOLLIPOP_MR1 and P
Review comment:
Yes, this works.
- I dit test with target API 29, and 5.1 emulator. It works.
The Build.VERSION_CODES are compile-time constants that gets inlined in the generated bytecode. Because of that, the symbol does not need to be resolved at runtime.
Some additional context:
- I did also make the decision to use smaller or equal to Build.VERSION_CODES.P instead of smaller than Build.VERSION_CODES.Q . Because When you downscale the compile version to 28, it keeps compiling and working.
- But you cannot compile with version 27. Becuase VERSION_CODES.P is not available in that sdk.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] PieterVanPoyer commented on a change in pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on a change in pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#discussion_r504806816
##########
File path: src/android/CameraLauncher.java
##########
@@ -494,16 +494,18 @@ private void processResultFromCamera(int destType, Intent intent) throws IOExcep
// in the gallery and the modified image is saved in the temporary
// directory
if (this.saveToPhotoAlbum) {
- galleryUri = Uri.fromFile(new File(getPicturesPath()));
+ GalleryPathVO galleryPathVO = getPicturesPath();
+ galleryUri = Uri.fromFile(new File(galleryPathVO.getGalleryPath()));
if (this.allowEdit && this.croppedUri != null) {
writeUncompressedImage(croppedUri, galleryUri);
} else {
- Uri imageUri = this.imageUri;
- writeUncompressedImage(imageUri, galleryUri);
+ if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) { // Between LOLLIPOP_MR1 and P
Review comment:
**Changing the defaultTargetSdkVersion to 22 keeps working with the code.**
Only when it is necessary to change the defaultCompileSdkVersion below 28, there is a problem.
When I change the `Build.VERSION_CODES.P` code to the numerical ``28`` I can go to defaultCompileSdkVersion 26.
With the defaultCompileSdkVersion set to 25. I've got resource linking errors.
Just to be sure: Does Cordova require the Cameraplugin (and any other official plugin) to be able to work with the `defaultCompileSdkVersion` set to 22?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-camera] PieterVanPoyer commented on a change in pull request #669: Bugfix issue 341 save to photo gallery - Fixes #341, fixes #577
Posted by GitBox <gi...@apache.org>.
PieterVanPoyer commented on a change in pull request #669:
URL: https://github.com/apache/cordova-plugin-camera/pull/669#discussion_r504514583
##########
File path: src/android/CameraLauncher.java
##########
@@ -494,16 +494,18 @@ private void processResultFromCamera(int destType, Intent intent) throws IOExcep
// in the gallery and the modified image is saved in the temporary
// directory
if (this.saveToPhotoAlbum) {
- galleryUri = Uri.fromFile(new File(getPicturesPath()));
+ GalleryPathVO galleryPathVO = getPicturesPath();
+ galleryUri = Uri.fromFile(new File(galleryPathVO.getGalleryPath()));
if (this.allowEdit && this.croppedUri != null) {
writeUncompressedImage(croppedUri, galleryUri);
} else {
- Uri imageUri = this.imageUri;
- writeUncompressedImage(imageUri, galleryUri);
+ if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) { // Between LOLLIPOP_MR1 and P
Review comment:
Yes, this works.
- I dit test with target API 29, and 5.1 emulator. It works.
The Build.VERSION_CODES are compile-time constants that gets inlined in the generated bytecode. Because of that, the symbol does not need to be resolved at runtime.
Some additional context:
- I did also make the decision to use smaller or equal to Build.VERSION_CODES.P instead of smaller than Build.VERSION_CODES.Q . Because When you downscale the compile version to 28, it keeps compiling and working.
- But you cannot compile with version 27. Because VERSION_CODES.P is not available in that sdk.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org