OFBiz › OFBiz - Dev

Re: svn commit: r1506828 - /ofbiz/trunk/framework/images/webapp/images/selectall.js

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

4 messages Options

Atul Vani-2

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, <[hidden email]> 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");
> }
>
>
>

Jacques Le Roux

Re: svn commit: r1506828 - /ofbiz/trunk/framework/images/webapp/images/selectall.js

Administrator

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" <[hidden email]>
To: <[hidden email]>
Cc: <[hidden email]>
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, <[hidden email]> 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");
>> }
>>
>>
>>
>

Atul Vani-2

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 <
[hidden email]> 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" <[hidden email]>
> To: <[hidden email]>
> Cc: <[hidden email]>
> 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, <[hidden email]> 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");
> >> }
> >>
> >>
> >>
> >
>

Jacques Le Roux

Re: svn commit: r1506828 - /ofbiz/trunk/framework/images/webapp/images/selectall.js

Administrator

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" <[hidden email]>
To: <[hidden email]>
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 <
> [hidden email]> 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" <[hidden email]>
>> To: <[hidden email]>
>> Cc: <[hidden email]>
>> 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, <[hidden email]> 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");
>> >> }
>> >>
>> >>
>> >>
>> >
>>
>