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" <atulv...@gmail.com> To: <dev@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.r...@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" <atulv...@gmail.com> >> To: <dev@ofbiz.apache.org> >> Cc: <comm...@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, <jler...@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"); >> >> } >> >> >> >> >> >> >> > >> >