Bug 18655 - Unimarc field 210c fails on importing fields with a simple quote
Summary: Unimarc field 210c fails on importing fields with a simple quote
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-05-23 08:52 UTC by Baptiste
Modified: 2019-10-14 19:57 UTC (History)
9 users (show)

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


Attachments
Bug 18655: correct the escaping of ' and move code toward jquery part (1.81 KB, patch)
2017-05-23 08:54 UTC, Baptiste
Details | Diff | Splinter Review
Bug 18655: Followup (1.78 KB, patch)
2017-05-23 13:35 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 18655: Correct the "chose" link (1.94 KB, patch)
2017-05-23 13:42 UTC, Baptiste
Details | Diff | Splinter Review
Bug 18655: Correct the choose link in unimarc_field_210c value builder (2.66 KB, patch)
2018-06-19 07:50 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 18655: Correct the choose link in unimarc_field_210c value builder (2.69 KB, patch)
2018-09-14 19:10 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 18655: Correct the choose link in unimarc_field_210c value builder (2.74 KB, patch)
2018-09-15 17:29 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 18655: Correct the choose link in unimarc_field_210c value builder (2.80 KB, patch)
2018-10-10 00:08 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Baptiste 2017-05-23 08:52:33 UTC
When you search for an editor with a simple quote in the name, the escaping fails and nothing happens.
Comment 1 Baptiste 2017-05-23 08:54:35 UTC Comment hidden (obsolete)
Comment 2 Fridolin Somers 2017-05-23 13:35:16 UTC Comment hidden (obsolete)
Comment 3 Baptiste 2017-05-23 13:42:33 UTC
Created attachment 63653 [details] [review]
Bug 18655: Correct the "chose" link

- Link 'chose' is now a button.
- It doesn't call directly javascript anymore (use of listener).
- field to load stored in html on the page (less escaping needed).
Comment 4 Katrin Fischer 2017-10-15 13:01:32 UTC
Maybe someone familiar with the plugin could test? 

It sounds like a search within the plugin which contains ' should fail without the patch.
Comment 5 Simon Pouchol 2017-10-26 13:11:54 UTC
Hi Baptiste,
I was looking to test this patch, could you provide a Test Plan ?
I can't seem to able to reproduce the bug within the unimarc plugin 210c, with or without the patch I can search for an authority with a ' in the name.
The choose button doesn't seem appear for me at all (can't find it hidden in dev tools either).
Comment 6 Fridolin Somers 2017-11-20 11:21:45 UTC
(In reply to Simon Pouchol from comment #5)
> Hi Baptiste,
> I was looking to test this patch, could you provide a Test Plan ?
> I can't seem to able to reproduce the bug within the unimarc plugin 210c,
> with or without the patch I can search for an authority with a ' in the name.
> The choose button doesn't seem appear for me at all (can't find it hidden in
> dev tools either).

I continue Baptiste's job.

You need an existing authority with a ' in its name, like "L'histoire".

When trying to choose this authority it fails.

Btw I think that writing JS inside HTML tags is now prohibited.
Comment 7 Charles Farmer 2018-03-30 21:33:10 UTC
Patch doesn't apply anymore, because of what looks like an added MACRO jsinclude BLOCK:

> CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/unimarc_field_210c.tt
Comment 8 Katrin Fischer 2018-03-31 05:58:52 UTC
Javascript has been moved to the bottom of all the tt in intranet - that's probably causing the conflict.
Comment 9 Fridolin Somers 2018-06-19 07:50:39 UTC
Created attachment 76168 [details] [review]
Bug 18655: Correct the choose link in unimarc_field_210c value builder

- Link 'choose' is now a button.
- It doesn't call directly javascript anymore (use of listener).
- field to load stored in html on the page (less escaping needed).

Test plan :
1) Use UNIMARC catalog
2) Define unimarc_field_210c value builder on 210$c
3) Define an autority type EDITORS with heading on 200$b
4) Create an autority of this type with a single quote in heading, ie : l'avenir
5) Index this new autority
6) Edit a biblio record and launch 210$c value builder
7) Search for new autority
8) Click on 'Choose'
=> Heading should be pasted in 210$c
Comment 10 Fridolin Somers 2018-09-14 19:10:10 UTC
Created attachment 78710 [details] [review]
Bug 18655: Correct the choose link in unimarc_field_210c value builder

- Link 'choose' is now a button.
- It doesn't call directly javascript anymore (use of listener).
- field to load stored in html on the page (less escaping needed).

Test plan :
1) Use UNIMARC catalog
2) Define unimarc_field_210c value builder on 210$c
3) Define an autority type EDITORS with heading on 200$b
4) Create an autority of this type with a single quote in heading, ie : l'avenir
5) Index this new autority
6) Edit a biblio record and launch 210$c value builder
7) Search for new autority
8) Click on 'Choose'
=> Heading should be pasted in 210$c
Comment 11 Biblibre Sandboxes 2018-09-15 17:29:14 UTC
Patch tested with a sandbox, by David Nind <david@davidnind.com>
Comment 12 Biblibre Sandboxes 2018-09-15 17:29:45 UTC
Created attachment 78865 [details] [review]
Bug 18655: Correct the choose link in unimarc_field_210c value builder

- Link 'choose' is now a button.
- It doesn't call directly javascript anymore (use of listener).
- field to load stored in html on the page (less escaping needed).

Test plan :
1) Use UNIMARC catalog
2) Define unimarc_field_210c value builder on 210$c
3) Define an autority type EDITORS with heading on 200$b
4) Create an autority of this type with a single quote in heading, ie : l'avenir
5) Index this new autority
6) Edit a biblio record and launch 210$c value builder
7) Search for new autority
8) Click on 'Choose'
=> Heading should be pasted in 210$c

Signed-off-by: David Nind <david@davidnind.com>
Comment 13 Chris Cormack 2018-10-10 00:08:58 UTC
Created attachment 80315 [details] [review]
Bug 18655: Correct the choose link in unimarc_field_210c value builder

- Link 'choose' is now a button.
- It doesn't call directly javascript anymore (use of listener).
- field to load stored in html on the page (less escaping needed).

Test plan :
1) Use UNIMARC catalog
2) Define unimarc_field_210c value builder on 210$c
3) Define an autority type EDITORS with heading on 200$b
4) Create an autority of this type with a single quote in heading, ie : l'avenir
5) Index this new autority
6) Edit a biblio record and launch 210$c value builder
7) Search for new autority
8) Click on 'Choose'
=> Heading should be pasted in 210$c

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 14 Nick Clemens 2018-10-15 16:57:40 UTC
Awesome work all!

Pushed to master for 18.11
Comment 15 Martin Renvoize 2018-10-17 10:44:59 UTC
Pushed to 18.05.x for 18.05.05
Comment 16 Fridolin Somers 2018-10-17 12:33:20 UTC
Pushed to 17.11.x for 17.11.11