Bug 24713 - JavaScript error on staff client catalog search results page
Summary: JavaScript error on staff client catalog search results page
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P3 major (vote)
Assignee: Owen Leonard
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-02-22 18:55 UTC by David Nind
Modified: 2020-04-01 22:40 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
This patch modifies the output of template toolkit variables so that values in the in-page JavaScript are quoted. This avoids JavaScript errors when the template variable is empty.
Version(s) released in:
20.05.00


Attachments
Screenshots - Checkbox not visible in staff interface search results (135.62 KB, application/pdf)
2020-02-22 18:55 UTC, David Nind
Details
Bug 24713: JavaScript error on staff client catalog search results page (2.55 KB, patch)
2020-02-25 18:24 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 24713: JavaScript error on staff client catalog search results page (2.60 KB, patch)
2020-02-26 08:48 UTC, David Nind
Details | Diff | Splinter Review
Bug 24713: JavaScript error on staff client catalog search results page (2.69 KB, patch)
2020-03-06 08:55 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Nind 2020-02-22 18:55:02 UTC
Created attachment 99461 [details]
Screenshots - Checkbox not visible in staff interface search results

The checkbox for selecting records for the standard search results in the staff interface is no longer visible.

Checked on latest Firefox, Chromium, and Google Chrome on Ubuntu 18.04.

I've only noticed this in the last few days - it is fine in 19.11.x and at least a week ago in master.

See the screenshots attached.
Comment 1 Owen Leonard 2020-02-25 18:05:21 UTC
Git blames me and Bug 23885
Comment 2 Owen Leonard 2020-02-25 18:24:00 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2020-02-25 18:25:04 UTC
QA tool reports an error which I assume is unrelated:

   FAIL   valid_template
                Attempt to reload Koha/Template/Plugin/Biblio.pm aborted.
Compilation failed in require at /usr/lib/x86_64-linux-gnu/perl5/5.24/Template/Plugins.pm line 206.

Please correct me if I'm wrong.
Comment 4 Nick Clemens 2020-02-25 19:16:09 UTC
To recreate:
1 - Set AmazonCoverImages to "Show"
2 - Set AmazonCoverImages to "Don't show"
3 - Check the DB, the pref is now ""
    select * from systempreferences where variable like '%amazoncover%';
4 - Do a search on the staff client
5 - Note checkboxes are missing and there is a JS error

I wonder if the problem is that we set the value for "Don't show" as "" (blank string) instead of 0
Comment 5 Jonathan Druart 2020-02-26 08:16:18 UTC
I do not recreate this error, however I got it yesterday and fixed it.
See
  commit 884ab0d98e54d3a9f5229e2d21fd234dddf298b0
  Bug 23777: (follow-up) Use To.json in results.tt
Comment 6 David Nind 2020-02-26 08:48:29 UTC
Created attachment 99631 [details] [review]
Bug 24713: JavaScript error on staff client catalog search results page

This patch modifies the output of template toolkit variables so that
values in the in-page JavaScript are quoted. This avoids JavaScript
errors when the template variable is empty.

To test, apply the patch and perform a catalog search in the staff
client. Use the browser's developer tool to check for JavaScript errors
in the console. There should be none.

Test with AmazonCoverImages both on and off.

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2020-02-26 08:50:09 UTC
Was just about to post - it seems that this was fixed over the last few days (one of the commits since since Saturday).

I've signed it off, but feel free to close if this is not required anymore.
Comment 8 Owen Leonard 2020-02-26 12:31:24 UTC

*** This bug has been marked as a duplicate of bug 23777 ***
Comment 9 Owen Leonard 2020-03-03 16:41:15 UTC
(In reply to Jonathan Druart from comment #5)
> I do not recreate this error, however I got it yesterday and fixed it.
> See
>   commit 884ab0d98e54d3a9f5229e2d21fd234dddf298b0
>   Bug 23777: (follow-up) Use To.json in results.tt

Bug 23777 has been pushed to master but this is still a problem. Reopening.
Comment 10 Jonathan Druart 2020-03-03 17:21:35 UTC
(In reply to Nick Clemens from comment #4)
> To recreate:
> 1 - Set AmazonCoverImages to "Show"
> 2 - Set AmazonCoverImages to "Don't show"

=> I get '0', and so I still do not recreate the bug.
I think the patch is valid however, but I'd like to understand why it works for me. Can you confirm those steps are sufficient to recreate the problem? Which OS?
Comment 11 Jonathan Druart 2020-03-03 17:22:48 UTC
I am running koha-testing-docker with KOHA_IMAGE=master with docker-compose.yml and docker-compose.mariadb_d9.yml
Comment 12 Nick Clemens 2020-03-03 19:10:16 UTC
(In reply to Jonathan Druart from comment #11)
> I am running koha-testing-docker with KOHA_IMAGE=master with
> docker-compose.yml and docker-compose.mariadb_d9.yml

I am using docker-compose.mariadb_d10.yml
Comment 13 Jonathan Druart 2020-03-04 12:06:46 UTC
(In reply to Nick Clemens from comment #12)
> (In reply to Jonathan Druart from comment #11)
> > I am running koha-testing-docker with KOHA_IMAGE=master with
> > docker-compose.yml and docker-compose.mariadb_d9.yml
> 
> I am using docker-compose.mariadb_d10.yml

For the record: Nick is using Debian 9 with mariaDB for Debian 10

Owen, what about you?
Comment 14 Owen Leonard 2020-03-04 12:39:03 UTC
The question is only whether or not the value returned by the system preference check is going to trigger the error or not. If the value for AmazonCoverImages is '', the error will happen. If it's 0 or 1 it won't.
Comment 15 Jonathan Druart 2020-03-04 12:45:49 UTC
(In reply to Owen Leonard from comment #14)
> The question is only whether or not the value returned by the system
> preference check is going to trigger the error or not. If the value for
> AmazonCoverImages is '', the error will happen. If it's 0 or 1 it won't.

It's more about fixing this specific error or other possible occurrences where prefs can be an empty strings (when 0 is expected).
Comment 16 Marcel de Rooy 2020-03-06 08:55:43 UTC
Created attachment 100218 [details] [review]
Bug 24713: JavaScript error on staff client catalog search results page

This patch modifies the output of template toolkit variables so that
values in the in-page JavaScript are quoted. This avoids JavaScript
errors when the template variable is empty.

To test, apply the patch and perform a catalog search in the staff
client. Use the browser's developer tool to check for JavaScript errors
in the console. There should be none.

Test with AmazonCoverImages both on and off.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 17 Marcel de Rooy 2020-03-06 09:04:33 UTC
git grep -l -E "parseInt\(\s*\[%"
Before
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt

After
Nothing
Comment 18 Jonathan Druart 2020-03-06 10:02:21 UTC
(In reply to Marcel de Rooy from comment #17)
> git grep -l -E "parseInt\(\s*\[%"
> Before
> koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt
> 
> After
> Nothing

You should also search for
var xxx = [% Koha.Preference('xxx') %]
Comment 19 Martin Renvoize 2020-03-06 10:04:59 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 20 Joy Nelson 2020-04-01 22:40:17 UTC
does not apply to 19.11.x branch.  please rebase if needed.