jQuery 3.5.0 breaks at least AngularJS interfaces

[Galen Charlton]

Testing today reveals that Evergreen 3.2 and later built with the most recent jQuery (3.5.0) breaks the AngularJS report template interface in at least two ways:

- when creating a new template, only the template name, URL, and description form fields are displayed; nothing else
- when cloning a template, only the template name, URL, and description form fields are displayed, and the console includes an error message about dgrid.refresh() not existing.

I'm reasonable sure that the new jQuery version is the culprit because reverting /openils/var/web/js/ui/default/common/build/js/jquery.min.js to 3.4.1 or earlier solves the problem.

Evergreen 3.2+

[Galen Charlton (gmc)]

Further research suggests that the specific culprit may be the security fix in 3.5.0 that changes how jquery.htmlPreFilter works. [1] In particular, it no longer converts closing tags to make them XHTML complaint.

The following appears to make the report template editor work with jQuery 3.5.0.

diff --git a/Open-ILS/src/templates/staff/reporter/t_edit_template.tt2 b/Open-ILS/src/templates/staff/reporter/t_edit_template.tt2
index 97b71c02f3..e85e526c99 100644
--- a/Open-ILS/src/templates/staff/reporter/t_edit_template.tt2
+++ b/Open-ILS/src/templates/staff/reporter/t_edit_template.tt2
@@ -21,7 +21,7 @@
     [% l('Template Description') %]
   </div>
   <div class="col-md-10">
- <div><textarea class="form-control" ng-model="templateDescription" required/></div>
+ <div><textarea class="form-control" ng-model="templateDescription" required></textarea></div>
   </div>
 </div>

[1] https://blog.jquery.com/2020/04/10/jquery-3-5-0-released/

[Galen Charlton (gmc)]

This _might_ also be associated with the use of angular-tree-control.

[Galen Charlton (gmc)]

After further testing, no, angular-tree-control probably isn't relevant.

[Galen Charlton (gmc)]

Another wrinkle: the volume copy editor is partially broken due to the jQuery update as well; culprit is non-self-closing tags being entered as self-closing:

diff --git a/Open-ILS/web/js/ui/default/staff/cat/volcopy/app.js b/Open-ILS/web/js/ui/default/staff/cat/volcopy/app.js
index 9a5aba7a70..22f282a2cf 100644
--- a/Open-ILS/web/js/ui/default/staff/cat/volcopy/app.js
+++ b/Open-ILS/web/js/ui/default/staff/cat/volcopy/app.js
@@ -599,17 +599,17 @@ function(egCore , $q) {
             '<div class="row">'+
                 '<div class="col-xs-2">'+
                     '<button aria-label="Delete" style="margin:-5px -15px; float:left;" ng-hide="callNumber.not_ephemeral" type="button" class="close" ng-click="remov
eCN()">&times;</button>' +
- '<select class="form-control" ng-model="classification" ng-change="updateClassification()" ng-options="cl.name() for cl in classification_list"/>'
+
+ '<select class="form-control" ng-model="classification" ng-change="updateClassification()" ng-options="cl.name() for cl in classification_list"></
select>'+
                 '</div>'+
                 '<div class="col-xs-1">'+
- '<select class="form-control" ng-model="prefix" ng-change="updatePrefix()" ng-options="p.label() for p in prefix_list"/>'+
+ '<select class="form-control" ng-model="prefix" ng-change="updatePrefix()" ng-options="p.label() for p in prefix_list"></select>'+
                 '</div>'+
                 '<div class="col-xs-2">'+
                     '<input class="form-control" type="text" ng-change="updateLabel()" ng-model="label"/>'+
                     '<div class="label label-danger" ng-if="empty_label">{{empty_label_string}}</div>'+
                 '</div>'+
                 '<div class="col-xs-1">'+
- '<select class="form-control" ng-model="suffix" ng-change="updateSuffix()" ng-options="s.label() for s in suffix_list"/>'+
+ '<select class="form-control" ng-model="suffix" ng-change="updateSuffix()" ng-options="s.label() for s in suffix_list"></select>'+
                 '</div>'+
                 '<div ng-hide="onlyVols" class="col-xs-1"><input class="form-control" type="number" ng-model="copy_count" min="{{orig_copy_count}}" ng-change="changeCPCount()"></div>'+
                 '<div ng-hide="onlyVols" class="col-xs-5">'+

[Galen Charlton (gmc)]

Here's a pass at identifying all of the non-void elements that are entered as self-closing:

gmc@gmc-eg:~/Evergreen$ grep -r -Pzo '<[^>]*?/>' Open-ILS/web/js/ui/default/staff/|perl -npe 's/\000/\n/g'|grep -Po '^.*?:<\S+'|grep -v -P ':\<(input|br|hr|eg-|link|img|!--)'|grep -v node_modules|grep -v '\.svg'|grep -v vendor.bundle.js
Open-ILS/web/js/ui/default/staff/services/ui.js:<progress/>
Open-ILS/web/js/ui/default/staff/services/ui.js:<progress/>
Open-ILS/web/js/ui/default/staff/cat/volcopy/app.js:<select
Open-ILS/web/js/ui/default/staff/cat/volcopy/app.js:<select
Open-ILS/web/js/ui/default/staff/cat/volcopy/app.js:<select
gmc@gmc-eg:~/Evergreen$ grep -r -Pzo '<[^>]*?/>' Open-ILS/src/templates/staff/|perl -npe 's/\000/\n/g'|grep -Po '^.*?:<\S+'|grep -v -P ':\<(input|br|hr|eg-|link|img|!--)'
Open-ILS/src/templates/staff/acq/requests/t_edit.tt2:<select
Open-ILS/src/templates/staff/acq/requests/t_cancel.tt2:<select
Open-ILS/src/templates/staff/reporter/t_edit_template.tt2:<textarea
Open-ILS/src/templates/staff/share/t_autogrid.tt2:<span
Open-ILS/src/templates/staff/cat/item/t_list.tt2:<span
Open-ILS/src/templates/staff/cat/item/t_list.tt2:<span
Open-ILS/src/templates/staff/cat/bucket/record/t_pending.tt2:<span
Open-ILS/src/templates/staff/cat/bucket/record/t_search.tt2:<span

[Galen Charlton (gmc)]

Another way of identifying in-appropriate self-closing tags from Mike Rylander is

ack '<[^>]+/>'|egrep -v 'area|base|br|embed|hr|iframe|img|input|link|meta|param|source|track|missing|xsd|xsl|examples|xul|font|i18n|idl|IDL|gnu|SuperCat|eg-'|less

This is broader that what I put in comment #5, but some of the results it turns up can be ignored after verifying that they're never run through jQuery.

[Jason Stephenson (jstephenson)]

For me, jQuery 3.5.0 also breaks the OPAC, just going to the OPAC home page or searching the catalog from the web staff client results in an Internal Server Error on Evergreen 3.2.8. Downloading jQuery 3.4.1 and replacing 3.5.0 with that resolves the Internal Server Error. I have not tested on a more recent Evergreen release. I have also not looked into potential fixes, yet, nor tested Galen's patches.

I'm willing to collaborate on efforts to fix this. A stop gap measure might be to pin jQuery at version 3.4.1 until a better fix is available.

[Jason Stephenson (jstephenson)]

More testing today reveals that my comment #7 is wrong. jQuery 3.5.0 does not break the OPAC on my 3.2.8 test VM. I don't know what the problem was. Perhaps some services were not running at the time that I started the test? I have tested again, both by doing make install and by manually copying jQuery 3.5.0 into place on the server, and both times my OPAC continues to function.

[Jason Stephenson (jstephenson)]

I have begun to attack fixing this. I have run Mike's suggested ack command on the branches from rel_3_2 to rel_3_5 and master. I excluded the MARC templates since these won't need changes.

I assume that all of these instances need to be fixed regardless of whether or not the tags are run through jQuery. The reason for this assumption is that these self-closed, non-void elements are technically invalid HTML.

I will skip the XHTML files as these are probably OK. XHTML, being a subset of XML, has different rules for this.

It looks like one patch will apply for master down to rel_3_3 based on a cursory examination of the output. I plan to do rel_3_2 as a separate patch branch for anyone who needs or wants it, such as possibly myself. If anyone thinks doing rel_3_2 is a waste of time, please speak up.

Also if anyone has any questions, comments, or complaints, let me know. I will probably not get to any actual code changes before some time tomorrow, April 24.

[Jason Stephenson (jstephenson)]

Here is my workng-in-progress branch so far. This first commit hopefully covers everything in master:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/dyrcona/lp1873286-nonvoid-tag-fix

A quick test shows that it cherry-picks cleanly into rel_3_5, rel_3_4, and rel_3_3. rel_3_2 will require a separate branch if we want to go that route.

I have yet to build this on a test vm and try things out.

Also, there is a change to the schema in the form of an updated template for the PO HTML action_trigger event. I have no, yet, provided an upgrade script for this as some sites may have customized this template.

Finally, here is a script that I wrote to find the broken tags:

https://pastebin.com/s5PaGy5T

If anyone else wants to bang on this, let me know and push to a collab branch. I'll probably get around to testing this over the weekend or early next week. I'll add the pullrequest tag after I've had a chance to bang on it some.

[Jason Stephenson (jstephenson)]

Here's a branch for rel_3_2 for anyone else who might want/need it. I'm not suggesting that we actually update rel_3_2 for this bug.

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/dyrcona/lp1873286-nonvoid-tag-fix_rel_3_2

[Jason Stephenson (jstephenson)]

I have looked at most of the affected interfaces and nothing jumps out at me as broken by this patch. The interfaces that were known to be broken by jQuery 3.5.0 appear to be repaired. I'm adding the pullrequest tag, now, so that we can get other eyes on the branch.

[Bill Erickson (berick)]

Sign-off branch pushed:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1873286-nonvoid-tag-fix

I eyeballed the changes and tested a variety of interfaces after updating to jquery 3.5.0.

[Chris Sharp (chrissharp123)]

Pushed to 3.3, 3.4, 3.5, and master. Thanks, Galen, Jason and Bill!