In several places we escape quotation marks using $value =~ s/"/"/g; All the occurrences are wrong and must be removed.
Created attachment 83120 [details] [review] Bug 21986: Do not escape quotation marks when cataloguing In several places we escape quotation marks using $value =~ s/"/"/g; All the occurrences are wrong and must be removed. Most of them are leftover of bug 11638 (Remove HTML from addbiblio.pl), which removes the construction of html from pl scripts. The problem has been highlighted by bug 13618, I did not track down why the issue did not exist before (?) Test plan: 0/ Use strings with quotation marks, like: 'Fiddle tune history : "bad" tunes' You can also use other html characters to make the tests more complete, like 'Fiddle tune history : <"bad" tunes>' 1/ authorities/authorities.pl a. Edit an authority filling different fields with quotation marks b. Edit it again => The display (inputs' values) is wrong, if you save the escaped quotes will be inserted 2/ cataloguing/addbiblio.pl Same editing a bibliographic record 3/ cataloguing/additem.pl Same editing items 4/ members/memberentry.pl Edit a patron's record and fill some fields with quotation marks + fields borrowernotes and opacnotes => The quotes are inserted directly in DB (escape is done before the insert!) 5/ opac/opac-review.pl For QA only: $js_ok_review is never used 6/ tools/batchMod.pl For QA only: $value is always undefined at that point
Created attachment 83143 [details] image of the problem
(In reply to Elaine Bradtke from comment #2) > Created attachment 83143 [details] > image of the problem Hi Elaine, do you still have this problem with the patch applied?
Created attachment 83179 [details] [review] Bug 21986: Do not escape quotation marks when cataloguing In several places we escape quotation marks using $value =~ s/"/"/g; All the occurrences are wrong and must be removed. Most of them are leftover of bug 11638 (Remove HTML from addbiblio.pl), which removes the construction of html from pl scripts. The problem has been highlighted by bug 13618, I did not track down why the issue did not exist before (?) Test plan: 0/ Use strings with quotation marks, like: 'Fiddle tune history : "bad" tunes' You can also use other html characters to make the tests more complete, like 'Fiddle tune history : <"bad" tunes>' 1/ authorities/authorities.pl a. Edit an authority filling different fields with quotation marks b. Edit it again => The display (inputs' values) is wrong, if you save the escaped quotes will be inserted 2/ cataloguing/addbiblio.pl Same editing a bibliographic record 3/ cataloguing/additem.pl Same editing items 4/ members/memberentry.pl Edit a patron's record and fill some fields with quotation marks + fields borrowernotes and opacnotes => The quotes are inserted directly in DB (escape is done before the insert!) 5/ opac/opac-review.pl For QA only: $js_ok_review is never used 6/ tools/batchMod.pl For QA only: $value is always undefined at that point Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
I cannot replicate the issue after applying the patches.. Signing Off
QA: Looking here
> The problem has been highlighted by bug 13618, I did not track down why > the issue did not exist before (?) If you pass a&b in a template var thru the html filter, it will run the ampersand conversion once again. Resulting in: a&amp;b So we need to choose where to escape.
(In reply to Martin Renvoize from comment #4) > For QA only: $js_ok_review is never used > 6/ tools/batchMod.pl > For QA only: $value is always undefined at that point Thx. Just seeing these remarks by accident. They were quite hidden..
(In reply to Marcel de Rooy from comment #7) > > The problem has been highlighted by bug 13618, I did not track down why > > the issue did not exist before (?) > > If you pass a&b in a template var thru the html filter, it will run the > ampersand conversion once again. Resulting in: a&amp;b > > So we need to choose where to escape. Should be quotation. But same thing applies.
Created attachment 83195 [details] [review] Bug 21986: Do not escape quotation marks when cataloguing In several places we escape quotation marks using $value =~ s/"/"/g; All the occurrences are wrong and must be removed. Most of them are leftover of bug 11638 (Remove HTML from addbiblio.pl), which removes the construction of html from pl scripts. The problem has been highlighted by bug 13618, I did not track down why the issue did not exist before (?) Test plan: 0/ Use strings with quotation marks, like: 'Fiddle tune history : "bad" tunes' You can also use other html characters to make the tests more complete, like 'Fiddle tune history : <"bad" tunes>' 1/ authorities/authorities.pl a. Edit an authority filling different fields with quotation marks b. Edit it again => The display (inputs' values) is wrong, if you save the escaped quotes will be inserted 2/ cataloguing/addbiblio.pl Same editing a bibliographic record 3/ cataloguing/additem.pl Same editing items 4/ members/memberentry.pl Edit a patron's record and fill some fields with quotation marks + fields borrowernotes and opacnotes => The quotes are inserted directly in DB (escape is done before the insert!) 5/ opac/opac-review.pl For QA only: $js_ok_review is never used 6/ tools/batchMod.pl For QA only: $value is always undefined at that point Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Awesome work all! Pushed to master for 19.05
Pushed to 18.11.x for 18.11.01
backported to 18.05 for 18.05.08
Pushed to 17.11.x for 17.11.14
Bug 22316 is similar to this, but only manifests when you try to sort the patron search result table. Tested with 18.11.02.
On version Koha 18.05.08, still experiencing issues with this same thing. When editing a record, or Edit as New - a title with quotation marks will be removed. Filing a new bug: 22395
Lucas and Fridolin, take a look at bug 22395, it seems that this patch should not have been backported into your versions.
(In reply to Jonathan Druart from comment #17) > Lucas and Fridolin, take a look at bug 22395, it seems that this patch > should not have been backported into your versions. Oh indeed I see the problem. Reverted from 17.11.x, will be removed in 17.11.16.
reverted in 18.05.x and will be removed in 18.05.10
*** Bug 22367 has been marked as a duplicate of this bug. ***
*** Bug 22769 has been marked as a duplicate of this bug. ***