jQuery 3.5.0 breaks at least AngularJS interfaces

Bug #1873286 reported by Galen Charlton
28
This bug affects 5 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Critical
Unassigned
3.3
Fix Released
Critical
Unassigned
3.4
Fix Released
Critical
Unassigned

Bug Description

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+

Tags: pullrequest
Revision history for this message
Galen Charlton (gmc) wrote :

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/

Revision history for this message
Galen Charlton (gmc) wrote :

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

Revision history for this message
Galen Charlton (gmc) wrote :

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

Revision history for this message
Galen Charlton (gmc) wrote :

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">'+

Revision history for this message
Galen Charlton (gmc) wrote :

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

Revision history for this message
Galen Charlton (gmc) wrote :

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.

Galen Charlton (gmc)
summary: - jQuery 3.5.0 breaks AngularJS reporter template interface
+ jQuery 3.5.0 breaks at least AngularJS interfaces
Changed in evergreen:
assignee: nobody → Jason Stephenson (jstephenson)
Changed in evergreen:
status: New → Confirmed
assignee: Jason Stephenson (jstephenson) → nobody
Revision history for this message
Jason Stephenson (jstephenson) wrote :

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.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

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.

Changed in evergreen:
assignee: nobody → Jason Stephenson (jstephenson)
Revision history for this message
Jason Stephenson (jstephenson) wrote :

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.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

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.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

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

Revision history for this message
Jason Stephenson (jstephenson) wrote :

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.

Changed in evergreen:
assignee: Jason Stephenson (jstephenson) → nobody
milestone: none → 3.5.0
tags: added: pullrequest
Bill Erickson (berick)
Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
Revision history for this message
Bill Erickson (berick) wrote :

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.

Changed in evergreen:
assignee: Bill Erickson (berick) → nobody
Revision history for this message
Chris Sharp (chrissharp123) wrote :

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

Changed in evergreen:
status: Confirmed → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.