You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Atul Vani <at...@gmail.com> on 2013/07/25 09:35:54 UTC
Re: svn commit: r1506828 - /ofbiz/trunk/framework/images/webapp/images/selectall.js
Hi Jacques,
As far as I have seen, it is always recommended to use === for comparison
in javascript, as others always end up with mysterious errors. References:
http://www.codeproject.com/Articles/580165/JavaScript-Best-Practices
http://net.tutsplus.com/tutorials/javascript-ajax/24-javascript-best-practices-for-beginners/
On Thu, Jul 25, 2013 at 12:48 PM, <jl...@apache.org> wrote:
> Author: jleroux
> Date: Thu Jul 25 07:18:03 2013
> New Revision: 1506828
>
> URL: http://svn.apache.org/r1506828
> Log:
> Rhaaa another wrong C/P, better use patches really :/
>
> Modified:
> ofbiz/trunk/framework/images/webapp/images/selectall.js
>
> Modified: ofbiz/trunk/framework/images/webapp/images/selectall.js
> URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/images/webapp/images/selectall.js?rev=1506828&r1=1506827&r2=1506828&view=diff
>
> ==============================================================================
> --- ofbiz/trunk/framework/images/webapp/images/selectall.js (original)
> +++ ofbiz/trunk/framework/images/webapp/images/selectall.js Thu Jul 25
> 07:18:03 2013
> @@ -352,7 +352,7 @@ function ajaxSubmitFormUpdateAreas(form,
> }
> updateFunction = function(data) {
> if (data._ERROR_MESSAGE_LIST_ != undefined || data._ERROR_MESSAGE_
> != undefined) {
> - if (jQuery('#content-messages').length == 0) {
> + if (!jQuery('#content-messages').length) {
> //add this div just after app-navigation
> if(jQuery('#content-main-section')){
> jQuery('#content-main-section' ).before('<div
> id="content-messages" onclick="hideErrorContainer()"></div>');
> @@ -367,8 +367,8 @@ function ajaxSubmitFormUpdateAreas(form,
> jQuery('#content-messages' ).html(data._ERROR_MESSAGE_);
> }
> jQuery('#content-messages').fadeIn('fast');
> - }else {
> - if (jQuery('#content-messages').length == 0) {
> + } else {
> + if (jQuery('#content-messages').length) {
> jQuery('#content-messages').html('');
>
> jQuery('#content-messages').removeClass('errorMessage').fadeIn("fast");
> }
>
>
>
Re: svn commit: r1506828 -
/ofbiz/trunk/framework/images/webapp/images/selectall.js
Posted by Jacques Le Roux <ja...@les7arts.com>.
You are certainly right Atul, but if we really want to get this way then we should also wonder about the 231 occurences of
.length) {
we have in OFBiz (no speaking about looking for ".length" itself).
There are mostly in jQuery plugins but even in jQuery UI.
BTW I wonder why we have jquery-ui-1.8.16.custom.js and jquery-ui-1.9.0.custom.js, seems that Sascha missed something when he respectively upgraded those
It's interesting to see though, that there are any occurences of this string (".length) {") in jQuery core!
Anyway as you said it's not an easy thing to change...
Jacques
----- Original Message -----
From: "Atul Vani" <at...@gmail.com>
To: <de...@ofbiz.apache.org>
Sent: Thursday, July 25, 2013 10:16 AM
Subject: Re: svn commit: r1506828 - /ofbiz/trunk/framework/images/webapp/images/selectall.js
> === 0 will ensure that the condition do not work out for other results like
> null, false, '' and undefined. Which is the recommended way. Just saying
> that while you are at it, we can do it this way. An effort at such scale
> will take a lot of testing and will break things. I think brute force is
> not the right way to do this kind of clean up.
>
>
> On Thu, Jul 25, 2013 at 1:35 PM, Jacques Le Roux <
> jacques.le.roux@les7arts.com> wrote:
>
>> You are right Atul, and in jQuery they use also !==
>>
>> Note that in the last version of the change below, == is not used, because
>> checking for lenght is enough, no needs to check its size
>>
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/images/webapp/images/selectall.js?r1=1506828&r2=1506827&pathrev=1506828
>>
>> By and large, we could change that in all the js we use in OFBiz.
>> Is that what you suggest, would you want to contribute a such effort?
>>
>> Thanks
>>
>> Jacques
>>
>> ----- Original Message -----
>> From: "Atul Vani" <at...@gmail.com>
>> To: <de...@ofbiz.apache.org>
>> Cc: <co...@ofbiz.apache.org>
>> Sent: Thursday, July 25, 2013 9:35 AM
>> Subject: Re: svn commit: r1506828 -
>> /ofbiz/trunk/framework/images/webapp/images/selectall.js
>>
>>
>> > Hi Jacques,
>> >
>> > As far as I have seen, it is always recommended to use === for comparison
>> > in javascript, as others always end up with mysterious errors.
>> References:
>> > http://www.codeproject.com/Articles/580165/JavaScript-Best-Practices
>> >
>> http://net.tutsplus.com/tutorials/javascript-ajax/24-javascript-best-practices-for-beginners/
>> >
>> >
>> >
>> > On Thu, Jul 25, 2013 at 12:48 PM, <jl...@apache.org> wrote:
>> >
>> >> Author: jleroux
>> >> Date: Thu Jul 25 07:18:03 2013
>> >> New Revision: 1506828
>> >>
>> >> URL: http://svn.apache.org/r1506828
>> >> Log:
>> >> Rhaaa another wrong C/P, better use patches really :/
>> >>
>> >> Modified:
>> >> ofbiz/trunk/framework/images/webapp/images/selectall.js
>> >>
>> >> Modified: ofbiz/trunk/framework/images/webapp/images/selectall.js
>> >> URL:
>> >>
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/images/webapp/images/selectall.js?rev=1506828&r1=1506827&r2=1506828&view=diff
>> >>
>> >>
>> ==============================================================================
>> >> --- ofbiz/trunk/framework/images/webapp/images/selectall.js (original)
>> >> +++ ofbiz/trunk/framework/images/webapp/images/selectall.js Thu Jul 25
>> >> 07:18:03 2013
>> >> @@ -352,7 +352,7 @@ function ajaxSubmitFormUpdateAreas(form,
>> >> }
>> >> updateFunction = function(data) {
>> >> if (data._ERROR_MESSAGE_LIST_ != undefined ||
>> data._ERROR_MESSAGE_
>> >> != undefined) {
>> >> - if (jQuery('#content-messages').length == 0) {
>> >> + if (!jQuery('#content-messages').length) {
>> >> //add this div just after app-navigation
>> >> if(jQuery('#content-main-section')){
>> >> jQuery('#content-main-section' ).before('<div
>> >> id="content-messages" onclick="hideErrorContainer()"></div>');
>> >> @@ -367,8 +367,8 @@ function ajaxSubmitFormUpdateAreas(form,
>> >> jQuery('#content-messages' ).html(data._ERROR_MESSAGE_);
>> >> }
>> >> jQuery('#content-messages').fadeIn('fast');
>> >> - }else {
>> >> - if (jQuery('#content-messages').length == 0) {
>> >> + } else {
>> >> + if (jQuery('#content-messages').length) {
>> >> jQuery('#content-messages').html('');
>> >>
>> >> jQuery('#content-messages').removeClass('errorMessage').fadeIn("fast");
>> >> }
>> >>
>> >>
>> >>
>> >
>>
>
Re: svn commit: r1506828 - /ofbiz/trunk/framework/images/webapp/images/selectall.js
Posted by Atul Vani <at...@gmail.com>.
=== 0 will ensure that the condition do not work out for other results like
null, false, '' and undefined. Which is the recommended way. Just saying
that while you are at it, we can do it this way. An effort at such scale
will take a lot of testing and will break things. I think brute force is
not the right way to do this kind of clean up.
On Thu, Jul 25, 2013 at 1:35 PM, Jacques Le Roux <
jacques.le.roux@les7arts.com> wrote:
> You are right Atul, and in jQuery they use also !==
>
> Note that in the last version of the change below, == is not used, because
> checking for lenght is enough, no needs to check its size
>
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/images/webapp/images/selectall.js?r1=1506828&r2=1506827&pathrev=1506828
>
> By and large, we could change that in all the js we use in OFBiz.
> Is that what you suggest, would you want to contribute a such effort?
>
> Thanks
>
> Jacques
>
> ----- Original Message -----
> From: "Atul Vani" <at...@gmail.com>
> To: <de...@ofbiz.apache.org>
> Cc: <co...@ofbiz.apache.org>
> Sent: Thursday, July 25, 2013 9:35 AM
> Subject: Re: svn commit: r1506828 -
> /ofbiz/trunk/framework/images/webapp/images/selectall.js
>
>
> > Hi Jacques,
> >
> > As far as I have seen, it is always recommended to use === for comparison
> > in javascript, as others always end up with mysterious errors.
> References:
> > http://www.codeproject.com/Articles/580165/JavaScript-Best-Practices
> >
> http://net.tutsplus.com/tutorials/javascript-ajax/24-javascript-best-practices-for-beginners/
> >
> >
> >
> > On Thu, Jul 25, 2013 at 12:48 PM, <jl...@apache.org> wrote:
> >
> >> Author: jleroux
> >> Date: Thu Jul 25 07:18:03 2013
> >> New Revision: 1506828
> >>
> >> URL: http://svn.apache.org/r1506828
> >> Log:
> >> Rhaaa another wrong C/P, better use patches really :/
> >>
> >> Modified:
> >> ofbiz/trunk/framework/images/webapp/images/selectall.js
> >>
> >> Modified: ofbiz/trunk/framework/images/webapp/images/selectall.js
> >> URL:
> >>
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/images/webapp/images/selectall.js?rev=1506828&r1=1506827&r2=1506828&view=diff
> >>
> >>
> ==============================================================================
> >> --- ofbiz/trunk/framework/images/webapp/images/selectall.js (original)
> >> +++ ofbiz/trunk/framework/images/webapp/images/selectall.js Thu Jul 25
> >> 07:18:03 2013
> >> @@ -352,7 +352,7 @@ function ajaxSubmitFormUpdateAreas(form,
> >> }
> >> updateFunction = function(data) {
> >> if (data._ERROR_MESSAGE_LIST_ != undefined ||
> data._ERROR_MESSAGE_
> >> != undefined) {
> >> - if (jQuery('#content-messages').length == 0) {
> >> + if (!jQuery('#content-messages').length) {
> >> //add this div just after app-navigation
> >> if(jQuery('#content-main-section')){
> >> jQuery('#content-main-section' ).before('<div
> >> id="content-messages" onclick="hideErrorContainer()"></div>');
> >> @@ -367,8 +367,8 @@ function ajaxSubmitFormUpdateAreas(form,
> >> jQuery('#content-messages' ).html(data._ERROR_MESSAGE_);
> >> }
> >> jQuery('#content-messages').fadeIn('fast');
> >> - }else {
> >> - if (jQuery('#content-messages').length == 0) {
> >> + } else {
> >> + if (jQuery('#content-messages').length) {
> >> jQuery('#content-messages').html('');
> >>
> >> jQuery('#content-messages').removeClass('errorMessage').fadeIn("fast");
> >> }
> >>
> >>
> >>
> >
>
Re: svn commit: r1506828 -
/ofbiz/trunk/framework/images/webapp/images/selectall.js
Posted by Jacques Le Roux <ja...@les7arts.com>.
You are right Atul, and in jQuery they use also !==
Note that in the last version of the change below, == is not used, because checking for lenght is enough, no needs to check its size
http://svn.apache.org/viewvc/ofbiz/trunk/framework/images/webapp/images/selectall.js?r1=1506828&r2=1506827&pathrev=1506828
By and large, we could change that in all the js we use in OFBiz.
Is that what you suggest, would you want to contribute a such effort?
Thanks
Jacques
----- Original Message -----
From: "Atul Vani" <at...@gmail.com>
To: <de...@ofbiz.apache.org>
Cc: <co...@ofbiz.apache.org>
Sent: Thursday, July 25, 2013 9:35 AM
Subject: Re: svn commit: r1506828 - /ofbiz/trunk/framework/images/webapp/images/selectall.js
> Hi Jacques,
>
> As far as I have seen, it is always recommended to use === for comparison
> in javascript, as others always end up with mysterious errors. References:
> http://www.codeproject.com/Articles/580165/JavaScript-Best-Practices
> http://net.tutsplus.com/tutorials/javascript-ajax/24-javascript-best-practices-for-beginners/
>
>
>
> On Thu, Jul 25, 2013 at 12:48 PM, <jl...@apache.org> wrote:
>
>> Author: jleroux
>> Date: Thu Jul 25 07:18:03 2013
>> New Revision: 1506828
>>
>> URL: http://svn.apache.org/r1506828
>> Log:
>> Rhaaa another wrong C/P, better use patches really :/
>>
>> Modified:
>> ofbiz/trunk/framework/images/webapp/images/selectall.js
>>
>> Modified: ofbiz/trunk/framework/images/webapp/images/selectall.js
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/images/webapp/images/selectall.js?rev=1506828&r1=1506827&r2=1506828&view=diff
>>
>> ==============================================================================
>> --- ofbiz/trunk/framework/images/webapp/images/selectall.js (original)
>> +++ ofbiz/trunk/framework/images/webapp/images/selectall.js Thu Jul 25
>> 07:18:03 2013
>> @@ -352,7 +352,7 @@ function ajaxSubmitFormUpdateAreas(form,
>> }
>> updateFunction = function(data) {
>> if (data._ERROR_MESSAGE_LIST_ != undefined || data._ERROR_MESSAGE_
>> != undefined) {
>> - if (jQuery('#content-messages').length == 0) {
>> + if (!jQuery('#content-messages').length) {
>> //add this div just after app-navigation
>> if(jQuery('#content-main-section')){
>> jQuery('#content-main-section' ).before('<div
>> id="content-messages" onclick="hideErrorContainer()"></div>');
>> @@ -367,8 +367,8 @@ function ajaxSubmitFormUpdateAreas(form,
>> jQuery('#content-messages' ).html(data._ERROR_MESSAGE_);
>> }
>> jQuery('#content-messages').fadeIn('fast');
>> - }else {
>> - if (jQuery('#content-messages').length == 0) {
>> + } else {
>> + if (jQuery('#content-messages').length) {
>> jQuery('#content-messages').html('');
>>
>> jQuery('#content-messages').removeClass('errorMessage').fadeIn("fast");
>> }
>>
>>
>>
>