Bug 28675

Summary: QOTD broken in 20.11 and below
Product: Koha Reporter: Lucas Gass <lucas>
Component: ToolsAssignee: Bugs List <koha-bugs>
Status: Pushed to oldoldoldstable --- QA Contact: Victor Grousset/tuxayo <victor>
Severity: major    
Priority: P5 - low CC: fridolin.somers, jonathan.druart, victor, wainuiwitikapark
Version: 20.11Keywords: rel_20_11_candidate
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.09,20.05.15,19.11.21
Bug Depends on: 27942    
Bug Blocks:    
Attachments: Bug 28675: (bug 27942 follow-up) Fix QOTD - add new quote
Bug 28675: (bug 27942 follow-up) Fix QOTD - add new quote
Bug 28675: (bug 27942 follow-up) Fix QOTD - add new quote

Description Lucas Gass 2021-07-07 14:36:32 UTC
To recreate:

1. Go to QOTD tool (/cgi-bin/koha/tools/quotes.pl) 
2. Click Add quote
3. A new quote is automatically added with Text & Source of:

<input id="quoteSource" type="text" style="height:14px; width:99%" onkeydown="fnClickAddQuote(event,this.parentNode.parentNode)"/>
Comment 1 Victor Grousset/tuxayo 2021-07-08 02:55:23 UTC
Thanks, I'll investigate the regression.
(I guess the severity is normal?)
Comment 2 Lucas Gass 2021-07-08 14:25:03 UTC
This appears to be caused by this commit:

https://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=d810cd63100caf91c7346b73fd7db1d04a77424d
Comment 3 Victor Grousset/tuxayo 2021-07-08 18:55:57 UTC
It didn't occurred in bug 27942 to test the manual quote addition when backporting ^^"

Fridolin: 20.11 is confirmed to be affected.
Wainui: 19.11 is very likely to be affected also.
Comment 4 Victor Grousset/tuxayo 2021-07-09 03:21:21 UTC
So. <input> are put in the datatable object by the callback of the "add quote" button. And the display of a cell must not escape them.

So if we keep the same logic, we have to escape the text from the server but not the text from the callback of the "add quote" button.

One way to do that is by removing the cell escape and escape the data when the ajax request comes back.

https://datatables.net/reference/option/ajax
in the example:
«Manipulate the data returned from the server»
So the dataSrc callback look good.

But datatables changed it's way to do ajax request so the above doesn't work without changing that.
https://datatables.net/manual/server-side#Legacy

If doesn't seem we can't process the received data with old datatatable ajax method
https://legacy.datatables.net/usage/server-side


That's what I've found so far, in case that helps.
Comment 5 Jonathan Druart 2021-07-09 08:37:19 UTC
Created attachment 122697 [details] [review]
Bug 28675: (bug 27942 follow-up) Fix QOTD - add new quote

Bug 27942 has been written for master on top of bug 27251, and the code
differ a lot from what we have in <= 20.11.

This patch is quite ugly but it works, and is certainly the less painful
solution to fix stable branches.
Feel free to provide an alternative patch.
Comment 6 Jonathan Druart 2021-07-09 08:37:39 UTC
Patch tested on 20.11.x
Comment 7 Victor Grousset/tuxayo 2021-07-09 17:37:16 UTC
Created attachment 122754 [details] [review]
Bug 28675: (bug 27942 follow-up) Fix QOTD - add new quote

Bug 27942 has been written for master on top of bug 27251, and the code
differ a lot from what we have in <= 20.11.

This patch is quite ugly but it works, and is certainly the less painful
solution to fix stable branches.
Feel free to provide an alternative patch.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 8 Victor Grousset/tuxayo 2021-07-09 18:05:11 UTC
No error found after some fiddling on 20.05.x which needed a conflict to solve.

code looks good.
QA script passes on the 20.11 version and the 20.05 one.

Above attached is the 20.11 version (not the one with the 20.05 conflict solving which would have hindered the applicability on 20.11)

So, stamped!
Comment 9 Katrin Fischer 2021-07-11 22:03:53 UTC
Created attachment 122760 [details] [review]
Bug 28675: (bug 27942 follow-up) Fix QOTD - add new quote

Bug 27942 has been written for master on top of bug 27251, and the code
differ a lot from what we have in <= 20.11.

This patch is quite ugly but it works, and is certainly the less painful
solution to fix stable branches.
Feel free to provide an alternative patch.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Jonathan Druart 2021-07-12 09:37:51 UTC
Sending to 20.11 queue.
Comment 11 Katrin Fischer 2021-07-23 15:32:20 UTC
@Frido can still still be pushed for 20.11.08?

I just tested and this bug affects 20.11.07.
Comment 12 Fridolin Somers 2021-08-09 23:16:14 UTC
Patch pushed to 20.11.x for 20.11.09

Thanks a lot to all.
Comment 13 Victor Grousset/tuxayo 2021-08-16 20:07:20 UTC
Backported: Pushed to 20.05.x branch for 20.05.15
Comment 14 wainuiwitikapark 2021-08-17 01:35:14 UTC
Should this be backported to 19.11.x for 19.11.21?

Or if it contains string changes, backported to 19.11.22? As we are currently in a string freeze.
Comment 15 Jonathan Druart 2021-08-17 08:04:54 UTC
(In reply to wainuiwitikapark from comment #14)
> Should this be backported to 19.11.x for 19.11.21?

If it's broken in 19.11 you should backport it for the next one.
Comment 16 wainuiwitikapark 2021-08-18 01:54:14 UTC
It is broken in 19.11 so will backport it.
Comment 17 wainuiwitikapark 2021-08-18 02:03:40 UTC
Backported to 19.11.x for 19.11.21