I have implemented and tested the DATA_URL fix across all of the Camera plugin supported platforms:
1. Android (Java) 2. iOS (Objective C) 3. OS X (Objective C) 4. Browser (JavaScript) 5. Windows Platform (JavaScript) Since this is a cross-cutting concern across all of platforms for an old issue, I need your green light first before creating a PR. Attached below my diff that contains the cross-platform fix: https://github.com/apache/cordova-plugin-camera/compare/master...hazems:hazems/CB-GH-420?expand=1 Thanks On Fri, Mar 1, 2019 at 9:34 PM Hazem Saleh <haz...@apache.org> wrote: > I agree with Jesse. > > IMHO regarding the positive impact of this change is that it simply fixes > a bug that exists for a long time ago, and will make the code that consumes > this API more cleaner, the user will not have hard code a DATA url prefix > every-time before the returned base64 string. > > On Fri, Mar 1, 2019 at 4:36 AM Jesse <purplecabb...@gmail.com> wrote: > >> The downside would be that it would add yet another quirky implementation, >> while this about not dragging along the existing quirky non-standard >> implementation. >> Existing users who don't want to change their code don't need to change >> anything. >> >> I expect usage of this api is quite low already ( anecdotally ) >> >> Here's a couple other reasons the base64 encoded methods are problematic: >> - missing exif data >> - possibly incorrect image/jpeg vs image/png, luckily user agents appear >> to >> just use it >> >> >> @purplecabbage >> risingj.com >> >> >> On Fri, Mar 1, 2019 at 12:50 AM Oliver Salzburg < >> oliver.salzb...@gmail.com> >> wrote: >> >> > What is the downside of just introducing new options and not creating a >> > breaking change? >> > And what is the positive impact of this change that it necessitates that >> > every consumer of the existing API change their code? >> > >> > IMHO that has not been made clear enough. >> > >> > On 01/03/2019 02:17, Hazem Saleh wrote: >> > >>> My only reservation is that it is a questionable api imho, >> converting >> > > binary to text just to pass it through the bridge is computationally >> > > expensive and ram intensive and better cameras continue to make it >> worse. >> > > >> > > Regarding this, I think we can generally recommend in the >> documentation >> > to >> > > use the base64 API option in case of small images (and it is up to the >> > app >> > > developers to decide based on the problem that they are trying to >> solve). >> > > >> > > What do you think? >> > > >> > > On Wed, Feb 27, 2019 at 5:17 AM Jesse <purplecabb...@gmail.com> >> wrote: >> > > >> > >> I think I am okay with the breaking change, as long as we update >> major >> > >> My only reservation is that it is a questionable api imho, converting >> > >> binary to text just to pass it through the bridge is computationally >> > >> expensive and ram intensive and better cameras continue to make it >> > worse. >> > >> >> > >>> On Feb 27, 2019, at 1:37 AM, julio cesar sanchez < >> > jcesarmob...@gmail.com> >> > >> wrote: >> > >>> I’m +1 to changing it and doing a major release. >> > >>> >> > >>> It was called data url but it’s not a data url, that’s a bug that >> has >> > >>> become “feature”. >> > >>> I think we should have the new type but that returns current string >> and >> > >>> make data url return the real data url and document it on the >> breaking >> > >>> changes. >> > >>> >> > >>> >> > >>> El miércoles, 27 de febrero de 2019, Oliver Salzburg < >> > >>> oliver.salzb...@gmail.com> escribió: >> > >>> >> > >>>> I don't think the behavior should be changed. If anything, just >> > >> introduce >> > >>>> another type that has the prefix. And maybe add an alias for the >> > >> existing >> > >>>> type to make the differences clearer. >> > >>>> >> > >>>>> On 27/02/2019 03:51, Hazem Saleh wrote: >> > >>>>> >> > >>>>> Hello Team, >> > >>>>> >> > >>>>> When calling navigator.camera.getPicture with destinationType set >> to >> > >>>>> Camera.DestinationType.DATA_URL, the success function is called >> with >> > >> the >> > >>>>> base64 encoded data while it should has suitable data URL format >> on >> > the >> > >>>>> following form: >> > >>>>> >> > >>>>> data:[<mediatype>][;base64],<data> >> > >>>>> >> > >>>>> >> > >>>>> It is an old issue in the library and it was previously reported >> > under: >> > >>>>> https://issues.apache.org/jira/browse/CB-9819 >> > >>>>> >> > >>>>> And recently, it was also reported by one of the library users: >> > >>>>> https://github.com/apache/cordova-plugin-camera/issues/420 >> > >>>>> >> > >>>>> I have a solution for this problem in Android[1], and I can >> create a >> > >>>>> similar one for iOS and windows platform but my question is, Do we >> > >> really >> > >>>>> want to support this since it is a breaking change as in all older >> > >>>>> versions >> > >>>>> of Cordova, a base64 encoded data was always sent, and definitely >> all >> > >>>>> documentation is needed to be updated. >> > >>>>> >> > >>>>> [OT] I'm sending this email since Julio asked me to post this >> > proposal >> > >>>>> here >> > >>>>> in a GitHub issue discussion. >> > >>>>> >> > >>>>> Thanks >> > >>>>> >> > >>>>> [1] >> > >>>>> https://github.com/hazems/cordova-plugin-camera/commit/e6609 >> > >>>>> c0b6a590a30ef7802826888693142576bee >> > >>>>> >> > >>>>> >> > >>>> >> --------------------------------------------------------------------- >> > >>>> To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org >> > >>>> For additional commands, e-mail: dev-h...@cordova.apache.org >> > >>>> >> > >>>> >> > >> --------------------------------------------------------------------- >> > >> To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org >> > >> For additional commands, e-mail: dev-h...@cordova.apache.org >> > >> >> > >> >> > >> > --------------------------------------------------------------------- >> > To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org >> > For additional commands, e-mail: dev-h...@cordova.apache.org >> > >> > >> > > > -- > Hazem Saleh > Twitter: http://www.twitter.com/hazems > -- Hazem Saleh Twitter: http://www.twitter.com/hazems
