[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
Github user cojomojo commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/115#issuecomment-201945371 @infil00p Should we close this PR? After thinking about this for quite some time, I think fixing https://issues.apache.org/jira/browse/CB-9506 would be favorable over this and would by consequence make this PR irrelevant anyways. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
Github user cojomojo commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/115#issuecomment-150748364 @infil00p Yes, it would still have the problem. I submitted this fix still because it is trivial (this PR won't change how the plugin is used at all, and only changes 2 lines, outside of the function itself), while the fix for 9506 will be non-trivial. Therefore, I think that unless 9506 is going to be fixed soon we should still patch 9490 with this. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
Github user infil00p commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/115#issuecomment-150703173 @cojomojo Won't this still have the problem you found when you filed this? https://issues.apache.org/jira/browse/CB-9506 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
Github user cojomojo commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/115#issuecomment-147473478 @sgrebnov @vladimir-kotikov OK, I fixed the issue caused by a bad merge. This PR is ready once we see how @infil00p 's PR for Cordova-Android 5.0.x and the permissions issue he mentioned affects things. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
Github user cojomojo commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/115#issuecomment-147443350 @infil00p Good idea, let me know when it's merged --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
Github user infil00p commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/115#issuecomment-147443002 Can this wait until after my Cordova-Android 5.0.x pull request is merged in? checkForDuplicateImage requires a permission on Android 6.0+ and I want to make sure that we don't accidentally add something without a permissions check. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
Github user cojomojo commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/115#issuecomment-147434553 @sgrebnov Something got messed up when I merged in the latest before resubmitting. I doubt this is even valid Java right now, I'll fix it later today and then we should be good to go. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
Github user sgrebnov commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/115#issuecomment-147357893 Reviewed: :+1: thank you @cojomojo! @vladimir-kotikov, I know that you are working on some other improvements so please merge this along with your work! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
Github user sgrebnov commented on a diff in the pull request: https://github.com/apache/cordova-plugin-camera/pull/115#discussion_r41742198 --- Diff: src/android/CameraLauncher.java --- @@ -386,12 +387,10 @@ private void processResultFromCamera(int destType, Intent intent) throws IOExcep FileHelper.stripFileProtocol(this.croppedUri.toString()) : FileHelper.stripFileProtocol(this.imageUri.toString()); -if (this.encodingType == JPEG) { -try { -//We don't support PNG, so let's not pretend we do -exif.createInFile(sourcePath); -exif.readExifData(); -rotate = exif.getOrientation(); +//We don't support PNG, so let's not pretend we do --- End diff -- I don't think this change should be in scope of this PR (I know there is a person working on .PNG support so this will just bring merge conflict) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
Github user sgrebnov commented on a diff in the pull request: https://github.com/apache/cordova-plugin-camera/pull/115#discussion_r41742092 --- Diff: src/android/CameraLauncher.java --- @@ -241,6 +241,7 @@ public void takePicture(int returnType, int encodingType) { * @return a File object pointing to the temporary picture */ private File createCaptureFile(int encodingType) { +File photo = null; --- End diff -- Looks like this variable is not used --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
Github user cojomojo commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/115#issuecomment-147061485 @sgrebnov Ok, I finally got around to doing what you suggested. Please re-review. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
Github user sgrebnov commented on a diff in the pull request: https://github.com/apache/cordova-plugin-camera/pull/115#discussion_r39375449 --- Diff: src/android/CameraLauncher.java --- @@ -983,12 +983,39 @@ private Cursor queryImgDB(Uri contentStore) { * @param newImage */ private void cleanup(int imageType, Uri oldImage, Uri newImage, Bitmap bitmap) { +Uri contentStore = whichContentStore(); +Cursor cursor = queryImgDB(contentStore); +int currentNumOfImages = cursor.getCount(); +int diff = currentNumOfImages - numPics; +int id; + if (bitmap != null) { bitmap.recycle(); } -// Clean up initial camera-written image file. -(new File(FileHelper.stripFileProtocol(oldImage.toString(.delete(); +// Check for camera-written image files that are not the chosen image +// Gets rid of inital camera-written image file as well as any retakes +// Addresses issue CD-9490 +if (currentNumOfImages > 0 && diff > 0) { +cursor.moveToLast(); +id = Integer.valueOf(cursor.getString(cursor.getColumnIndex(MediaStore.Images.Media._ID))); + +if (imageType == FILE_URI) { +id--; +for (int i = diff; i > 0; i--) { +Uri uri = Uri.parse(contentStore + "/" + id); + this.cordova.getActivity().getContentResolver().delete(uri, null, null); +id--; +} +} else { +for (int i = diff; i > 0; i--) { --- End diff -- looks like this `for` loop is the same as above; what if we keep only one loop, for example ``` if (imageType == FILE_URI) { id--; } for (int i = diff; i > 0; i--) { Uri uri = Uri.parse(contentStore + "/" + id); this.cordova.getActivity().getContentResolver().delete(uri, null, null); id--; } ``` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
Github user sgrebnov commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/115#issuecomment-140016250 @cojomojo the fix makes perfect sense, but I see that the code you use is very similar to what we have in `checkForDuplicateImage` and looks like that function is not required anymore (as I think you cover that case) so I propose to rename `checkForDuplicateImage` to `removeTemporaryImages` (or `cleanupTempImages`) and move your logic there. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
Github user nikhilkh commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/115#issuecomment-139374302 @sgrebnov to see if you can help merge this. Do we have the correct test coverage for this? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
Github user cojomojo commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/115#issuecomment-137577342 Is this PR still being considered? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
Github user cojomojo commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/115#issuecomment-133206691 Fixed the PR to only affect the cleanup function. Please re review. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
Github user cojomojo commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/115#issuecomment-133204027 That was not suppose to be removed, must have done that accidentally. Ill fix it, and update the PR in a few minutes. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
Github user infil00p commented on the pull request: https://github.com/apache/cordova-plugin-camera/pull/115#issuecomment-133185529 You removed part of the cropping functionality, we can't accept this pull request as it is. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-plugin-camera pull request: CB-9490 Fixed cleanup function
GitHub user cojomojo opened a pull request: https://github.com/apache/cordova-plugin-camera/pull/115 CB-9490 Fixed cleanup function Modfied cleanup function to deal with camera applications that save photos that should have been discarded when the retake option was chosen You can merge this pull request into a Git repository by running: $ git pull https://github.com/cojomojo/cordova-plugin-camera CB-9490 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-plugin-camera/pull/115.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #115 commit 5d5ffeacca32ec7676a205353286f7fc649517c5 Author: Cody Balos Date: 2015-08-14T21:05:10Z CB-9490 Fixed cleanup function Modfied cleanup function to deal with camera applications that save photos that should have been discarded when the retake option was chosen --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org For additional commands, e-mail: dev-h...@cordova.apache.org