ImagePicker returning same image #306
Conversation
iOS 10.2+ web view keep returning same image regardless whichever we pick from gallery view, given if we use FILE_URI method. I was looking for a solution of this issue, and while most suggestion was to use DATA_URI instead of FILE_URI that increases memory usage (at least in my case). After putting some breakpoint and debugging here is what I found: - Cordova app code request camera to pick a image from gallery with FILE_URI destination - CDVCamera.m contains code which execute that native task and store file at a `temporary` location and return its URI. - In order to get the temporary file name, `tempFilePath` method is used, which sets an integer value of 1 and then loop through temporary directory until it find an unused name with incrementing that value. - Cordova app consume the URI and read data. - As a best practice it calls `cleanup()` for freeing temporary resource. - CDVCamera.m removes the temporary file. This all flow works fine. However next time in same session when client request another picture, tempFilePath method returns same filename because there is no previous file holding the name. That is also fine, but here the twist by web view, which getting same URL returns old image (from browser's cache). Experimenting with filename gives a fix that if we use a random filename, it works as expected. So I propose this change (which should work almost in all cases, as picking a image from gallery will take in general more than 1 second), which uses unix timestamp value instead of integer ` i`. I don't have coding experience with Objective-C, so code change between line apache#402-404 can be updated to use better approach instead of 3 objects (date, interval and number).
ImagePicker returning same image
|
@janpio Any particular reason why this hasn't been merged yet? This plugin is impossible to use in production, unless you use the more memory-intensive |
|
Any particular reason why you mention someone (me) in a PR who hasn't appeared in its reviews, description or comments? I consider this pretty impolite. I am not your free labor - even when I am active in other issues or pull requests. I will answer you question anyway:
No. Just because nobody reviewed and merged this PR until now. Apache Cordova is a bunch of volunteers, so stuff happens when it happens. You can help by checking out @klodha's branch of the plugin yourself and test it, confirm that it works and leave an "Approve" review or whatever you experience when testing as a comment. If there are indicators a PR is good, it is much easier for a Cordova contributor to jump in, which of course moves the PR to the top of the pile. |
|
My apologies, my intent was not to be rude. I just wasn't sure how to get this 8 month old PR the attention it deserves. My fear was that this would never end up being merged, even though this plugin is unusable as is for a live production app (with FILE_URI). I've been using @klodha's branch for a few months, and it works like a charm. Figured it would be good idea to get this plugin production-ready. |
Fixed indention as per @janpio's comment.
|
Hi, Only change I made in plugin was iOS source files. However I see that Travis CI is failing build. Further checking the details, I found its failing build for Browser platform. So that is not related to the change I did. Is there anything which I can do to get Travis test getting passed as well? Thanks |
|
The same test failed on the last pure docs commit to Still, someone else besides me should review and approve this PR before we merge it. I just don't know enough to merge this in myself. |
janpio
moved this from ⛔ Blocked: Tests failing
to ⏳ Ready for Review
in Apache Cordova: Plugin Pull Requests
|
Hi, Just checking if anyone got chance to review this change :) For another project I started using camera plugin and then it reminded me on this. No hurries, just wanted to ensure if its in review queue still :) |
|
It is definitely in the queue, but the queue is pretty endless and not many people are working on it right now. If you can, please test out this PR in your own app and let us know if it works by reviewing the PR in the "Files changed" tab. Thanks. |
|
Hi. I can confirm that this works. I have an application in App Store which is using this change. And since then tried this in 2 other apps including the most recent one, and this fix seems to be working fine for me. Mainly the change I made in this PR is generating unique file name for temporary file using the timestamps. That way web view can't cache it for each new image. With the original approach, it was using an integer starting from 1, and in each session it was creating same file name as last one. Web views got confused perhaps and thought we are requesting same file and started returning from the cache. That was the origin and alternate was making each url unique, and using a timestamp sound like a good option to me. As any camera action would take enough time to ensure that we get different value. Hope this helps. |
There was a problem hiding this comment.
This change seems to be working fine in new project as well existing project in which I used it. Whole point of this change to generate a unique file name, for which timestamp seems best candidate. Because gap between 2 consecutive camera operation allows enough time to make a difference in this value. So instead of an integer starting with 1 for each camera session, started using timestamp to ensure webkit get a new FILE_URI each time.
|
Can anyone merge this changes please? |
|
Hi, It has been almost a year and half, since originally pull request was created. Given it passed reviews and has required approvals, is there any chance to get it merged before iOS 13 release. Regards |
|
Thanks for the PR @klodha. I'm going to merge it when the pipelines succeed. |
* commit 'a9436b1a185b37269366ec3e2c5f337fb14cba61': ImagePicker returning same image (apache#306) chore(release): 4.1.1-dev chore(release): 4.1.0 (version string) chore(release): release notes for 4.1.0 docs: remove outdated test docs translations build: remove `.ratignore` file that is not needed any more chore: fix repo and issue urls and license in package.json and plugin.xml fix: temporarily remove Appium tests (apache#468) ci(travis): Update Travis CI configuration for new paramedic (apache#455) (Android) Fix NullPointerException error on some Android phones (apache#429) Update CI Environment Setup for Node.js 6 (apache#438)
ImagePicker returning same image Co-authored-by: Jan Piotrowski <piotrowski+github@gmail.com> Co-authored-by: Tim Brust <github@timbrust.de> (cherry picked from commit a9436b1)
iOS 10.2+ web view keep returning same image regardless whichever we pick from gallery view, given if we use FILE_URI method. I was looking for a solution of this issue, and while most suggestion was to use DATA_URI instead of FILE_URI that increases memory usage (at least in my case). After putting some breakpoint and debugging here is what I found:
temporarylocation and return its URI.tempFilePathmethod is used, which sets an integer value of 1 and then loop through temporary directory until it find an unused name with incrementing that value.cleanup()for freeing temporary resource.This all flow works fine. However next time in same session when client request another picture, tempFilePath method returns same filename because there is no previous file holding the name. That is also fine, but here the twist by web view, which getting same URL returns old image (from browser's cache). Experimenting with filename gives a fix that if we use a random filename, it works as expected.
So I propose this change (which should work almost in all cases, as picking a image from gallery will take in general more than 1 second), which uses unix timestamp value instead of integer
i.I don't have coding experience with Objective-C, so code change between line #402-404 can be updated to use better approach instead of 3 objects (date, interval and number).
Platforms affected
What does this PR do?
What testing has been done on this change?
Checklist