Bug 35126 - Remove the use of event attributes from opac-addbybiblionumber.pl
Summary: Remove the use of event attributes from opac-addbybiblionumber.pl
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Lari Taskula
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 34709
  Show dependency treegraph
 
Reported: 2023-10-22 22:24 UTC by David Cook
Modified: 2024-10-24 03:29 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 35126: Remove onload from opac-addbybiblionumber.pl (3.31 KB, patch)
2024-10-02 19:00 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 35126: Remove onload from opac-addbybiblionumber.pl (3.36 KB, patch)
2024-10-24 03:29 UTC, David Cook
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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>