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");
>> >>             }
>> >>
>> >>
>> >>
>> >
>>
>

Reply via email to