# Wrong to use !jQuery('#content-messages') in \framework\images\webapp\images\selectall.js line #354

## Details

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: Trunk
• Fix Version/s:
• Component/s:
• Labels:
None

## Description

I think we should use

if(jQuery('#content-messages').length==0)

insead of

if(!jQuery('#content-messages'))

in framework\images\webapp\images\selectall.js line #354.

## Attachments

1. patches.zip
1 kB
Wei Zhang

## Activity

Hide
Wei Zhang added a comment -
Show
Wei Zhang added a comment - in addition, please refer to http://www.paulund.co.uk/check-to-see-if-a-element-exists-in-jquery
Hide
Jacques Le Roux added a comment -

I totally agree with you, I missed this when I committed for OFBIZ-4022. It's now committed in trunk at revision: 1506274, and will backport in releases when OFBIZ-4794 will de done (so I don't close now), thanks!

Show
Jacques Le Roux added a comment - I totally agree with you, I missed this when I committed for OFBIZ-4022 . It's now committed in trunk at revision: 1506274, and will backport in releases when OFBIZ-4794 will de done (so I don't close now), thanks!
Hide
Wei Zhang added a comment -

In addition the hideErrorContainer function in line #349 cannot hide the error message. Maybe we should use jQuery('#content-messages').html('');

Show
Wei Zhang added a comment - In addition the hideErrorContainer function in line #349 cannot hide the error message. Maybe we should use jQuery('#content-messages').html('');
Hide
Wei Zhang added a comment -

Hi Jacques,

There is a Syntax Error in line #345, you missed "{".

Regards,

Wei

Show
Wei Zhang added a comment - Hi Jacques, There is a Syntax Error in line #345, you missed "{". Regards, Wei
Hide
Jacques Le Roux added a comment -

Oops, indeed, thanks Wei Zhang, fixed at revision 1506418.

Show
Jacques Le Roux added a comment - Oops, indeed, thanks Wei Zhang, fixed at revision 1506418.
Hide
Jacques Le Roux added a comment -

Wei Zhang,

I did not test, about your 1st comment

In addition the hideErrorContainer function in line #349 cannot hide the error message. Maybe we should use jQuery('#content-messages').html('');

is it still true after the typo fix?

Show
Jacques Le Roux added a comment - Wei Zhang, I did not test, about your 1st comment In addition the hideErrorContainer function in line #349 cannot hide the error message. Maybe we should use jQuery('#content-messages').html(''); is it still true after the typo fix?
Hide
Wei Zhang added a comment - - edited

Yes, the statement below in line #350 cannot hide message.

jQuery('#content-messages').removeClass('errorMessage').fadeIn('fast')

Show
Wei Zhang added a comment - - edited Yes, the statement below in line #350 cannot hide message. jQuery('#content-messages').removeClass('errorMessage').fadeIn('fast')
Hide
Jacques Le Roux added a comment -

Did you test
jQuery('#content-messages').html('');
?

Show
Jacques Le Roux added a comment - Did you test jQuery('#content-messages').html(''); ?
Hide
Jacques Le Roux added a comment -

BTW there is the same line at line 371, I guess what you suggest should work, but I have no time to test, please confirm, thanks!

Show
Jacques Le Roux added a comment - BTW there is the same line at line 371, I guess what you suggest should work, but I have no time to test, please confirm, thanks!
Hide
Wei Zhang added a comment -

Yes, I tested. and I think we also need to change line #371. So the correct function should be

```function ajaxSubmitFormUpdateAreas(form, areaCsvString) {
waitSpinnerShow();
hideErrorContainer = function() {
jQuery('#content-messages').html('');
jQuery('#content-messages').removeClass('errorMessage').fadeIn('fast');
}
updateFunction = function(data) {
if (data._ERROR_MESSAGE_LIST_ != undefined || data._ERROR_MESSAGE_ != undefined) {
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>');
}
}
jQuery('#content-messages').addClass('errorMessage');
if (data._ERROR_MESSAGE_LIST_ != undefined && data._ERROR_MESSAGE_ != undefined) {
jQuery('#content-messages' ).html(data._ERROR_MESSAGE_LIST_ + " " + data._ERROR_MESSAGE_);
} else if (data._ERROR_MESSAGE_LIST_ != undefined) {
jQuery('#content-messages' ).html(data._ERROR_MESSAGE_LIST_);
} else {
jQuery('#content-messages' ).html(data._ERROR_MESSAGE_);
}
jQuery('#content-messages').fadeIn('fast');
}else {
if(jQuery('#content-messages').length) {
jQuery('#content-messages').html('');
jQuery('#content-messages').removeClass('errorMessage').fadeIn("fast");
}
ajaxUpdateAreas(areaCsvString);
}
waitSpinnerHide();
}

jQuery.ajax({
type: "POST",
url: jQuery("#" + form).attr("action"),
data: jQuery("#" + form).serialize(),
success: function(data) {
updateFunction(data);
}
});
}
```
Show
Wei Zhang added a comment - Yes, I tested. and I think we also need to change line #371. So the correct function should be function ajaxSubmitFormUpdateAreas(form, areaCsvString) { waitSpinnerShow(); hideErrorContainer = function() { jQuery('#content-messages').html(''); jQuery('#content-messages').removeClass('errorMessage').fadeIn('fast'); } updateFunction = function(data) { if (data._ERROR_MESSAGE_LIST_ != undefined || data._ERROR_MESSAGE_ != undefined) { 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>'); } } jQuery('#content-messages').addClass('errorMessage'); if (data._ERROR_MESSAGE_LIST_ != undefined && data._ERROR_MESSAGE_ != undefined) { jQuery('#content-messages' ).html(data._ERROR_MESSAGE_LIST_ + " " + data._ERROR_MESSAGE_); } else if (data._ERROR_MESSAGE_LIST_ != undefined) { jQuery('#content-messages' ).html(data._ERROR_MESSAGE_LIST_); } else { jQuery('#content-messages' ).html(data._ERROR_MESSAGE_); } jQuery('#content-messages').fadeIn('fast'); } else { if (jQuery('#content-messages').length) { jQuery('#content-messages').html(''); jQuery('#content-messages').removeClass('errorMessage').fadeIn( "fast" ); } ajaxUpdateAreas(areaCsvString); } waitSpinnerHide(); } jQuery.ajax({ type: "POST" , url: jQuery( "#" + form).attr( "action" ), data: jQuery( "#" + form).serialize(), success: function(data) { updateFunction(data); } }); }
Hide
Jacques Le Roux added a comment -

Could you please rather provide a patch based on trunk HEAD?

Except if you confirme that I just have to add the line

jQuery('#content-messages').html('');

above the 2
jQuery('#content-messages').removeClass('errorMessage').fadeIn("fast");
lines

right?

Show
Jacques Le Roux added a comment - Could you please rather provide a patch based on trunk HEAD? Except if you confirme that I just have to add the line jQuery('#content-messages').html(''); above the 2 jQuery('#content-messages').removeClass('errorMessage').fadeIn("fast"); lines right?
Hide
Wei Zhang added a comment -

Please see attached patch

Show
Wei Zhang added a comment - Please see attached patch
Hide
Jacques Le Roux added a comment -

Thanks Wei Zhang,

I have completed by hand using your patch as a basis. Completed: At revision: 1506499

Next time please follow https://cwiki.apache.org/confluence/display/OFBADMIN/OFBiz+Contributors+Best+Practices

Thanks!

Show
Jacques Le Roux added a comment - Thanks Wei Zhang, I have completed by hand using your patch as a basis. Completed: At revision: 1506499 Next time please follow https://cwiki.apache.org/confluence/display/OFBADMIN/OFBiz+Contributors+Best+Practices Thanks!
Hide
Jacques Le Roux added a comment -

I committed a typo fix at revision 1506504. Another reason to follow the normal way for patches

Show
Jacques Le Roux added a comment - I committed a typo fix at revision 1506504. Another reason to follow the normal way for patches
Hide
Wei Zhang added a comment -

Hi Jacques,

The line below in line #371 was not changed correctly

```if (jQuery('#content-messages').length == 0)
```

It should be(It should be opposite of line #355)

```if (jQuery('#content-messages').length)
```

or

```if (jQuery('#content-messages').length>0)
```

Thanks,

Wei

Show
Wei Zhang added a comment - Hi Jacques, The line below in line #371 was not changed correctly if (jQuery('#content-messages').length == 0) It should be(It should be opposite of line #355) if (jQuery('#content-messages').length) or if (jQuery('#content-messages').length>0) Thanks, Wei
Hide
Jacques Le Roux added a comment - - edited

Thanks Wai Zhang,

I should have taken this fix more seriously (I was focused on smthg else). BTW really next time use a real patch, but yes my bad, thanks for review!

Committed at revision 1506828.

Show
Jacques Le Roux added a comment - - edited Thanks Wai Zhang, I should have taken this fix more seriously (I was focused on smthg else). BTW really next time use a real patch, but yes my bad, thanks for review! Committed at revision 1506828.
Hide
Jacques Le Roux added a comment -

I put as resolved/incomplete to facilitate searches with "My resolved pending" own filter

Show
Jacques Le Roux added a comment - I put as resolved/incomplete to facilitate searches with "My resolved pending" own filter
Hide
Jacques Le Roux added a comment -

Backported in
R13.07 r1533495 + r1533500 + r1533508
R12.04 r1533496 + r1533501 + r1533509
R11.04 r1533497 + r1533502 + r1533510

Show
Jacques Le Roux added a comment - Backported in R13.07 r1533495 + r1533500 + r1533508 R12.04 r1533496 + r1533501 + r1533509 R11.04 r1533497 + r1533502 + r1533510
Hide
Jacques Le Roux added a comment -

Need to clean the Jira situation here

Show
Jacques Le Roux added a comment - Need to clean the Jira situation here
Hide
Jacques Le Roux added a comment -

Done!

Show
Jacques Le Roux added a comment - Done!

## People

• Assignee:
Jacques Le Roux
Reporter:
Wei Zhang
• Votes:
0 Vote for this issue
Watchers:
2 Start watching this issue

## Dates

• Created:
Updated:
Resolved: