Bug 35126

Summary:	Remove the use of event attributes from opac-addbybiblionumber.pl
Product:	Koha	Reporter:	David Cook <dcook>
Component:	OPAC	Assignee:	Lari Taskula <lari.taskula>
Status:	Signed Off ---	QA Contact:	Testopia <testopia>
Severity:	minor
Priority:	P5 - low	CC:	victor
Version:	Main
Hardware:	All
OS:	All
Change sponsored?:	---	Patch complexity:	---
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:
Circulation function:
Bug Depends on:
Bug Blocks:	34709
Attachments:	Bug 35126: Remove onload from opac-addbybiblionumber.pl Bug 35126: Remove onload from opac-addbybiblionumber.pl

Description David Cook 2023-10-22 22:24:28 UTC

There are some event attributes in opac-addbybiblionumber.pl. These should be removed in favour of defining events in JavaScript.

./opac/opac-addbybiblionumber.pl:            print "<html><meta http-equiv=\"refresh\" content=\"0\" /><body onload=\"window.opener.location.reload(true);self.close();\"></body></html>";
./opac/opac-addbybiblionumber.pl:        print "<html><meta http-equiv=\"refresh\" content=\"0\" /><body onload=\"self.close();\"></body></html>";

Comment 1 Owen Leonard 2024-07-05 18:30:42 UTC

These instances I'm honestly not concerned about. I don't think there would be a benefit to trying to change the way these work.

Anyone is free to re-open it if I'm not seeing an issue with this.

Comment 2 David Cook 2024-07-07 23:24:36 UTC

(In reply to Owen Leonard from comment #1)
> These instances I'm honestly not concerned about. I don't think there would
> be a benefit to trying to change the way these work.
> 
> Anyone is free to re-open it if I'm not seeing an issue with this.

There's not an immediate issue, but one day if/when we have a Content-Security-Policy, these will stop working. 

I haven't looked at the script specifically to see what the impact of that will be.

Comment 3 Victor Grousset/tuxayo 2024-07-19 20:57:19 UTC

Should this be done before Bug 20397? Or should the challenges specific to 20397 be tackled first before coming back to this ticket?

Comment 4 David Cook 2024-07-22 05:48:31 UTC

(In reply to Victor Grousset/tuxayo from comment #3)
> Should this be done before Bug 20397? Or should the challenges specific to
> 20397 be tackled first before coming back to this ticket?

Yeah, it should be done before bug 20397.

Comment 5 David Cook 2024-07-22 05:50:09 UTC

I'll reopen and take it as assignee. Probably won't do it right away but have it on my TODO list...

Comment 6 Lari Taskula 2024-10-02 19:00:56 UTC

Created attachment 172332 [details] [review]
Bug 35126: Remove onload from opac-addbybiblionumber.pl

To test:

Case a) Adding to a new list

1. Enable virtualshelves system preference
2. Search for any record and click it
3. On the right hand side of record view, click "Save to your lists"
4. A new pop up opens.
5. Under "Add to a new list", give any name to the list
6. Under "Add to a new list", click "Save"
7. Observe popup closing
8. Observe record view page reloading

Case b) Adding to an existing list

1. Enable virtualshelves system preference
2. Search for any record and click it
3. On the right hand side of record view, click "Save to your lists"
4. A new pop up opens.
5. Under "Add to a list", select an existing list
(it can be the same as the one you created in case a, it does not matter here)
6. Under "Add to a list", click "Save"
7. Observe popup closing
8. Unlike case a, observe record view page NOT reloading

Comment 7 David Cook 2024-10-24 02:16:43 UTC

Just on a call at the moment, but I'm hoping to look at this today

Comment 8 David Cook 2024-10-24 03:26:04 UTC

Yep, good enough. Works as described.

Comment 9 David Cook 2024-10-24 03:29:11 UTC

Created attachment 173247 [details] [review]
Bug 35126: Remove onload from opac-addbybiblionumber.pl

To test:

Case a) Adding to a new list

1. Enable virtualshelves system preference
2. Search for any record and click it
3. On the right hand side of record view, click "Save to your lists"
4. A new pop up opens.
5. Under "Add to a new list", give any name to the list
6. Under "Add to a new list", click "Save"
7. Observe popup closing
8. Observe record view page reloading

Case b) Adding to an existing list

1. Enable virtualshelves system preference
2. Search for any record and click it
3. On the right hand side of record view, click "Save to your lists"
4. A new pop up opens.
5. Under "Add to a list", select an existing list
(it can be the same as the one you created in case a, it does not matter here)
6. Under "Add to a list", click "Save"
7. Observe popup closing
8. Unlike case a, observe record view page NOT reloading

Signed-off-by: David Cook <dcook@prosentient.com.au>