Bug 14703 - Holidays description shows \r\n for every new line
Summary: Holidays description shows \r\n for every new line
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Holidays (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: j.kylmala
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-08-21 09:47 UTC by j.kylmala
Modified: 2016-06-21 21:38 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 14703: Holidays description shows \r\n for every new line (1.66 KB, patch)
2015-08-25 07:31 UTC, j.kylmala
Details | Diff | Splinter Review
Bug 14703: Holidays description shows \r\n for every new line (1.69 KB, patch)
2015-08-27 12:48 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 j.kylmala 2015-08-21 09:47:33 UTC
First go to /cgi-bin/koha/tools/holidays.pl. If one adds there a new line to a holiday's description, in the left side of the page, under Description column, the new line is written in the form of \r\n. I think this should be fixed to show a new line and not a newline character.
Comment 1 j.kylmala 2015-08-25 07:31:35 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2015-08-27 12:48:54 UTC
Created attachment 42025 [details] [review]
Bug 14703: Holidays description shows \r\n for every new line

Adds a new line for the holiday's description instead of the string
\r\n.

Test plan:
1. In tools -> Calendar look at some holiday's description
   (Do a new holiday with description of multiple lines if
   there is not already)
2. Notice that there is characters \r\n if someone has put
   a newline in the holiday's description
3. Apply patch
4. See that the new lines show there now nicely

Sponsored-by: Vaara-kirjastot

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended slightly: no need to replace title.
Comment 3 Marcel de Rooy 2015-08-27 12:51:55 UTC
Taking the liberty to promote this trivial patch to PQA directly.
Thanks, Joonas.
Comment 4 Jonathan Druart 2015-08-27 14:12:35 UTC
IMO this should be done after the input (and correct the values in DB), not on display.
Comment 5 j.kylmala 2015-08-27 14:26:57 UTC
(In reply to Jonathan Druart from comment #4)
> IMO this should be done after the input (and correct the values in DB), not
> on display.

I tried that in a way that I left the new line character there, but then there was some JavaScript which refused to work with that. And if we put the <br> tag in the database I'm not sure if it will show in the description as the string <br> or as a new line. Let me try that.
Comment 6 j.kylmala 2015-08-27 14:28:44 UTC
(In reply to Joonas Kylmälä from comment #5)
> (In reply to Jonathan Druart from comment #4)
> > IMO this should be done after the input (and correct the values in DB), not
> > on display.
> 
> I tried that in a way that I left the new line character there, but then
> there was some JavaScript which refused to work with that. And if we put the
> <br> tag in the database I'm not sure if it will show in the description as
> the string <br> or as a new line. Let me try that.

But wait, I think I tried that and the <br> will show in the edit text box as <br>.. So I came to the conclusion that it's much more better to have the fix in only one place.
Comment 7 Jonathan Druart 2015-08-27 15:19:23 UTC
Forget the idea if it's not easy to do :)
Comment 8 Marcel de Rooy 2015-08-28 06:43:39 UTC
Hobby holiday project:

See newHolidays.pl

if ($description) {
    $description =~ s/\r/\\r/g;
    $description =~ s/\n/\\n/g;
}
Comment 9 Marcel de Rooy 2015-08-28 06:45:56 UTC
Joonas: If you want to change the patch, go ahead and change the status. But I do not mind pushing this tiny adjustment.
Comment 10 j.kylmala 2015-08-28 07:20:53 UTC
(In reply to Marcel de Rooy from comment #8)
> Hobby holiday project:
> 
> See newHolidays.pl
> 
> if ($description) {
>     $description =~ s/\r/\\r/g;
>     $description =~ s/\n/\\n/g;
> }

I looked into that. Changing the new line into <br> didn't work in the because then we need to also change the description editor to convert <br> to \r\n. So it's similar fix to that what we do now when displaying the description (but more changes with no reason). And so we have done already the fix to change the description so I will go with the current patch.
Comment 11 Tomás Cohen Arazi 2015-08-28 13:44:13 UTC
Patch pushed to master.

Thanks Joonas!
Comment 12 Chris Cormack 2015-09-22 01:18:17 UTC
Pushed to 3.20.x will be in 3.20.4