Hello Team, A PR is now ready for your review, I also updated all unit tests with this new Camera API behavior across all platforms: https://github.com/apache/cordova-plugin-camera/pull/432
Thank you On Fri, Mar 8, 2019 at 6:42 PM Hazem Saleh <haz...@apache.org> wrote: > 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 > -- Hazem Saleh Twitter: http://www.twitter.com/hazems
