Need to use html filter on mkurl() output

Bug #1914116 reported by Jason Boyer
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Evergreen
Status tracked in Master
3.5
Confirmed
High
Unassigned
3.6
Confirmed
High
Unassigned
Master
Confirmed
High
Unassigned

Bug Description

Evergreen 3.5, 3.6, and master are affected.

So, recently we fixed bug 1687545 and now can sure that all query parameters consistently use ampersands rather than semicolons. Hurray!

But it turns out all of these semicolons were hiding areas where we were not using the necessary filters in our .tt2 files, so now things like the "Show more copies" link in the opac has changed from

.../eg/opac/record/2?locg=1;detail_record_view=0;query=concerto;copy_limit=50;copy_offset=0

to the new and exciting

.../eg/opac/record/2?locg=1&detail_record_view=0&query=concerto©_limit=50©_offset=0

which works about as well as one might expect.

The minimum required fix to correct this is to run anything that has query parameters and will end up in an 'a' tag href attribute through the html filter, though there may be times where multiple filters may be needed.

Jason Boyer (jboyer)
description: updated
Garry Collum (gcollum)
Changed in evergreen:
status: New → Confirmed
Revision history for this message
Jason Boyer (jboyer) wrote :

It looks like this can easily be handled in mkurl() itself. Since we don't currently run mkurl() output through the html filter anywhere it's easier to use the filter directly inside the macro and not add it everywhere mkurl is ever used. Branch is at https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/jboyer/lp1914116_html_filter / working/user/jboyer/lp1914116_html_filter

tags: added: pullrequest
Changed in evergreen:
milestone: none → 3.6.2
summary: - An audit of query parameters is needed
+ Need to use html filter on mkurl() output
Revision history for this message
Jason Boyer (jboyer) wrote :

To test on recent master:

Without the patch:
1. Load record id 2 from the Conerto dataset ("Le concerto") or any other record that has more than 10 copies.
2. Click the Show More Copies link at the bottom of the item table.
3. No more than 10 copies are displayed and your browser's address bar looks similar to this: https://server.time/eg/opac/record/2?query=concerto&qtype=keyword&locg=1&detail_record_view=0©_offset=0©_limit=50

Apply patch and repeat
1. Load record id 2 from the Conerto dataset ("Le concerto") or any other record that has more than 10 copies.
2. Click the Show More Copies link at the bottom of the item table.
3. Up to 50 copies are displayed and your browser's address bar looks similar to this: https://server.time/eg/opac/record/2?query=concerto&qtype=keyword&locg=1&detail_record_view=0&copy_offset=0&copy_limit=50

Revision history for this message
Garry Collum (gcollum) wrote :

Tested in both opacs. I was able to show more copies and the pagination worked. The ampersands in the urls were a bit more standard. A sign-off branch is at https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/gcollum/lp1914116_html_filter-signoff

tags: added: signedoff
Jason Boyer (jboyer)
Changed in evergreen:
assignee: nobody → Jason Boyer (jboyer)
Revision history for this message
Jason Boyer (jboyer) wrote :

Pushed to rel3_5, rel_3_6, and master. Thanks Garry!

Changed in evergreen:
assignee: Jason Boyer (jboyer) → nobody
status: Confirmed → Fix Committed
Revision history for this message
Jason Stephenson (jstephenson) wrote :

As a temporary fix for bug 1918470, the code from this branch was reverted from where it was pushed. Therefore, this bug is still open.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers