Info about 856
https://www.loc.gov/marc/bibliographic/bd856.html

856$h Non-functioning Uniform Resource Identifier
Uniform Resource Identifier (URI), which is no longer functional, for example, due to link rot, content drift, etc.
Subfield $h may be repeated if there is more than one non-functioning URI. A note on the status change (including the date) may be added in either in subfield $x or subfield $z, depending on the local policy.

I also tried only filling out 856$s (file size) and the link appears.

I was able to replicate this and IMO it's bad--very, very bad!  I even had "Is a URL:" in the Framework unchecked, and a clickable "Click here to access online" was still displayed.

I would completely hide this subfield in my OPAC *and* staff client, frankly!

(The label for the field should also be changed from "Processor of request" to "Non-functioning URI" to conform to the MARC21 standard. IMO.)
--h2

(In reply to Heather from comment #3)

> (The label for the field should also be changed from "Processor of request"
> to "Non-functioning URI" to conform to the MARC21 standard. IMO.)

This part was done in bug 34665. It's just that we don't update frameworks for existing installations.

(In reply to Heather from comment #3)

> I would completely hide this subfield in my OPAC *and* staff client, frankly!

I'm using this field for a migration where all the links have to be changed (the client is also changing their website at the same time, so the links won't work anymore). I'm using 856$h to store the "old" link. I think it's interesting to see it in the staff interface for that case, without having to go "edit" the record. But like we both mentioned, it should not trigger the link display.

That's a great use of the field, Caroline--thank you for mentioning it!  (And that the frameworks aren't updated for existing installations--alerts me to some changes I might want to make!)

I could see this subfield being useful to use for staff access to a site vs. patron/public access, so it would be good to have this subfield display only in the staff client.

I filed bug 36123 with my somewhat clumsy explanation on a related problem with configuring visibility for the 856 $u in the OPAC. The larger question is if 856$u is hidden, should any other fields with data still function as a link? I think no, especially if the URL checkbox is not checked for the other fields.

Created attachment 162406 [details] [review]
Bug 36111: 856$h should not appear as a link in detailed record

The newish MARC21 field 856$h is used to store non-functioning
URLs. However, if a record has data in 856$h (or any 856
subfield), a link "Click here to access online" appears in the
search results list and detailed record in the OPAC and the
staff interface.

This patch fixes this so that URLs in 856$h are not displayed
in the search results and record details pages.

Test plan:
1. Add 856$h to MARC editor
   1.1. Go to Administration > MARC bibliographic frameworks
   1.2. Next to Default framework, click Actions > MARC structure
   1.3. Search for field 856
   1.4. Click Actions > Edit subfields
   1.5. Click h
   1.6. Check the Editor box
   1.7. Click Save changes
2. Catalog a new record with a random URL in 856$h
   2.1. Go to Cataloging
   2.2. Click New record
   2.3. Fill out the mandatory fields (000, 003, 005, 008, 040$c,
        245$a, 942$c)
   2.4. Go to tab 8 and enter a value in 856$h
   2.5. Click Save (No need to add an item)
3. Search for the title in the staff interface (a large enough
   search to have more than one result)
   --> Notice it says "Online resources: Click here to access online"
   3.1. Try to click the link
        --> Blank page
4. Access the detailed record in the staff interface
   --> Notice it says "Online resources: Click here to access online"
   4.1. Try to click the link
        --> Blank page
5. Search for the title in the opac (a large enough search to have
   more than one result)
   --> Notice it says "Online resources: Click here to access online"
   5.1. Try to click the link
        --> Blank page
6. Access the detailed record in the opac
   --> Notice it says "Online resources: Click here to access online"
   6.1. Try to click the link
        --> Blank page
7. Apply the patch
8. Replay steps 3 through 6, but this time, the "Online resources"
   field should not be present.
9. Edit the item and add a url in field 856$u
10. Replay steps 3 through 6, but this time, clicking on the link
    should take you to the URL specified in the 856$u field.

Signed-off-by: David Nind <david@davidnind.com>

For some reason, when I signed off the patch the test plan was not included. I have amended the patch to include a description and have added the (very good) test plan. Feel free to amend if I have not got it right.

I have also added Matthias as the assignee, changed the importance from enhancement to normal (as it is a bug), and added some text for the release notes.

Testing notes using koha-testing-docker (KTD):

1. If using Firefox, instead of a blank page nothing happens. You get a blank page using a Chromium-based browser (such as Chromium or Google Chrome).

> For some reason, when I signed off the patch the test plan was not
> included. I have amended the patch to include a description and
> have added the (very good) test plan.

Thanks David for the help :)
 	
@Matthias I think that happens when the test plan is not written in the commit message but written in bugzilla's UI/git-bz when attaching a patch. The test plan needs to be in the commit message to be preserved between signoffs and in the Koha commit history.

---

If that can help someone to review this, here a git command to show the diff with additional colors for moved lines. To go more quickly and just checking that part of the stuff is just moved and not new or totally removed.

git show --color-moved --color-moved-ws=allow-indentation-change

That's was still not enough for me to be able to get a good grasp of this one, sorry ^^"

(In reply to Victor Grousset/tuxayo from comment #12)
> > For some reason, when I signed off the patch the test plan was not
> > included. I have amended the patch to include a description and
> > have added the (very good) test plan.
> 
> Thanks David for the help :)
>  	
> @Matthias I think that happens when the test plan is not written in the
> commit message but written in bugzilla's UI/git-bz when attaching a patch.
> The test plan needs to be in the commit message to be preserved between
> signoffs and in the Koha commit history.
> 
> ---
> 
> If that can help someone to review this, here a git command to show the diff
> with additional colors for moved lines. To go more quickly and just checking
> that part of the stuff is just moved and not new or totally removed.
> 
> git show --color-moved --color-moved-ws=allow-indentation-change
> 
> That's was still not enough for me to be able to get a good grasp of this
> one, sorry ^^"

thank for the help, I need to amend the commit to attach the test plan ?

No need, David did it when signing off :)

Hi :)

The second patch now duplicates the test plan of the 1st patch. That will cause confusion.

About the switch back to Needs Signoff: are the steps to test the additional changes the same? Or is there something additional to check about these latest changes?

The idea is that if someone would forget to apply the second patch, when testing, they should not get what is expected. Thus demonstrating that the test plan really tests the changes. If not, that means the test plan isn't complete enough.
No need to have manual tests showing every single change made by the patches. It's a balance to show enough of the functional changes and edge cases and to demonstrate that the related code works.

Ok I understand I will change the plan test to test the fix

Created attachment 164029 [details] [review]
Bug 36111: removes display of fields 856y3z if not 856u

Test plan:
1. Add 856$h to MARC editor
   1.1. Go to Administration > MARC bibliographic frameworks
   1.2. Next to Default framework, click Actions > MARC structure
   1.3. Search for field 856
   1.4. Click Actions > Edit subfields
   1.5. Click h
   1.6. Check the Editor box
   1.7. Click Save changes
2. Catalog a new record with a random URL in 856$h
   2.1. Go to Cataloging
   2.2. Click New record
   2.3. Fill out the mandatory fields (000, 003, 005, 008, 040$c,
        245$a, 942$c)
   2.4. Go to tab 8 and enter a value in 856$h
   2.5. Click Save (No need to add an item)
3. Search for the title in the staff interface (a large enough
   search to have more than one result)
   --> Notice it says "Online resources: Click here to access online"
   3.1. Try to click the link
        --> Blank page
4. Access the detailed record in the staff interface
   --> Notice it says "Online resources: Click here to access online"
   4.1. Try to click the link
        --> Blank page
5. Search for the title in the opac (a large enough search to have
   more than one result)
   --> Notice it says "Online resources: Click here to access online"
   5.1. Try to click the link
        --> Blank page
6. Access the detailed record in the opac
   --> Notice it says "Online resources: Click here to access online"
   6.1. Try to click the link
        --> Blank page
7. Edit the item and add a text in field 856$y
8. Access the detailed record in the staff interface
   --> Notice it says what you put in 856$y field
9. Apply the patch
10. Replay steps 3 through 6, but this time, the "Online resources"
   field should not be present.
11. Replay steps 7 through 8, but this time, what you put in 856$y
   field should not be present.
12. Edit the item and add a url in field 856$u
13. Replay steps 3 through 6, but this time, clicking on the link
    should take you to the URL specified in the 856$u field.
14. Replay steps 7 through 8, but this time, what you put in 856$y
   field should be present and 
   clicking on the link should take you to the URL specified in the 856$u field.
   
Before the second patch, if you put something in the 856$y field and there was nothing in the 856$u field, the text was displayed. 
Now, with the second patch, the problem has been solved.

Thanks for the additional steps :)
The back to need signoff now is clear, there is additional behavior to test.

Remaining is the issue of the 1st patch having the old test plan. So there is duplication and someone coming to test is likely to miss the full test plan in the 2nd patch. And signoff without having actually tested the last changes.

Created attachment 164151 [details] [review]
Bug 36111: removes display of fields 856y3z if not 856u

Test plan:
1. Add 856$h to MARC editor
   1.1. Go to Administration > MARC bibliographic frameworks
   1.2. Next to Default framework, click Actions > MARC structure
   1.3. Search for field 856
   1.4. Click Actions > Edit subfields
   1.5. Click h
   1.6. Check the Editor box
   1.7. Click Save changes
2. Catalog a new record with a random URL in 856$h
   2.1. Go to Cataloging
   2.2. Click New record
   2.3. Fill out the mandatory fields (000, 003, 005, 008, 040$c,
        245$a, 942$c)
   2.4. Go to tab 8 and enter a value in 856$h
   2.5. Click Save (No need to add an item)
3. Search for the title in the staff interface (a large enough
   search to have more than one result)
   --> Notice it says "Online resources: Click here to access online"
   3.1. Try to click the link
        --> Blank page
4. Access the detailed record in the staff interface
   --> Notice it says "Online resources: Click here to access online"
   4.1. Try to click the link
        --> Blank page
5. Search for the title in the opac (a large enough search to have
   more than one result)
   --> Notice it says "Online resources: Click here to access online"
   5.1. Try to click the link
        --> Blank page
6. Access the detailed record in the opac
   --> Notice it says "Online resources: Click here to access online"
   6.1. Try to click the link
        --> Blank page
7. Edit the item and add a text in field 856$y
8. Access the detailed record in the staff interface
   --> Notice it says what you put in 856$y field
9. Apply the patch
10. Replay steps 3 through 6, but this time, the "Online resources"
   field should not be present.
11. Replay steps 7 through 8, but this time, what you put in 856$y
   field should not be present.
12. Edit the item and add a url in field 856$u
13. Replay steps 3 through 6, but this time, clicking on the link
    should take you to the URL specified in the 856$u field.
14. Replay steps 7 through 8, but this time, what you put in 856$y
   field should be present and
   clicking on the link should take you to the URL specified in the 856$u field.

I put the test plan in the commit message
It's better now?

Created attachment 164275 [details] [review]
Bug 36111: 856$h should not appear as a link in detailed record

The newish MARC21 field 856$h is used to store non-functioning
URLs. However, if a record has data in 856$h (or any 856
subfield), a link "Click here to access online" appears in the
search results list and detailed record in the OPAC and the
staff interface.

This patch fixes this so that URLs in 856$h are not displayed
in the search results and record details pages.

Test plan:
1. Add 856$h to MARC editor
   1.1. Go to Administration > MARC bibliographic frameworks
   1.2. Next to Default framework, click Actions > MARC structure
   1.3. Search for field 856
   1.4. Click Actions > Edit subfields
   1.5. Click h
   1.6. Check the Editor box
   1.7. Click Save changes
2. Catalog a new record with a random URL in 856$h
   2.1. Go to Cataloging
   2.2. Click New record
   2.3. Fill out the mandatory fields (000, 003, 005, 008, 040$c,
        245$a, 942$c)
   2.4. Go to tab 8 and enter a value in 856$h
   2.5. Click Save (No need to add an item)
3. Search for the title in the staff interface (a large enough
   search to have more than one result)
   --> Notice it says "Online resources: Click here to access online"
   3.1. Try to click the link
        --> Blank page
4. Access the detailed record in the staff interface
   --> Notice it says "Online resources: Click here to access online"
   4.1. Try to click the link
        --> Blank page
5. Search for the title in the opac (a large enough search to have
   more than one result)
   --> Notice it says "Online resources: Click here to access online"
   5.1. Try to click the link
        --> Blank page
6. Access the detailed record in the opac
   --> Notice it says "Online resources: Click here to access online"
   6.1. Try to click the link
        --> Blank page
7. Apply the patch
8. Replay steps 3 through 6, but this time, the "Online resources"
   field should not be present.
9. Edit the item and add a url in field 856$u
10. Replay steps 3 through 6, but this time, clicking on the link
    should take you to the URL specified in the 856$u field.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Created attachment 164276 [details] [review]
Bug 36111: removes display of fields 856y3z if not 856u

Test plan:
1. Add 856$h to MARC editor
   1.1. Go to Administration > MARC bibliographic frameworks
   1.2. Next to Default framework, click Actions > MARC structure
   1.3. Search for field 856
   1.4. Click Actions > Edit subfields
   1.5. Click h
   1.6. Check the Editor box
   1.7. Click Save changes
2. Catalog a new record with a random URL in 856$h
   2.1. Go to Cataloging
   2.2. Click New record
   2.3. Fill out the mandatory fields (000, 003, 005, 008, 040$c,
        245$a, 942$c)
   2.4. Go to tab 8 and enter a value in 856$h
   2.5. Click Save (No need to add an item)
3. Search for the title in the staff interface (a large enough
   search to have more than one result)
   --> Notice it says "Online resources: Click here to access online"
   3.1. Try to click the link
        --> Blank page
4. Access the detailed record in the staff interface
   --> Notice it says "Online resources: Click here to access online"
   4.1. Try to click the link
        --> Blank page
5. Search for the title in the opac (a large enough search to have
   more than one result)
   --> Notice it says "Online resources: Click here to access online"
   5.1. Try to click the link
        --> Blank page
6. Access the detailed record in the opac
   --> Notice it says "Online resources: Click here to access online"
   6.1. Try to click the link
        --> Blank page
7. Edit the item and add a text in field 856$y
8. Access the detailed record in the staff interface
   --> Notice it says what you put in 856$y field
9. Apply the patch
10. Replay steps 3 through 6, but this time, the "Online resources"
   field should not be present.
11. Replay steps 7 through 8, but this time, what you put in 856$y
   field should not be present.
12. Edit the item and add a url in field 856$u
13. Replay steps 3 through 6, but this time, clicking on the link
    should take you to the URL specified in the 856$u field.
14. Replay steps 7 through 8, but this time, what you put in 856$y
   field should be present and
   clicking on the link should take you to the URL specified in the 856$u field.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

(In reply to Matthias Le Gac from comment #21)
> I put the test plan in the commit message

I missed that there was again the 1st mistake: so indeed the patch on comment 18 didn't have the test plan and it would have been lost on Owen's signoff. So thanks for fixing this. The most reliable approach is to not use the comment function when attaching patches. Everything related to the patch itself should be in the commit message. And additional comments should be written in a separate bugzilla comment.

> It's better now?

What I meant in comment 19 by duplication is that when looking at the latest version of :
- Bug 36111: 856$h should not appear as a link in detailed record
- Bug 36111: removes display of fields 856y3z if not 856u

they are 80% duplicate and it's the second one that is the complete one. So it's easy to miss.
To fix this, the full test plan should be put in the 1st commit. And it should be removed from the 2nd commit. So there is only one test plan and it's complete.
(there is a gotcha when doing back and forths for the 1st time with patches on bugzilla: grab the patches from bugzilla and not from your local branch before starting working on changes, to not loose the signoff that have been added here)

@Owen when you signed-off, did you use the full test plan that is in the second commit?

Created attachment 164429 [details] [review]
Bug 36111: 856$h should not appear as a link in detailed record

Test plan:
1. Add 856$h to MARC editor
   1.1. Go to Administration > MARC bibliographic frameworks
   1.2. Next to Default framework, click Actions > MARC structure
   1.3. Search for field 856
   1.4. Click Actions > Edit subfields
   1.5. Click h
   1.6. Check the Editor box
   1.7. Click Save changes
2. Catalog a new record with a random URL in 856$h
   2.1. Go to Cataloging
   2.2. Click New record
   2.3. Fill out the mandatory fields (000, 003, 005, 008, 040$c,
        245$a, 942$c)
   2.4. Go to tab 8 and enter a value in 856$h
   2.5. Click Save (No need to add an item)
3. Search for the title in the staff interface (a large enough
   search to have more than one result)
   --> Notice it says "Online resources: Click here to access online"
   3.1. Try to click the link
        --> Blank page
4. Access the detailed record in the staff interface
   --> Notice it says "Online resources: Click here to access online"
   4.1. Try to click the link
        --> Blank page
5. Search for the title in the opac (a large enough search to have
   more than one result)
   --> Notice it says "Online resources: Click here to access online"
   5.1. Try to click the link
        --> Blank page
6. Access the detailed record in the opac
   --> Notice it says "Online resources: Click here to access online"
   6.1. Try to click the link
        --> Blank page
7. Edit the item and add a text in field 856$y
8. Access the detailed record in the staff interface
   --> Notice it says what you put in 856$y field
9. Apply the patch
10. Replay steps 3 through 6, but this time, the "Online resources"
   field should not be present.
11. Replay steps 7 through 8, but this time, what you put in 856$y
   field should not be present.
12. Edit the item and add a url in field 856$u
13. Replay steps 3 through 6, but this time, clicking on the link
    should take you to the URL specified in the 856$u field.
14. Replay steps 7 through 8, but this time, what you put in 856$y
   field should be present and
   clicking on the link should take you to the URL specified in the 856$u field.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>


Current status: Signed Off

Created attachment 164430 [details] [review]
Bug 36111: removes display of fields 856y3z if not 856u

Current status: Signed Off

(In reply to Victor Grousset/tuxayo from comment #24)

I put the last test plan in the first commit and removed the test plan in the second commit.

Philippe Blouin has asked me to simplify my changes, so I'm passing the task back to Failed QA until I attach the changes, which will have no impact on the test plan.

Created attachment 164476 [details] [review]
Bug 36111: 856$h should not appear as a link in detailed record

Test plan:
1. Add 856$h to MARC editor
   1.1. Go to Administration > MARC bibliographic frameworks
   1.2. Next to Default framework, click Actions > MARC structure
   1.3. Search for field 856
   1.4. Click Actions > Edit subfields
   1.5. Click h
   1.6. Check the Editor box
   1.7. Click Save changes
2. Catalog a new record with a random URL in 856$h
   2.1. Go to Cataloging
   2.2. Click New record
   2.3. Fill out the mandatory fields (000, 003, 005, 008, 040$c,
        245$a, 942$c)
   2.4. Go to tab 8 and enter a value in 856$h
   2.5. Click Save (No need to add an item)
3. Search for the title in the staff interface (a large enough
   search to have more than one result)
   --> Notice it says "Online resources: Click here to access online"
   3.1. Try to click the link
        --> Blank page
4. Access the detailed record in the staff interface
   --> Notice it says "Online resources: Click here to access online"
   4.1. Try to click the link
        --> Blank page
5. Search for the title in the opac (a large enough search to have
   more than one result)
   --> Notice it says "Online resources: Click here to access online"
   5.1. Try to click the link
        --> Blank page
6. Access the detailed record in the opac
   --> Notice it says "Online resources: Click here to access online"
   6.1. Try to click the link
        --> Blank page
7. Edit the item and add a text in field 856$y
8. Access the detailed record in the staff interface
   --> Notice it says what you put in 856$y field
9. Apply the patch
10. Replay steps 3 through 6, but this time, the "Online resources"
   field should not be present.
11. Replay steps 7 through 8, but this time, what you put in 856$y
   field should not be present.
12. Edit the item and add a url in field 856$u
13. Replay steps 3 through 6, but this time, clicking on the link
    should take you to the URL specified in the 856$u field.
14. Replay steps 7 through 8, but this time, what you put in 856$y
   field should be present and
   clicking on the link should take you to the URL specified in the 856$u field.

When I had made my modifications in the first commit, I had indented the code, which generated a lot of differences with the code and didn't make it easy to understand where I had really made the modifications. So I redid the minimal changes, which had no impact on the test plan, and put them in the commit.

Do you confirm the commit/patch "Bug 36111: removes display of fields 856y3z if not 856u" isn't needed anymore?

Asking in case there was a confusion about removing the duplicate test plan: it was about amending the commit to remove the duplicate test plan. And reattach it here which would replace the previous version.

Here what happened is that the 2nd commit isn't here anymore. But if the rework also made the 2nd commit totally unnecessary then, all good :)

(In reply to Victor Grousset/tuxayo from comment #32)
> Do you confirm the commit/patch "Bug 36111: removes display of fields 856y3z
> if not 856u" isn't needed anymore?
> 
> Asking in case there was a confusion about removing the duplicate test plan:
> it was about amending the commit to remove the duplicate test plan. And
> reattach it here which would replace the previous version.
> 
> Here what happened is that the 2nd commit isn't here anymore. But if the
> rework also made the 2nd commit totally unnecessary then, all good :)

Yes, the commit/patch "Bug 36111 : suppresses the display of fields 856y3z if they are not 856u" is no longer useful because when I simplified my modifications I fussed the two commits to make it simpler and have a single commit/patch that has all the modifications with the least number of modified lines.
Sorry for the confusion.

Created attachment 165236 [details] [review]
Bug 36111: 856$h should not appear as a link in detailed record

Test plan:
1. Add 856$h to MARC editor
   1.1. Go to Administration > MARC bibliographic frameworks
   1.2. Next to Default framework, click Actions > MARC structure
   1.3. Search for field 856
   1.4. Click Actions > Edit subfields
   1.5. Click h
   1.6. Check the Editor box
   1.7. Click Save changes
2. Catalog a new record with a random URL in 856$h
   2.1. Go to Cataloging
   2.2. Click New record
   2.3. Fill out the mandatory fields (000, 003, 005, 008, 040$c,
        245$a, 942$c)
   2.4. Go to tab 8 and enter a value in 856$h
   2.5. Click Save (No need to add an item)
3. Search for the title in the staff interface (a large enough
   search to have more than one result)
   --> Notice it says "Online resources: Click here to access online"
   3.1. Try to click the link
        --> Blank page
4. Access the detailed record in the staff interface
   --> Notice it says "Online resources: Click here to access online"
   4.1. Try to click the link
        --> Blank page
5. Search for the title in the opac (a large enough search to have
   more than one result)
   --> Notice it says "Online resources: Click here to access online"
   5.1. Try to click the link
        --> Blank page
6. Access the detailed record in the opac
   --> Notice it says "Online resources: Click here to access online"
   6.1. Try to click the link
        --> Blank page
7. Edit the item and add a text in field 856$y
8. Access the detailed record in the staff interface
   --> Notice it says what you put in 856$y field
9. Apply the patch
10. Replay steps 3 through 6, but this time, the "Online resources"
   field should not be present.
11. Replay steps 7 through 8, but this time, what you put in 856$y
   field should not be present.
12. Edit the item and add a url in field 856$u
13. Replay steps 3 through 6, but this time, clicking on the link
    should take you to the URL specified in the 856$u field.
14. Replay steps 7 through 8, but this time, what you put in 856$y
   field should be present and
   clicking on the link should take you to the URL specified in the 856$u field.

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>

I had a go at testing, not realising it had already been signed off.

Everything works as per the test plan.

However, something is not quite right with the spacing between labels and information when the 856$h and $y are not displayed (after the patch is applied). 

It is very subtle (to replicate with patch applied, edit the book framework, use biblionumber 262 (Programming Perl), and searched for perl):

1. $856$h and $y: 
   - Staff interface - search results: extra space after 'Edition'
   - Staff interface - details page: extra space between 'LOC classification' and 'OPAC view' 
   - OPAC - search results: extra space between 'Publication detail' and 'Availability'
   - OPAC - details page: extra space between 'LOC classification' and 'Tags from this library'

2. $856$u and $y: displays correctly in search results and details page (for both the staff interface and OPAC)

If you use the element selector, you can see there is an extra green line (with different pixel sizes depending on location) with these classes:
- Staff interface - results and details page: span.results_summary.online_resources (6.5px)
- OPAC - results page: div.results_summary.online_resources (6.12px)
- OPAC - details page: span.results_summary.online_resources (6.8px)

Created attachment 165335 [details] [review]
Bug 36111: fixed extra space

I fixed the problem when "online resources" is not displayed. There is no longer an extra space between fields. 

As for the second problem, it's not related to this bz because the difference is that it's remapped depending on the page. This is present between any field and whether the patch is applied or not.

(In reply to Matthias Le Gac from comment #37)
> I fixed the problem when "online resources" is not displayed. There is no
> longer an extra space between fields. 
> 
> As for the second problem, it's not related to this bz because the
> difference is that it's remapped depending on the page. This is present
> between any field and whether the patch is applied or not.

Thanks Matthias!

Sign off on the way.

Hope QA goes smoothly!

David

Created attachment 165346 [details] [review]
Bug 36111: 856$h should not appear as a link in detailed record

Test plan:
1. Add 856$h to MARC editor
   1.1. Go to Administration > MARC bibliographic frameworks
   1.2. Next to Default framework, click Actions > MARC structure
   1.3. Search for field 856
   1.4. Click Actions > Edit subfields
   1.5. Click h
   1.6. Check the Editor box
   1.7. Click Save changes
2. Catalog a new record with a random URL in 856$h
   2.1. Go to Cataloging
   2.2. Click New record
   2.3. Fill out the mandatory fields (000, 003, 005, 008, 040$c,
        245$a, 942$c)
   2.4. Go to tab 8 and enter a value in 856$h
   2.5. Click Save (No need to add an item)
3. Search for the title in the staff interface (a large enough
   search to have more than one result)
   --> Notice it says "Online resources: Click here to access online"
   3.1. Try to click the link
        --> Blank page
4. Access the detailed record in the staff interface
   --> Notice it says "Online resources: Click here to access online"
   4.1. Try to click the link
        --> Blank page
5. Search for the title in the opac (a large enough search to have
   more than one result)
   --> Notice it says "Online resources: Click here to access online"
   5.1. Try to click the link
        --> Blank page
6. Access the detailed record in the opac
   --> Notice it says "Online resources: Click here to access online"
   6.1. Try to click the link
        --> Blank page
7. Edit the item and add a text in field 856$y
8. Access the detailed record in the staff interface
   --> Notice it says what you put in 856$y field
9. Apply the patch
10. Replay steps 3 through 6, but this time, the "Online resources"
   field should not be present.
11. Replay steps 7 through 8, but this time, what you put in 856$y
   field should not be present.
12. Edit the item and add a url in field 856$u
13. Replay steps 3 through 6, but this time, clicking on the link
    should take you to the URL specified in the 856$u field.
14. Replay steps 7 through 8, but this time, what you put in 856$y
   field should be present and
   clicking on the link should take you to the URL specified in the 856$u field.

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: David Nind <david@davidnind.com>

Created attachment 165347 [details] [review]
Bug 36111: fixed extra space

Signed-off-by: David Nind <david@davidnind.com>

Created attachment 165355 [details] [review]
Bug 36111: 856$h should not appear as a link in detailed record

Test plan:
1. Add 856$h to MARC editor
   1.1. Go to Administration > MARC bibliographic frameworks
   1.2. Next to Default framework, click Actions > MARC structure
   1.3. Search for field 856
   1.4. Click Actions > Edit subfields
   1.5. Click h
   1.6. Check the Editor box
   1.7. Click Save changes
2. Catalog a new record with a random URL in 856$h
   2.1. Go to Cataloging
   2.2. Click New record
   2.3. Fill out the mandatory fields (000, 003, 005, 008, 040$c,
        245$a, 942$c)
   2.4. Go to tab 8 and enter a value in 856$h
   2.5. Click Save (No need to add an item)
3. Search for the title in the staff interface (a large enough
   search to have more than one result)
   --> Notice it says "Online resources: Click here to access online"
   3.1. Try to click the link
        --> Blank page
4. Access the detailed record in the staff interface
   --> Notice it says "Online resources: Click here to access online"
   4.1. Try to click the link
        --> Blank page
5. Search for the title in the opac (a large enough search to have
   more than one result)
   --> Notice it says "Online resources: Click here to access online"
   5.1. Try to click the link
        --> Blank page
6. Access the detailed record in the opac
   --> Notice it says "Online resources: Click here to access online"
   6.1. Try to click the link
        --> Blank page
7. Edit the item and add a text in field 856$y
8. Access the detailed record in the staff interface
   --> Notice it says what you put in 856$y field
9. Apply the patch
10. Replay steps 3 through 6, but this time, the "Online resources"
   field should not be present.
11. Replay steps 7 through 8, but this time, what you put in 856$y
   field should not be present.
12. Edit the item and add a url in field 856$u
13. Replay steps 3 through 6, but this time, clicking on the link
    should take you to the URL specified in the 856$u field.
14. Replay steps 7 through 8, but this time, what you put in 856$y
   field should be present and
   clicking on the link should take you to the URL specified in the 856$u field.

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Created attachment 165356 [details] [review]
Bug 36111: fixed extra space

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

This makes sense to me and code is reasonably clear.

Passing QA

I am not sure about the proposed patches here as what they actually do and what is advertised in the bug title doesn't match up.

1) This changes the current behavior beyond not displaying $h: 

We won't be displaying any "Online resources" info if there isn't a $u. The LOC documentation lists a few examüles where we don't have a $u:

https://www.loc.gov/marc/bibliographic/bd856.html

856 	1#$amaine.maine.edu$cMust be decompressed with PKUNZIP$fresource.zip
856 	0#$akentvm.bitnet$facadlist file1$facadlist file2$facadlist file3
856 	0#$auicvm.bitnet$fAN2
856 	2#$amadlab.sprl.umich.edu$p3000
856 	10$zFTP access to PostScript version includes groups of article files with .pdf extension$aftp.cdc.gov$d/pub/EIS/vol*no*/adobe$f*.pdf$qapplication/pdf 

So are we sure there are no unintended side effects of this change?

2) I am not sure what this line does - but including style inline is usually not quite a good sign:

<xsl:attribute name="style">pointer-events: none; color: #202020;</xsl:attribute>

3) This does look overly complicated:

     <xsl:if test="marc:datafield[@tag=856]">
+        <xsl:if test="marc:datafield[@tag=856]/marc:subfield[@code='u']">

In full:

    <xsl:if test="marc:datafield[@tag=856]">
        <xsl:if test="marc:datafield[@tag=856]/marc:subfield[@code='u']">
        <span class="results_summary online_resources">
                <span class="label">Online resources: </span>
                    <xsl:for-each select="marc:datafield[@tag=856]">


So: Check if 856 exists... check if at least one of those has a $u... then loop over all 856 again. If we really want to limit display to only 856 with $u it would make sense to check for at least one with... and then adjust the for-each to only loop over those with $u.

4) This will just result in a broken link when there is no $u, it won't take care of removing the whole link from the HTML. It's either not needed as we check $u before, or it results in a broken link:

+                                <xsl:when test="marc:subfield[@code='u']">
                                     <xsl:text>Click here to access online</xsl:text>
-                                </xsl:otherwise>
+                                </xsl:when>


5) If we only select the 856 with $u for looping, this change won't be required. Also: it doesn't look right. Before we ask:

Is this the last, then show " ", otherwise print |.
Now it's turned into a really complex expression.

if we have (y or 3 or z or u) and (u) - at least one u too many:

-                <xsl:otherwise> | </xsl:otherwise>
+                <xsl:when test="(marc:subfield[@code='y'] or marc:subfield[@code='3'] or marc:subfield[@code='z'] or marc:subfield[@code='u']) and marc:subfield[@code='u']"><xsl:text> | </xsl:text></xsl:when>

I accidentally pushed this to main, but will revert, I think easier and less confusing to push a clean patch later. Please let me know if I can help - I have done quite a bit of XSLT work.

(In reply to Katrin Fischer from comment #44)
> I am not sure about the proposed patches here as what they actually do and
> what is advertised in the bug title doesn't match up.
> 
> 1) This changes the current behavior beyond not displaying $h: 
> 
> We won't be displaying any "Online resources" info if there isn't a $u. The
> LOC documentation lists a few examüles where we don't have a $u:
> 
> https://www.loc.gov/marc/bibliographic/bd856.html
> 
> 856 	1#$amaine.maine.edu$cMust be decompressed with PKUNZIP$fresource.zip
> 856 	0#$akentvm.bitnet$facadlist file1$facadlist file2$facadlist file3
> 856 	0#$auicvm.bitnet$fAN2
> 856 	2#$amadlab.sprl.umich.edu$p3000
> 856 	10$zFTP access to PostScript version includes groups of article files
> with .pdf
> extension$aftp.cdc.gov$d/pub/EIS/vol*no*/adobe$f*.pdf$qapplication/pdf 
> 
> So are we sure there are no unintended side effects of this change?
> 

I can't really answer the more technical questions, I will leave this to Matthias, but I can answer this one. Currently, there is a link "Click here to access online" whether there is a $u or not and the href is the contents of the $u. That is definitely not wanted, I don't think. If anyone currently uses the 856 field without $u, they must have display problems. 

I think the 856$u alternative in MARC is to use a combination of $a, $d, and $f. Those subfields are currently not supported in the XSLT.  

I don't really see how we could introduce a regression if the current behaviour is broken and the alternative behaviour is not implemented.

You're right that it kind of veered off from the title of the bug. It started with my issue with 856$h, but as Matthias looked at the code, we noticed that only $3, $u, $y and $z were implemented in the display, with the $u as the href and it was displayed if there is any 856 subfield. Since the link depends on the $u, I instructed him to hide the whole thing if there is no $u. I will change the bug's title to reflect more the current behaviour.

If we want to implement $a/$d/$f for those who use it, I think it should be done in the context of another bug.

I'll take a look at the technical problems.

(In reply to Caroline Cyr La Rose from comment #46)
> (In reply to Katrin Fischer from comment #44)
> > I am not sure about the proposed patches here as what they actually do and
> > what is advertised in the bug title doesn't match up.
> > 
> > 1) This changes the current behavior beyond not displaying $h: 
> > 
> > We won't be displaying any "Online resources" info if there isn't a $u. The
> > LOC documentation lists a few examüles where we don't have a $u:
> > 
> > https://www.loc.gov/marc/bibliographic/bd856.html
> > 
> > 856 	1#$amaine.maine.edu$cMust be decompressed with PKUNZIP$fresource.zip
> > 856 	0#$akentvm.bitnet$facadlist file1$facadlist file2$facadlist file3
> > 856 	0#$auicvm.bitnet$fAN2
> > 856 	2#$amadlab.sprl.umich.edu$p3000
> > 856 	10$zFTP access to PostScript version includes groups of article files
> > with .pdf
> > extension$aftp.cdc.gov$d/pub/EIS/vol*no*/adobe$f*.pdf$qapplication/pdf 
> > 
> > So are we sure there are no unintended side effects of this change?
> > 
> 
> I can't really answer the more technical questions, I will leave this to
> Matthias, but I can answer this one. Currently, there is a link "Click here
> to access online" whether there is a $u or not and the href is the contents
> of the $u. That is definitely not wanted, I don't think. If anyone currently
> uses the 856 field without $u, they must have display problems. 
> I think the 856$u alternative in MARC is to use a combination of $a, $d, and
> $f. Those subfields are currently not supported in the XSLT.  
> I don't really see how we could introduce a regression if the current
> behaviour is broken and the alternative behaviour is not implemented.
> 
> You're right that it kind of veered off from the title of the bug. It
> started with my issue with 856$h, but as Matthias looked at the code, we
> noticed that only $3, $u, $y and $z were implemented in the display, with
> the $u as the href and it was displayed if there is any 856 subfield. Since
> the link depends on the $u, I instructed him to hide the whole thing if
> there is no $u. I will change the bug's title to reflect more the current
> behaviour.
> 
> If we want to implement $a/$d/$f for those who use it, I think it should be
> done in the context of another bug.

All good points :)

I didn't have the time to dig deeper than checking if it could be a valid use case to have an 856 without $u. So I put the question out there. 

I am ok with relying on $u for now, but:

* Could we maybe adjust bug description and release notes to reflect the slightly different route we've taken?
* When making the comments 2)-5) I assumed we would take this route, so they should still be valid. Would be great to get some feedback/follow-ups on those. :)

Created attachment 165688 [details] [review]
Bug 36111: online resource link should be based on the presence of 856

Compared to the proposed enhancement, I've changed to loop only on the 856 that have the u field, and that way I've removed the if u that were overloading. 
However, I still left the first if u before the loop because otherwise it would leave an empty div which would modify the display space.

Caroline changed the task title and description to be more consistent.

Created attachment 165709 [details] [review]
Bug 36111: 856$h should not appear as a link in detailed record

Test plan:
1. Add 856$h to MARC editor
   1.1. Go to Administration > MARC bibliographic frameworks
   1.2. Next to Default framework, click Actions > MARC structure
   1.3. Search for field 856
   1.4. Click Actions > Edit subfields
   1.5. Click h
   1.6. Check the Editor box
   1.7. Click Save changes
2. Catalog a new record with a random URL in 856$h
   2.1. Go to Cataloging
   2.2. Click New record
   2.3. Fill out the mandatory fields (000, 003, 005, 008, 040$c,
        245$a, 942$c)
   2.4. Go to tab 8 and enter a value in 856$h
   2.5. Click Save (No need to add an item)
3. Search for the title in the staff interface (a large enough
   search to have more than one result)
   --> Notice it says "Online resources: Click here to access online"
   3.1. Try to click the link
        --> Blank page
4. Access the detailed record in the staff interface
   --> Notice it says "Online resources: Click here to access online"
   4.1. Try to click the link
        --> Blank page
5. Search for the title in the opac (a large enough search to have
   more than one result)
   --> Notice it says "Online resources: Click here to access online"
   5.1. Try to click the link
        --> Blank page
6. Access the detailed record in the opac
   --> Notice it says "Online resources: Click here to access online"
   6.1. Try to click the link
        --> Blank page
7. Edit the item and add a text in field 856$y
8. Access the detailed record in the staff interface
   --> Notice it says what you put in 856$y field
9. Apply the patch
10. Replay steps 3 through 6, but this time, the "Online resources"
   field should not be present.
11. Replay steps 7 through 8, but this time, what you put in 856$y
   field should not be present.
12. Edit the item and add a url in field 856$u
13. Replay steps 3 through 6, but this time, clicking on the link
    should take you to the URL specified in the 856$u field.
14. Replay steps 7 through 8, but this time, what you put in 856$y
   field should be present and
   clicking on the link should take you to the URL specified in the 856$u field.

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Created attachment 165710 [details] [review]
Bug 36111: fixed extra space

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Created attachment 165711 [details] [review]
Bug 36111: online resource link should be based on the presence of 856

Signed-off-by: David Nind <david@davidnind.com>

Testing notes (using KTD):

1. Edited the Books framework to make 856$h visible in the editor
2. Edited Programming Perl (biblionumber=262) to add two entries:
   Entry 1:
   . 856$h = https://example.com
   . 856$y = Non-functioning URL
   Entry 2:
   . 856$u = https://example.com
   . 856$y = Functioning URL
3. Before the patch, clicking on the non-functional URL link opens a blank page. The staff interface and OPAC search result and detail pages all display:
     Online resources: Non-functioning URL | Functioning URL
4. After the patch:
   4.1 The non-functioning URL is no longer displayed:
         Online resources: Functioning URL
   4.2 If there is only one 856 entry AND it has an $h, then 'Online resources' information is no longer displayed in any of the search result or detail pages. (As expected.)
   4.3 If a single entry has both an 856$h and an 856$y, it is not displayed. (As expected.) (My reading of the 856 information indicates you shouldn't do this. That is, every URL (whether $h or $u) should have a separate 856 entry.)
   4.4 If there is a $u and no $y subfield, then the text displayed in the search result and detail pages is:
         Online resources: Click here to access online

(In reply to Katrin Fischer from comment #45)
> I accidentally pushed this to main, but will revert, I think easier and less
> confusing to push a clean patch later. Please let me know if I can help - I
> have done quite a bit of XSLT work.

Oh I see 2 patches but 1 revert :
$ git ll --grep=36111 origin/main 
09eb174599 Revert "Bug 36111 - 856$h should not appear as a link in detailed record"
273e25f788 Bug 36111: fixed extra space
fe37e7eda6 Bug 36111: 856$h should not appear as a link in detailed record

(In reply to Fridolin Somers from comment #55)
> (In reply to Katrin Fischer from comment #45)
> > I accidentally pushed this to main, but will revert, I think easier and less
> > confusing to push a clean patch later. Please let me know if I can help - I
> > have done quite a bit of XSLT work.
> 
> Oh I see 2 patches but 1 revert :
> $ git ll --grep=36111 origin/main 
> 09eb174599 Revert "Bug 36111 - 856$h should not appear as a link in detailed
> record"
> 273e25f788 Bug 36111: fixed extra space
> fe37e7eda6 Bug 36111: 856$h should not appear as a link in detailed record

The revert should be for both patches.

What is the status of this patch, exactly?  Reverted after having a status "Need Sign off" is not common.

What is the path forward?

(In reply to Blou from comment #57)
> What is the status of this patch, exactly?  Reverted after having a status
> "Need Sign off" is not common.
> 
> What is the path forward?

First patches were pushed, then I reverted them. Both in one commit, which was what Frido was refering too when working through the git history for his backports. 

Now the re-worked patches are waiting for QA. Current status is correct.

Created attachment 170437 [details] [review]
Bug 36111: 856$h should not appear as a link in detailed record

Test plan:
1. Add 856$h to MARC editor
   1.1. Go to Administration > MARC bibliographic frameworks
   1.2. Next to Default framework, click Actions > MARC structure
   1.3. Search for field 856
   1.4. Click Actions > Edit subfields
   1.5. Click h
   1.6. Check the Editor box
   1.7. Click Save changes
2. Catalog a new record with a random URL in 856$h
   2.1. Go to Cataloging
   2.2. Click New record
   2.3. Fill out the mandatory fields (000, 003, 005, 008, 040$c,
        245$a, 942$c)
   2.4. Go to tab 8 and enter a value in 856$h
   2.5. Click Save (No need to add an item)
3. Search for the title in the staff interface (a large enough
   search to have more than one result)
   --> Notice it says "Online resources: Click here to access online"
   3.1. Try to click the link
        --> Blank page
4. Access the detailed record in the staff interface
   --> Notice it says "Online resources: Click here to access online"
   4.1. Try to click the link
        --> Blank page
5. Search for the title in the opac (a large enough search to have
   more than one result)
   --> Notice it says "Online resources: Click here to access online"
   5.1. Try to click the link
        --> Blank page
6. Access the detailed record in the opac
   --> Notice it says "Online resources: Click here to access online"
   6.1. Try to click the link
        --> Blank page
7. Edit the item and add a text in field 856$y
8. Access the detailed record in the staff interface
   --> Notice it says what you put in 856$y field
9. Apply the patch
10. Replay steps 3 through 6, but this time, the "Online resources"
   field should not be present.
11. Replay steps 7 through 8, but this time, what you put in 856$y
   field should not be present.
12. Edit the item and add a url in field 856$u
13. Replay steps 3 through 6, but this time, clicking on the link
    should take you to the URL specified in the 856$u field.
14. Replay steps 7 through 8, but this time, what you put in 856$y
   field should be present and
   clicking on the link should take you to the URL specified in the 856$u field.

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Bug 36111: fixed extra space

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Bug 36111: online resource link should be based on the presence of 856

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

This looks like we are almost there, but there is a change, that should not be there:

-                <ul class="resource_list">

The class makes it so that we show separators between multiple fields using CSS. 
If you add that back, it should make this unnecessary:

+                            <xsl:choose>
+                                <xsl:when test="position()=last()"><xsl:text>  </xsl:text></xsl:when>
+                                <xsl:otherwise> | </xsl:otherwise>
+                            </xsl:choose>

(In reply to Katrin Fischer from comment #60)

I've restored <ul class="resource_list"> and removed
<xsl:choose>
    <xsl:when test="position()=last()"><xsl:text>  </xsl:text></xsl:when>
    <xsl:otherwise> | </xsl:otherwise>
</xsl:choose>

Created attachment 170474 [details] [review]
Bug 36111: 856$h should not appear as a link in detailed record

Test plan:
1. Add 856$h to MARC editor
   1.1. Go to Administration > MARC bibliographic frameworks
   1.2. Next to Default framework, click Actions > MARC structure
   1.3. Search for field 856
   1.4. Click Actions > Edit subfields
   1.5. Click h
   1.6. Check the Editor box
   1.7. Click Save changes
2. Catalog a new record with a random URL in 856$h
   2.1. Go to Cataloging
   2.2. Click New record
   2.3. Fill out the mandatory fields (000, 003, 005, 008, 040$c,
        245$a, 942$c)
   2.4. Go to tab 8 and enter a value in 856$h
   2.5. Click Save (No need to add an item)
3. Search for the title in the staff interface (a large enough
   search to have more than one result)
   --> Notice it says "Online resources: Click here to access online"
   3.1. Try to click the link
        --> Blank page
4. Access the detailed record in the staff interface
   --> Notice it says "Online resources: Click here to access online"
   4.1. Try to click the link
        --> Blank page
5. Search for the title in the opac (a large enough search to have
   more than one result)
   --> Notice it says "Online resources: Click here to access online"
   5.1. Try to click the link
        --> Blank page
6. Access the detailed record in the opac
   --> Notice it says "Online resources: Click here to access online"
   6.1. Try to click the link
        --> Blank page
7. Edit the item and add a text in field 856$y
8. Access the detailed record in the staff interface
   --> Notice it says what you put in 856$y field
9. Apply the patch
10. Replay steps 3 through 6, but this time, the "Online resources"
   field should not be present.
11. Replay steps 7 through 8, but this time, what you put in 856$y
   field should not be present.
12. Edit the item and add a url in field 856$u
13. Replay steps 3 through 6, but this time, clicking on the link
    should take you to the URL specified in the 856$u field.
14. Replay steps 7 through 8, but this time, what you put in 856$y
   field should be present and
   clicking on the link should take you to the URL specified in the 856$u field.

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Bug 36111: fixed extra space

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Bug 36111: online resource link should be based on the presence of 856

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

I'm going to restore the PQA so it hits Katrins queue for a final review as she was close to this and appears to understand the final requirements best.

I am really sorry, but the change removing the resource_list class is still present in koha-tmpl/opac-tmpl/bootstrap/en/xslt/MARC21slim2OPACResults.xsl

You are also hardcoding a style here?
+                                <xsl:if test="not(marc:subfield[@code='u'])">
+                                    <xsl:attribute name="style">pointer-events: none; color: #202020;</xsl:attribute>
+                                </xsl:if>

(In reply to Katrin Fischer from comment #65)

> You are also hardcoding a style here?
> +                                <xsl:if
> test="not(marc:subfield[@code='u'])">
> +                                    <xsl:attribute
> name="style">pointer-events: none; color: #202020;</xsl:attribute>
> +                                </xsl:if>

I had restored the ul according to the last patch which included these lines, I will remove them

Created attachment 170576 [details] [review]
Bug 36111: 856 should not appear as a link in detailed record

Test plan:
1. Add 856 to MARC editor
   1.1. Go to Administration > MARC bibliographic frameworks
   1.2. Next to Default framework, click Actions > MARC structure
   1.3. Search for field 856
   1.4. Click Actions > Edit subfields
   1.5. Click h
   1.6. Check the Editor box
   1.7. Click Save changes
2. Catalog a new record with a random URL in 856
   2.1. Go to Cataloging
   2.2. Click New record
   2.3. Fill out the mandatory fields (000, 003, 005, 008, 040,
        245, 942)
   2.4. Go to tab 8 and enter a value in 856
   2.5. Click Save (No need to add an item)
3. Search for the title in the staff interface (a large enough
   search to have more than one result)
   --> Notice it says "Online resources: Click here to access online"
   3.1. Try to click the link
        --> Blank page
4. Access the detailed record in the staff interface
   --> Notice it says "Online resources: Click here to access online"
   4.1. Try to click the link
        --> Blank page
5. Search for the title in the opac (a large enough search to have
   more than one result)
   --> Notice it says "Online resources: Click here to access online"
   5.1. Try to click the link
        --> Blank page
6. Access the detailed record in the opac
   --> Notice it says "Online resources: Click here to access online"
   6.1. Try to click the link
        --> Blank page
7. Edit the item and add a text in field 856
8. Access the detailed record in the staff interface
   --> Notice it says what you put in 856 field
9. Apply the patch
10. Replay steps 3 through 6, but this time, the "Online resources"
   field should not be present.
11. Replay steps 7 through 8, but this time, what you put in 856
   field should not be present.
12. Edit the item and add a url in field 856
13. Replay steps 3 through 6, but this time, clicking on the link
    should take you to the URL specified in the 856 field.
14. Replay steps 7 through 8, but this time, what you put in 856
   field should be present and
   clicking on the link should take you to the URL specified in the 856 field.

Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Bug 36111: fixed extra space

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>

Bug 36111: online resource link should be based on the presence of 856

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Sorry, not pushed yet, my script got confused by the revert. But I'll be back.

Thanks for your perseverance! :)

Pushed for 24.11!

Well done everyone, thank you!