Bug 38096

Summary: Field 857 is not considered for display on XSLT files
Product: Koha Reporter: Pedro Amorim <pedro.amorim>
Component: MARC Bibliographic data supportAssignee: Pedro Amorim <pedro.amorim>
Status: Pushed to main --- QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: aude.charillon, bibliothek, caroline.cyr-la-rose, david, jonathan.druart, lucas, m.de.rooy, martin.renvoize, pedro.amorim
Version: MainKeywords: additional_work_needed, rel_25_11_candidate, release-notes-needed
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34667
GIT URL: Initiative type: ---
Sponsorship status: --- Comma delimited list of Sponsors:
Crowdfunding goal: 0 Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
26.05.00
Circulation function:
Bug Depends on:    
Bug Blocks: 6892    
Attachments: Bug 38096: OPAC: Preparation
Bug 38096: OPAC: Move 856 logic to renderMARCOnlineResourceField utils
Bug 38096: OPAC: Display 857 on detail and results pages
Bug 38096: Staff: Display 857 to detail and results pages
[DONT PUSH] Bug 38096: Add test data for fields 856 and 857
Bug 38096: [DONT PUSH] Add test data for fields 856 and 857
Bug 38096: OPAC: Preparation
Bug 38096: OPAC: Move 856 logic to renderMARCOnlineResourceField utils
Bug 38096: OPAC: Display 857 on detail and results pages
Bug 38096: Staff: Display 857 to detail and results pages
Bug 38096: [DONT PUSH] Add test data for fields 856 and 857
Bug 38096: OPAC: Preparation
Bug 38096: OPAC: Move 856 logic to renderMARCOnlineResourceField utils
Bug 38096: OPAC: Display 857 on detail and results pages
Bug 38096: Staff: Display 857 to detail and results pages
Bug 38096: [DONT PUSH] Add test data for fields 856 and 857
Bug 38096: (QA follow-up): Fix passing string param to template
Bug 38096: OPAC: Preparation
Bug 38096: OPAC: Move 856 logic to renderMARCOnlineResourceField utils
Bug 38096: OPAC: Display 857 on detail and results pages
Bug 38096: Staff: Display 857 to detail and results pages
Bug 38096: [DONT PUSH] Add test data for fields 856 and 857
Bug 38096: (QA follow-up): Fix passing string param to template
Bug 38096: OPAC: Preparation
Bug 38096: OPAC: Move 856 logic to renderMARCOnlineResourceField utils
Bug 38096: OPAC: Display 857 on detail and results pages
Bug 38096: Staff: Display 857 to detail and results pages
Bug 38096: [DONT PUSH] Add test data for fields 856 and 857
Bug 38096: (QA follow-up): Fix passing string param to template
Bug 38096: (QA follow-up): Fix indentation
Bug 38096: OPAC: Preparation
Bug 38096: OPAC: Move 856 logic to renderMARCOnlineResourceField utils
Bug 38096: OPAC: Display 857 on detail and results pages
Bug 38096: Staff: Display 857 to detail and results pages
Bug 38096: (QA follow-up): Fix passing string param to template
Bug 38096: (QA follow-up): Fix indentation
Bug 38096: (QA follow-up): Fix URLLinkText
Bug 38096: (follow-up) Fix missing biblionumber parameter in renderMARCOnlineResourceField
Bug 38096: (QA follow-up): Fix URLLinkText
Bug 38096: (follow-up) Fix missing biblionumber parameter in renderMARCOnlineResourceField
Bug 38096: (QA follow-up): Add URLLinkText to opac xsl also
Bug 38096: (QA follow-up): Add URLLinkText to opac xsl also

Description Pedro Amorim 2024-10-04 14:47:39 UTC
Bug 34667 added MARC framework support for field 857 in MARC21 to 23.11 but it is not displayed.
Comment 1 Pedro Amorim 2024-10-07 14:16:04 UTC
Created attachment 172466 [details] [review]
Bug 38096: OPAC: Preparation

The OPAC logic for 856 in results page and detail page has diverged.
This commit ensures the 856 in the OPAC detail and search results pages
are consistent, in preparation to become DRY in the following commits.

With this DRY refactor, the following divergences are now aligned:
- 856 block now calls AddMissingProtocol regardless of value OPACURLOpenInNewWindow (previously was only being called if OPACURLOpenInNewWindow was 0). Now it's in line with what exists in MARC21slim2OPACDetail.xsl
- substring(,1,6)='image/' is also now a consideration when OPACURLOpenInNewWindow is 1. (previously was only being called if OPACURLOpenInNewWindow was 0). This is now in line with that exists in MARC21slim2OPACDetail.xsl
Comment 2 Pedro Amorim 2024-10-07 14:16:10 UTC
Created attachment 172467 [details] [review]
Bug 38096: OPAC: Move 856 logic to renderMARCOnlineResourceField utils

Now that OPAC detail and search results are consitent, we can DRY this
logic to its own function to be also utilized by 857 in the following
commit
Comment 3 Pedro Amorim 2024-10-07 14:16:17 UTC
Created attachment 172468 [details] [review]
Bug 38096: OPAC: Display 857 on detail and results pages
Comment 4 Pedro Amorim 2024-10-07 14:16:26 UTC
Created attachment 172469 [details] [review]
Bug 38096: Staff: Display 857 to detail and results pages

This work DRYs the 856 display logic to renderMARCOnlineResourceField.
This was previously written in detail and results pages.

Once this single function exists, 857 can also make use of it to be
displayed using the same logic/features already present for 856.

1) If testing k-t-d, add the test data for test plan (skip if sandboxes):
updatedatabase
2) Force reindex of this biblio by saving it, visit the biblio:
<staff_url>/cgi-bin/koha/catalogue/detail.pl?biblionumber=76
3) Click "Edit" -> "Edit record" -> "Save"
4) Open the following URL's, preferably on 4 different browser tabs (we're using biblio 76 for this test):
<staff_url>/cgi-bin/koha/catalogue/search.pl?q=music (first record [76] is the one we're testing)
<staff_url>/cgi-bin/koha/catalogue/detail.pl?biblionumber=76
<opac_url>/cgi-bin/koha/opac-search.pl?idx=&q=music (first record [76] is the one we're testing)
<opac_url>/cgi-bin/koha/opac-detail.pl?biblionumber=76
5) Verify that on all pages there are 3 instances of 856 (Online resources) and 3 instances of 857 (Online archive resources) with their respective link labels and URLs.
6) Click on the "Koha community no protocol" link for 856 and 857, notice the 'http://' protocol is added.
7) Change the values of OPACDisplay856uAsImage + Display856uAsImage and refresh the respective page. Verify it works as expected (respects the system preference). Note: This only applies for 856, not 857 (out of scope here).
8) Change the value of OPACURLOpenInNewWindow and click on the links before and after. Verify they respect the system preference.
Comment 5 Pedro Amorim 2024-10-07 14:18:32 UTC
Created attachment 172470 [details] [review]
[DONT PUSH] Bug 38096: Add test data for fields 856 and 857
Comment 6 Pedro Amorim 2024-10-07 14:19:09 UTC
Created attachment 172471 [details] [review]
Bug 38096: [DONT PUSH] Add test data for fields 856 and 857
Comment 7 Caroline Cyr La Rose 2024-10-08 15:27:16 UTC
Created attachment 172532 [details] [review]
Bug 38096: OPAC: Preparation

The OPAC logic for 856 in results page and detail page has diverged.
This commit ensures the 856 in the OPAC detail and search results pages
are consistent, in preparation to become DRY in the following commits.

With this DRY refactor, the following divergences are now aligned:
- 856 block now calls AddMissingProtocol regardless of value OPACURLOpenInNewWindow (previously was only being called if OPACURLOpenInNewWindow was 0). Now it's in line with what exists in MARC21slim2OPACDetail.xsl
- substring(,1,6)='image/' is also now a consideration when OPACURLOpenInNewWindow is 1. (previously was only being called if OPACURLOpenInNewWindow was 0). This is now in line with that exists in MARC21slim2OPACDetail.xsl

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 8 Caroline Cyr La Rose 2024-10-08 15:27:52 UTC
Created attachment 172533 [details] [review]
Bug 38096: OPAC: Move 856 logic to renderMARCOnlineResourceField utils

Now that OPAC detail and search results are consitent, we can DRY this
logic to its own function to be also utilized by 857 in the following
commit

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 9 Caroline Cyr La Rose 2024-10-08 15:27:54 UTC
Created attachment 172534 [details] [review]
Bug 38096: OPAC: Display 857 on detail and results pages

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 10 Caroline Cyr La Rose 2024-10-08 15:27:56 UTC
Created attachment 172535 [details] [review]
Bug 38096: Staff: Display 857 to detail and results pages

This work DRYs the 856 display logic to renderMARCOnlineResourceField.
This was previously written in detail and results pages.

Once this single function exists, 857 can also make use of it to be
displayed using the same logic/features already present for 856.

1) If testing k-t-d, add the test data for test plan (skip if sandboxes):
updatedatabase
2) Force reindex of this biblio by saving it, visit the biblio:
<staff_url>/cgi-bin/koha/catalogue/detail.pl?biblionumber=76
3) Click "Edit" -> "Edit record" -> "Save"
4) Open the following URL's, preferably on 4 different browser tabs (we're using biblio 76 for this test):
<staff_url>/cgi-bin/koha/catalogue/search.pl?q=music (first record [76] is the one we're testing)
<staff_url>/cgi-bin/koha/catalogue/detail.pl?biblionumber=76
<opac_url>/cgi-bin/koha/opac-search.pl?idx=&q=music (first record [76] is the one we're testing)
<opac_url>/cgi-bin/koha/opac-detail.pl?biblionumber=76
5) Verify that on all pages there are 3 instances of 856 (Online resources) and 3 instances of 857 (Online archive resources) with their respective link labels and URLs.
6) Click on the "Koha community no protocol" link for 856 and 857, notice the 'http://' protocol is added.
7) Change the values of OPACDisplay856uAsImage + Display856uAsImage and refresh the respective page. Verify it works as expected (respects the system preference). Note: This only applies for 856, not 857 (out of scope here).
8) Change the value of OPACURLOpenInNewWindow and click on the links before and after. Verify they respect the system preference.

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 11 Caroline Cyr La Rose 2024-10-08 15:27:57 UTC
Created attachment 172536 [details] [review]
Bug 38096: [DONT PUSH] Add test data for fields 856 and 857

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 12 Caroline Cyr La Rose 2024-10-08 15:29:12 UTC
Thanks Pedro! 

In the test, I'd add to refresh the page in step 8 after changing the pref.

I hope I attached everything correctly... I don't often test bugs with many patches like this...
Comment 13 Pedro Amorim 2024-10-08 15:42:57 UTC
(In reply to Caroline Cyr La Rose from comment #12)
> Thanks Pedro! 
> 
> In the test, I'd add to refresh the page in step 8 after changing the pref.

That's great Caroline, thank you very much. 

(In reply to Caroline Cyr La Rose from comment #12)
> I hope I attached everything correctly... I don't often test bugs with many
> patches like this...

All patches seem to have been attached correctly. Thank you.
Comment 14 Pedro Amorim 2024-11-12 12:11:41 UTC
Patch no longer applies, updating here to come back to this later.
Comment 15 Pedro Amorim 2026-02-03 12:55:15 UTC
Created attachment 192376 [details] [review]
Bug 38096: OPAC: Preparation

The OPAC logic for 856 in results page and detail page has diverged.
This commit ensures the 856 in the OPAC detail and search results pages
are consistent, in preparation to become DRY in the following commits.

With this DRY refactor, the following divergences are now aligned:
- 856 block now calls AddMissingProtocol regardless of value OPACURLOpenInNewWindow (previously was only being called if OPACURLOpenInNewWindow was 0). Now it's in line with what exists in MARC21slim2OPACDetail.xsl
- substring(,1,6)='image/' is also now a consideration when OPACURLOpenInNewWindow is 1. (previously was only being called if OPACURLOpenInNewWindow was 0). This is now in line with that exists in MARC21slim2OPACDetail.xsl
Comment 16 Pedro Amorim 2026-02-03 12:55:18 UTC
Created attachment 192377 [details] [review]
Bug 38096: OPAC: Move 856 logic to renderMARCOnlineResourceField utils

Now that OPAC detail and search results are consitent, we can DRY this
logic to its own function to be also utilized by 857 in the following
commit
Comment 17 Pedro Amorim 2026-02-03 12:55:20 UTC
Created attachment 192378 [details] [review]
Bug 38096: OPAC: Display 857 on detail and results pages
Comment 18 Pedro Amorim 2026-02-03 12:55:23 UTC
Created attachment 192379 [details] [review]
Bug 38096: Staff: Display 857 to detail and results pages

This work DRYs the 856 display logic to renderMARCOnlineResourceField.
This was previously written in detail and results pages.

Once this single function exists, 857 can also make use of it to be
displayed using the same logic/features already present for 856.

1) If testing k-t-d, add the test data for test plan (skip if sandboxes):
updatedatabase
2) Force reindex of this biblio by saving it, visit the biblio:
<staff_url>/cgi-bin/koha/catalogue/detail.pl?biblionumber=76
3) Click "Edit" -> "Edit record" -> "Save"
4) Open the following URL's, preferably on 4 different browser tabs (we're using biblio 76 for this test):
<staff_url>/cgi-bin/koha/catalogue/search.pl?q=music (first record [76] is the one we're testing)
<staff_url>/cgi-bin/koha/catalogue/detail.pl?biblionumber=76
<opac_url>/cgi-bin/koha/opac-search.pl?idx=&q=music (first record [76] is the one we're testing)
<opac_url>/cgi-bin/koha/opac-detail.pl?biblionumber=76
5) Verify that on all pages there are 3 instances of 856 (Online resources) and 3 instances of 857 (Online archive resources) with their respective link labels and URLs.
6) Click on the "Koha community no protocol" link for 856 and 857, notice the 'http://' protocol is added.
7) Change the values of OPACDisplay856uAsImage + Display856uAsImage and refresh the respective page. Verify it works as expected (respects the system preference). Note: This only applies for 856, not 857 (out of scope here).
8) Change the value of OPACURLOpenInNewWindow and click on the links before and after. Verify they respect the system preference.
Comment 19 Pedro Amorim 2026-02-03 12:55:25 UTC
Created attachment 192380 [details] [review]
Bug 38096: [DONT PUSH] Add test data for fields 856 and 857
Comment 20 Pedro Amorim 2026-02-03 12:55:28 UTC
Created attachment 192381 [details] [review]
Bug 38096: (QA follow-up): Fix passing string param to template

Upon following original test plan after rebase, noticed that
OPACDisplay856uAsImage wasn't working for 'Details' or 'Results'.
The culprit was how each screen is invoking the template, passing the
string param in a wrong XSL way.
Comment 21 Jonathan Druart 2026-02-03 14:20:00 UTC
Marcel, are you willing to QA this one? :)

My hands start shaking when I see xslt...
Comment 22 Marcel de Rooy 2026-02-03 14:26:09 UTC
(In reply to Jonathan Druart from comment #21)
> Marcel, are you willing to QA this one? :)
> 
> My hands start shaking when I see xslt...

Sure.
Comment 23 Pedro Amorim 2026-02-03 16:31:56 UTC
I've identified an issue with the first patch Bug 38096: OPAC: Preparation.
The rebase is not correct. Looking again.
Comment 24 Pedro Amorim 2026-02-03 16:42:01 UTC
False alarm, all appears well.
The first patch appears to be "removing" functionality but it's not. It's DRYing the code as it was effectively duplicated for with and without OPACURLOpenInNewWindow.
This DRY work puts MARC21slim2OPACResults in line with MARC21slim2OPACDetail ensuring that the following patches converging the two do not introduce new regressions.
Comment 25 Marcel de Rooy 2026-02-06 09:04:35 UTC
(In reply to Pedro Amorim from comment #14)
> Patch no longer applies, updating here to come back to this later.

You removed all previous signoff lines..
Comment 26 Marcel de Rooy 2026-02-06 09:06:21 UTC
(In reply to Pedro Amorim from comment #18)
> 1) If testing k-t-d, add the test data for test plan (skip if sandboxes):
> updatedatabase
Hmm. Dont like such test plans as a member of the non-ktd minority..
Comment 27 Pedro Amorim 2026-02-06 09:35:56 UTC
Created attachment 192589 [details] [review]
Bug 38096: OPAC: Preparation

The OPAC logic for 856 in results page and detail page has diverged.
This commit ensures the 856 in the OPAC detail and search results pages
are consistent, in preparation to become DRY in the following commits.

With this DRY refactor, the following divergences are now aligned:
- 856 block now calls AddMissingProtocol regardless of value OPACURLOpenInNewWindow (previously was only being called if OPACURLOpenInNewWindow was 0). Now it's in line with what exists in MARC21slim2OPACDetail.xsl
- substring(,1,6)='image/' is also now a consideration when OPACURLOpenInNewWindow is 1. (previously was only being called if OPACURLOpenInNewWindow was 0). This is now in line with that exists in MARC21slim2OPACDetail.xsl

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 28 Pedro Amorim 2026-02-06 09:35:59 UTC
Created attachment 192590 [details] [review]
Bug 38096: OPAC: Move 856 logic to renderMARCOnlineResourceField utils

Now that OPAC detail and search results are consitent, we can DRY this
logic to its own function to be also utilized by 857 in the following
commit

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 29 Pedro Amorim 2026-02-06 09:36:02 UTC
Created attachment 192591 [details] [review]
Bug 38096: OPAC: Display 857 on detail and results pages

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 30 Pedro Amorim 2026-02-06 09:36:04 UTC
Created attachment 192592 [details] [review]
Bug 38096: Staff: Display 857 to detail and results pages

This work DRYs the 856 display logic to renderMARCOnlineResourceField.
This was previously written in detail and results pages.

Once this single function exists, 857 can also make use of it to be
displayed using the same logic/features already present for 856.

1) If testing k-t-d, add the test data for test plan (otherwise prep instructions below):
updatedatabase
2) Force reindex of this biblio by saving it, visit the biblio:
<staff_url>/cgi-bin/koha/catalogue/detail.pl?biblionumber=76
3) Click "Edit" -> "Edit record" -> "Save"
4) Open the following URL's, preferably on 4 different browser tabs (we're using biblio 76 for this test):
<staff_url>/cgi-bin/koha/catalogue/search.pl?q=music (first record [76] is the one we're testing)
<staff_url>/cgi-bin/koha/catalogue/detail.pl?biblionumber=76
<opac_url>/cgi-bin/koha/opac-search.pl?idx=&q=music (first record [76] is the one we're testing)
<opac_url>/cgi-bin/koha/opac-detail.pl?biblionumber=76
5) Verify that on all pages there are 3 instances of 856 (Online resources) and 3 instances of 857 (Online archive resources) with their respective link labels and URLs.
6) Click on the "Koha community no protocol" link for 856 and 857, notice the 'http://' protocol is added.
7) Change the values of OPACDisplay856uAsImage + Display856uAsImage and refresh the respective page. Verify it works as expected (respects the system preference). Note: This only applies for 856, not 857 (out of scope here).
8) Change the value of OPACURLOpenInNewWindow and click on the links before and after. Verify they respect the system preference.

If not on k-t-d and updatedatabase not available:

perl /kohadevbox/koha/installer/data/mysql/updatedatabase.pl
or
koha-shell -c "perl /kohadevbox/koha/installer/data/mysql/updatedatabase.pl" <instance_name>

If CLI not available (preparation on UI):
A) Add field 857 to the 'Default' framework
B) Edit the field:
<staff_url>/cgi-bin/koha/admin/marc_subfields_structure.pl?op=add_form&tagfield=857&frameworkcode=
C) Check the 'Editor' checkbox for subfields q, u and z (Add these if not there already)
D) Edit the record in question, visit:
<staff_url>/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=76
E) Click 'Settings' dropdown and pick 'Default' framework.
F) Add MARC data that corresponds to the following:
3 instances of 856 as below. 3 instances of 857 as below.

  <datafield tag="856" ind1="4" ind2="2">
    <subfield code="z">Koha community no protocol</subfield>
    <subfield code="u">koha-community.org/</subfield>
  </datafield>
  <datafield tag="856" ind1="4" ind2="2">
    <subfield code="z">Koha community with protocol</subfield>
    <subfield code="u">https://koha-community.org/</subfield>
  </datafield>
  <datafield tag="856" ind1="4" ind2="2">
    <subfield code="z">Koha logo image</subfield>
    <subfield code="q">jpeg</subfield>
    <subfield code="u">https://koha-community.org/files/2013/09/cropped-kohabanner3.jpg</subfield>
  </datafield>
  <datafield tag="857" ind1="4" ind2="2">
    <subfield code="z">Koha community no protocol</subfield>
    <subfield code="u">koha-community.org/</subfield>
  </datafield>
  <datafield tag="857" ind1="4" ind2="2">
    <subfield code="z">Koha community with protocol</subfield>
    <subfield code="u">https://koha-community.org/</subfield>
  </datafield>
  <datafield tag="857" ind1="4" ind2="2">
    <subfield code="z">KohaCon 2024 logo image</subfield>
    <subfield code="q">jpeg</subfield>
    <subfield code="u">https://2024.kohacon.org/wp-content/uploads/2024/01/kohacon24_fr_verticale_vert_noir.png</subfield>
  </datafield>
G) At this point you're ready to start the original test plan from step 4)

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 31 Pedro Amorim 2026-02-06 09:36:06 UTC
Created attachment 192593 [details] [review]
Bug 38096: [DONT PUSH] Add test data for fields 856 and 857
Comment 32 Pedro Amorim 2026-02-06 09:36:09 UTC
Created attachment 192594 [details] [review]
Bug 38096: (QA follow-up): Fix passing string param to template

Upon following original test plan after rebase, noticed that
OPACDisplay856uAsImage wasn't working for 'Details' or 'Results'.
The culprit was how each screen is invoking the template, passing the
string param in a wrong XSL way.
Comment 33 Marcel de Rooy 2026-02-06 09:36:58 UTC
Please note that I am still working on this. So we are now on parallel tracks :)
Comment 34 Pedro Amorim 2026-02-06 09:42:22 UTC
(In reply to Marcel de Rooy from comment #33)
> Please note that I am still working on this. So we are now on parallel
> tracks :)

Hi Marcel, appreciate your time here. My new patches are the same, just with signed-off lines added and an updated test plan in case not using k-t-d.
Comment 35 Marcel de Rooy 2026-02-06 10:24:14 UTC
Great work. I like the improvement to consolidate code in the Utils. Looks good to me.

Compare opac and staff Utils addition
diff koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slimUtils.xsl koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl
Too much differences. At some point we have let go here and left the idea of keeping those two more or less in sync. This patch set adds changes too.
No blocker. Lets continue.

koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2intranetDetail.xsl
koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2intranetResults.xsl
koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slim2OPACDetail.xsl
koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slim2OPACResults.xsl
Indentation of those new 856 and 857 blocks is a bit messy. Please correct.
Could we perhaps squash some patches?

Changing status for minor correction.
Comment 36 Pedro Amorim 2026-02-09 15:37:53 UTC
Doesn't apply. This time because of bug 41588 (maybe more). Working on it.
Comment 37 Pedro Amorim 2026-02-09 16:03:44 UTC
Created attachment 192813 [details] [review]
Bug 38096: OPAC: Preparation

The OPAC logic for 856 in results page and detail page has diverged.
This commit ensures the 856 in the OPAC detail and search results pages
are consistent, in preparation to become DRY in the following commits.

With this DRY refactor, the following divergences are now aligned:
- 856 block now calls AddMissingProtocol regardless of value OPACURLOpenInNewWindow (previously was only being called if OPACURLOpenInNewWindow was 0). Now it's in line with what exists in MARC21slim2OPACDetail.xsl
- substring(,1,6)='image/' is also now a consideration when OPACURLOpenInNewWindow is 1. (previously was only being called if OPACURLOpenInNewWindow was 0). This is now in line with that exists in MARC21slim2OPACDetail.xsl

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 38 Pedro Amorim 2026-02-09 16:03:47 UTC
Created attachment 192814 [details] [review]
Bug 38096: OPAC: Move 856 logic to renderMARCOnlineResourceField utils

Now that OPAC detail and search results are consitent, we can DRY this
logic to its own function to be also utilized by 857 in the following
commit

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 39 Pedro Amorim 2026-02-09 16:03:49 UTC
Created attachment 192815 [details] [review]
Bug 38096: OPAC: Display 857 on detail and results pages

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 40 Pedro Amorim 2026-02-09 16:03:52 UTC
Created attachment 192816 [details] [review]
Bug 38096: Staff: Display 857 to detail and results pages

This work DRYs the 856 display logic to renderMARCOnlineResourceField.
This was previously written in detail and results pages.

Once this single function exists, 857 can also make use of it to be
displayed using the same logic/features already present for 856.

1) If testing k-t-d, add the test data for test plan (otherwise prep instructions below):
updatedatabase
2) Force reindex of this biblio by saving it, visit the biblio:
<staff_url>/cgi-bin/koha/catalogue/detail.pl?biblionumber=76
3) Click "Edit" -> "Edit record" -> "Save"
4) Open the following URL's, preferably on 4 different browser tabs (we're using biblio 76 for this test):
<staff_url>/cgi-bin/koha/catalogue/search.pl?q=music (first record [76] is the one we're testing)
<staff_url>/cgi-bin/koha/catalogue/detail.pl?biblionumber=76
<opac_url>/cgi-bin/koha/opac-search.pl?idx=&q=music (first record [76] is the one we're testing)
<opac_url>/cgi-bin/koha/opac-detail.pl?biblionumber=76
5) Verify that on all pages there are 3 instances of 856 (Online resources) and 3 instances of 857 (Online archive resources) with their respective link labels and URLs.
6) Click on the "Koha community no protocol" link for 856 and 857, notice the 'http://' protocol is added.
7) Change the values of OPACDisplay856uAsImage + Display856uAsImage and refresh the respective page. Verify it works as expected (respects the system preference). Note: This only applies for 856, not 857 (out of scope here).
8) Change the value of OPACURLOpenInNewWindow and click on the links before and after. Verify they respect the system preference.

If not on k-t-d and updatedatabase not available:

perl /kohadevbox/koha/installer/data/mysql/updatedatabase.pl
or
koha-shell -c "perl /kohadevbox/koha/installer/data/mysql/updatedatabase.pl" <instance_name>

If CLI not available (preparation on UI):
A) Add field 857 to the 'Default' framework
B) Edit the field:
<staff_url>/cgi-bin/koha/admin/marc_subfields_structure.pl?op=add_form&tagfield=857&frameworkcode=
C) Check the 'Editor' checkbox for subfields q, u and z (Add these if not there already)
D) Edit the record in question, visit:
<staff_url>/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=76
E) Click 'Settings' dropdown and pick 'Default' framework.
F) Add MARC data that corresponds to the following:
3 instances of 856 as below. 3 instances of 857 as below.

  <datafield tag="856" ind1="4" ind2="2">
    <subfield code="z">Koha community no protocol</subfield>
    <subfield code="u">koha-community.org/</subfield>
  </datafield>
  <datafield tag="856" ind1="4" ind2="2">
    <subfield code="z">Koha community with protocol</subfield>
    <subfield code="u">https://koha-community.org/</subfield>
  </datafield>
  <datafield tag="856" ind1="4" ind2="2">
    <subfield code="z">Koha logo image</subfield>
    <subfield code="q">jpeg</subfield>
    <subfield code="u">https://koha-community.org/files/2013/09/cropped-kohabanner3.jpg</subfield>
  </datafield>
  <datafield tag="857" ind1="4" ind2="2">
    <subfield code="z">Koha community no protocol</subfield>
    <subfield code="u">koha-community.org/</subfield>
  </datafield>
  <datafield tag="857" ind1="4" ind2="2">
    <subfield code="z">Koha community with protocol</subfield>
    <subfield code="u">https://koha-community.org/</subfield>
  </datafield>
  <datafield tag="857" ind1="4" ind2="2">
    <subfield code="z">KohaCon 2024 logo image</subfield>
    <subfield code="q">jpeg</subfield>
    <subfield code="u">https://2024.kohacon.org/wp-content/uploads/2024/01/kohacon24_fr_verticale_vert_noir.png</subfield>
  </datafield>
G) At this point you're ready to start the original test plan from step 4)

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 41 Pedro Amorim 2026-02-09 16:03:55 UTC
Created attachment 192817 [details] [review]
Bug 38096: [DONT PUSH] Add test data for fields 856 and 857
Comment 42 Pedro Amorim 2026-02-09 16:03:58 UTC
Created attachment 192818 [details] [review]
Bug 38096: (QA follow-up): Fix passing string param to template

Upon following original test plan after rebase, noticed that
OPACDisplay856uAsImage wasn't working for 'Details' or 'Results'.
The culprit was how each screen is invoking the template, passing the
string param in a wrong XSL way.
Comment 43 Pedro Amorim 2026-02-09 16:04:01 UTC
Created attachment 192819 [details] [review]
Bug 38096: (QA follow-up): Fix indentation
Comment 44 Pedro Amorim 2026-02-09 16:09:26 UTC
Rebased and retested with test plan to ensure all works as before and recent changes from bug 41588 have moved over.

(In reply to Marcel de Rooy from comment #35)
> koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2intranetDetail.xsl
> koha-tmpl/intranet-tmpl/prog/en/xslt/MARC21slim2intranetResults.xsl
> koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slim2OPACDetail.xsl
> koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slim2OPACResults.xsl
> Indentation of those new 856 and 857 blocks is a bit messy. Please correct.
> Could we perhaps squash some patches?
> 
> Changing status for minor correction.

Marcel, thank you, submitted follow-up to fix indentation.
I'm on board with squashing the patches, I organized them that way initially to ease the review (i.e. make the 'change path' more clear). If you agree, I'll leave to your best judgement in regards to squashing (I'm on board with whatever you decide). Thank you once again.
Comment 45 Marcel de Rooy 2026-02-10 15:34:08 UTC
Created attachment 192906 [details] [review]
Bug 38096: OPAC: Preparation

The OPAC logic for 856 in results page and detail page has diverged.
This commit ensures the 856 in the OPAC detail and search results pages
are consistent, in preparation to become DRY in the following commits.

With this DRY refactor, the following divergences are now aligned:
- 856 block now calls AddMissingProtocol regardless of value OPACURLOpenInNewWindow (previously was only being called if OPACURLOpenInNewWindow was 0). Now it's in line with what exists in MARC21slim2OPACDetail.xsl
- substring(,1,6)='image/' is also now a consideration when OPACURLOpenInNewWindow is 1. (previously was only being called if OPACURLOpenInNewWindow was 0). This is now in line with that exists in MARC21slim2OPACDetail.xsl

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 46 Marcel de Rooy 2026-02-10 15:34:11 UTC
Created attachment 192907 [details] [review]
Bug 38096: OPAC: Move 856 logic to renderMARCOnlineResourceField utils

Now that OPAC detail and search results are consitent, we can DRY this
logic to its own function to be also utilized by 857 in the following
commit

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 47 Marcel de Rooy 2026-02-10 15:34:13 UTC
Created attachment 192908 [details] [review]
Bug 38096: OPAC: Display 857 on detail and results pages

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 48 Marcel de Rooy 2026-02-10 15:34:16 UTC
Created attachment 192909 [details] [review]
Bug 38096: Staff: Display 857 to detail and results pages

This work DRYs the 856 display logic to renderMARCOnlineResourceField.
This was previously written in detail and results pages.

Once this single function exists, 857 can also make use of it to be
displayed using the same logic/features already present for 856.

1) If testing k-t-d, add the test data for test plan (otherwise prep instructions below):
updatedatabase
2) Force reindex of this biblio by saving it, visit the biblio:
<staff_url>/cgi-bin/koha/catalogue/detail.pl?biblionumber=76
3) Click "Edit" -> "Edit record" -> "Save"
4) Open the following URL's, preferably on 4 different browser tabs (we're using biblio 76 for this test):
<staff_url>/cgi-bin/koha/catalogue/search.pl?q=music (first record [76] is the one we're testing)
<staff_url>/cgi-bin/koha/catalogue/detail.pl?biblionumber=76
<opac_url>/cgi-bin/koha/opac-search.pl?idx=&q=music (first record [76] is the one we're testing)
<opac_url>/cgi-bin/koha/opac-detail.pl?biblionumber=76
5) Verify that on all pages there are 3 instances of 856 (Online resources) and 3 instances of 857 (Online archive resources) with their respective link labels and URLs.
6) Click on the "Koha community no protocol" link for 856 and 857, notice the 'http://' protocol is added.
7) Change the values of OPACDisplay856uAsImage + Display856uAsImage and refresh the respective page. Verify it works as expected (respects the system preference). Note: This only applies for 856, not 857 (out of scope here).
8) Change the value of OPACURLOpenInNewWindow and click on the links before and after. Verify they respect the system preference.

If not on k-t-d and updatedatabase not available:

perl /kohadevbox/koha/installer/data/mysql/updatedatabase.pl
or
koha-shell -c "perl /kohadevbox/koha/installer/data/mysql/updatedatabase.pl" <instance_name>

If CLI not available (preparation on UI):
A) Add field 857 to the 'Default' framework
B) Edit the field:
<staff_url>/cgi-bin/koha/admin/marc_subfields_structure.pl?op=add_form&tagfield=857&frameworkcode=
C) Check the 'Editor' checkbox for subfields q, u and z (Add these if not there already)
D) Edit the record in question, visit:
<staff_url>/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=76
E) Click 'Settings' dropdown and pick 'Default' framework.
F) Add MARC data that corresponds to the following:
3 instances of 856 as below. 3 instances of 857 as below.

  <datafield tag="856" ind1="4" ind2="2">
    <subfield code="z">Koha community no protocol</subfield>
    <subfield code="u">koha-community.org/</subfield>
  </datafield>
  <datafield tag="856" ind1="4" ind2="2">
    <subfield code="z">Koha community with protocol</subfield>
    <subfield code="u">https://koha-community.org/</subfield>
  </datafield>
  <datafield tag="856" ind1="4" ind2="2">
    <subfield code="z">Koha logo image</subfield>
    <subfield code="q">jpeg</subfield>
    <subfield code="u">https://koha-community.org/files/2013/09/cropped-kohabanner3.jpg</subfield>
  </datafield>
  <datafield tag="857" ind1="4" ind2="2">
    <subfield code="z">Koha community no protocol</subfield>
    <subfield code="u">koha-community.org/</subfield>
  </datafield>
  <datafield tag="857" ind1="4" ind2="2">
    <subfield code="z">Koha community with protocol</subfield>
    <subfield code="u">https://koha-community.org/</subfield>
  </datafield>
  <datafield tag="857" ind1="4" ind2="2">
    <subfield code="z">KohaCon 2024 logo image</subfield>
    <subfield code="q">jpeg</subfield>
    <subfield code="u">https://2024.kohacon.org/wp-content/uploads/2024/01/kohacon24_fr_verticale_vert_noir.png</subfield>
  </datafield>
G) At this point you're ready to start the original test plan from step 4)

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 49 Marcel de Rooy 2026-02-10 15:34:19 UTC
Created attachment 192910 [details] [review]
Bug 38096: (QA follow-up): Fix passing string param to template

Upon following original test plan after rebase, noticed that
OPACDisplay856uAsImage wasn't working for 'Details' or 'Results'.
The culprit was how each screen is invoking the template, passing the
string param in a wrong XSL way.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 50 Marcel de Rooy 2026-02-10 15:34:21 UTC
Created attachment 192911 [details] [review]
Bug 38096: (QA follow-up): Fix indentation

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 51 Lucas Gass (lukeg) 2026-02-10 17:53:54 UTC
Nice work everyone!

Pushed to main for 26.05
Comment 52 Pedro Amorim 2026-02-10 18:13:29 UTC
Created attachment 192919 [details] [review]
Bug 38096: (QA follow-up): Fix URLLinkText
Comment 53 Pedro Amorim 2026-02-10 18:13:32 UTC
Created attachment 192920 [details] [review]
Bug 38096: (follow-up) Fix missing biblionumber parameter in renderMARCOnlineResourceField

The renderMARCOnlineResourceField template uses $biblionumber for the
TrackClicks feature but this parameter was never declared in the template
definition nor passed by any of the call sites.

This caused XSLT runtime errors when TrackClicks system preference is
enabled ('track' or 'anonymous') and records contain MARC fields 856 or
857 with subfield $u (URLs):

  Variable 'biblionumber' has not been declared.
  Undefined variable
  XPath evaluation returned no result.

This patch:
1. Adds biblionumber parameter to renderMARCOnlineResourceField template
2. Passes biblionumber value in all 4 call sites (2 in Detail, 2 in Results)

Test plan:
1. Enable TrackClicks: System Preferences > OPAC > TrackClicks = "track"
2. Find a bibliographic record with field 856 or 857 containing subfield $u
3. Run: yarn build (to regenerate XSLT for all languages)
4. View the record in OPAC detail and search results pages
5. Verify no XSLT errors in plack-opac-error.log
6. Verify online resource links display correctly
7. Verify links include biblionumber parameter in tracklinks.pl URLs
Comment 54 Pedro Amorim 2026-02-10 18:15:14 UTC
Lucas these 2 follow-ups are very important and are fixing 2 major oversights from the original patch.
Things will blow up as described in Martin's commit.
The short of it is, the DRY work missed some variables that are only utilized in specific conditions.
Comment 55 Pedro Amorim 2026-02-10 18:21:44 UTC
Created attachment 192921 [details] [review]
Bug 38096: (QA follow-up): Fix URLLinkText
Comment 56 Pedro Amorim 2026-02-10 18:21:47 UTC
Created attachment 192922 [details] [review]
Bug 38096: (follow-up) Fix missing biblionumber parameter in renderMARCOnlineResourceField

The renderMARCOnlineResourceField template uses $biblionumber for the
TrackClicks feature but this parameter was never declared in the template
definition nor passed by any of the call sites.

This caused XSLT runtime errors when TrackClicks system preference is
enabled ('track' or 'anonymous') and records contain MARC fields 856 or
857 with subfield $u (URLs):

  Variable 'biblionumber' has not been declared.
  Undefined variable
  XPath evaluation returned no result.

This patch:
1. Adds biblionumber parameter to renderMARCOnlineResourceField template
2. Passes biblionumber value in all 4 call sites (2 in Detail, 2 in Results)

Test plan:
1. Enable TrackClicks: System Preferences > OPAC > TrackClicks = "track"
2. Find a bibliographic record with field 856 or 857 containing subfield $u
3. Run: yarn build (to regenerate XSLT for all languages)
4. View the record in OPAC detail and search results pages
5. Verify no XSLT errors in plack-opac-error.log
6. Verify online resource links display correctly
7. Verify links include biblionumber parameter in tracklinks.pl URLs
Comment 57 Pedro Amorim 2026-02-10 18:22:07 UTC
Rebased to apply nicely due to tidy.
Comment 58 Lucas Gass (lukeg) 2026-02-10 22:49:06 UTC
follow-ups pushed to main
Comment 59 Jonathan Druart 2026-02-11 08:15:10 UTC
t/db_dependent/Search.t is failing, I guess it's coming from here.

02:30:02 koha-1      | #     XSLT::Base: runtime error: file /kohadevbox/koha/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl line 730 element choose
02:30:02 koha-1      | # Variable 'URLLinkText' has not been declared.
02:30:02 koha-1      | # Undefined variable
Comment 60 Pedro Amorim 2026-02-11 08:47:24 UTC
(In reply to Jonathan Druart from comment #59)
> t/db_dependent/Search.t is failing, I guess it's coming from here.
> 
> 02:30:02 koha-1      | #     XSLT::Base: runtime error: file
> /kohadevbox/koha/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl
> line 730 element choose
> 02:30:02 koha-1      | # Variable 'URLLinkText' has not been declared.
> 02:30:02 koha-1      | # Undefined variable

The last 2 patches should've fixed that. Is this after the last 2 follow-ups?
Comment 61 Pedro Amorim 2026-02-11 09:20:50 UTC
Looking into it, the URLLinkText intra fix is also required on the opac MARC21slimUtils.xsl
Comment 62 Pedro Amorim 2026-02-11 09:27:39 UTC
Created attachment 192966 [details] [review]
Bug 38096: (QA follow-up): Add URLLinkText to opac xsl also

Fix error:
[2026/02/11 09:18:00] [WARN] XSLT::Base: runtime error: file /usr/share/koha/opac/htdocs/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl line 726 element choose
Variable 'URLLinkText' has not been declared.
Undefined variable
 at /usr/share/koha/lib/Koha/XSLT/Base.pm line 231.

Run tests to prove the patch fixes the issue (this fails before the patch):
prove t/db_dependent/Search.t
Comment 63 Pedro Amorim 2026-02-11 09:28:12 UTC
(In reply to Jonathan Druart from comment #59)
> t/db_dependent/Search.t is failing, I guess it's coming from here.
> 
> 02:30:02 koha-1      | #     XSLT::Base: runtime error: file
> /kohadevbox/koha/koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl
> line 730 element choose
> 02:30:02 koha-1      | # Variable 'URLLinkText' has not been declared.
> 02:30:02 koha-1      | # Undefined variable

Last patch fixes it please verify.
Comment 64 Pedro Amorim 2026-02-11 11:02:56 UTC
Adding 25.11.x release candidate here for the following reasons:
- Not showing 857 on the search results + detail pages may arguably be considered a bug by some as that field has been included on the DEFAULT framework since 23.11 (bug 34667).
- Back porting fixes on top of this rewrite will become increasingly difficult if not back ported.
Comment 65 Jonathan Druart 2026-02-12 09:07:01 UTC
Created attachment 193000 [details] [review]
Bug 38096: (QA follow-up): Add URLLinkText to opac xsl also

Fix error:
[2026/02/11 09:18:00] [WARN] XSLT::Base: runtime error: file /usr/share/koha/opac/htdocs/opac-tmpl/bootstrap/en/xslt/MARC21slimUtils.xsl line 726 element choose
Variable 'URLLinkText' has not been declared.
Undefined variable
 at /usr/share/koha/lib/Koha/XSLT/Base.pm line 231.

Run tests to prove the patch fixes the issue (this fails before the patch):
prove t/db_dependent/Search.t

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 66 Lucas Gass (lukeg) 2026-02-13 14:24:12 UTC
follow-up pushed to main