Bug 8062

Confirmed in master using fresh nb-NO templates.

Ok, I compared the English with the translated German templates. The translated file is missing all line breaks after line 7 <MESSAGE> and is broken because of that. I am not sure how to fix it as it seems to be a problem with the translation scripts not preserving the line breaks correctly.

Ok, the error is starting even before line 7. Still investigating.

After giving it some thought I think the main problem here is that our translation scripts are not made to parse a plain text file, but TT and HTML. I also tried adding the line breaks back in Poedit, but they get taken out again on saving the file. We can also not use \n or similar, because that will also cause problems (bug 4883)

Without the line breaks the formatting is broken and the regex searching for the markers (<SUBJECT><END_SUBJECT> etc) are not working.

I am a bit out of ideas right now on how to fix that.

This is a big problem for every translated Koha installation. It's not only affecting the OPAC cart, but also staff cart and sending lists from both staff and cart.

I confirm the bug, its severity, and your diagnostic. There should be
already get an issue before 3.8. Email sent by Koha comes from a
template which isn't strictly a template. There is a pseudo-syntax
allowing subject/message extraction. But this mini-templating system
isn't compliant with our current translation system which doesn't
preserve new lines: it doesn't matter in HTML; it's important in raw
text, which is used in basket email.

We must rewrite in HTML the email sent by OPAC Send your Basket
functionality.

Hi Frederic, 
thank you for providing a patch! I have tried to find out when the feature broke and tracked it back to the switch to TT. I am not sure how it could go unnoticed for so long.

I am a bit worried about switching to HTML, because normally Koha is sending text mails. Is there a way we could make it work as text mail still?

We could perhaps trick the translation tools by using <br/> and a paragraph in the notice template, but remove those (HTML::Scrubber?) before sending the mail out.

> I am a bit worried about switching to HTML, because normally Koha is
> sending text mails.

Take a look a the resulting mail. It's not that bad. 

> Is there a way we could make it work as text mail still?

We could fix the translator, and get it preserving new line characters.
But it's not that easy.

> We could perhaps trick the translation tools by using <br/> and a
> paragraph in the notice template, but remove those (HTML::Scrubber?)
> before sending the mail out.

I fear that chunk of HTML finish in the .po file, and so disturb
translators. We could also transform the HTML into Text just before
sending the email.

It works fine :)

Perharps a followup is required to specified the new dependency (in C4/Installer/PerlDependencies.pm ?)

> Perharps a followup is required to specified the new dependency (in
> C4/Installer/PerlDependencies.pm ?)

The template contains a 'USE HtmlToText' directive, which use a
Template::Toolkit module. It's not a Perl module, and it's included by
default with Template::Toolkit. Is it the dependency we were thinking
about?

To test your patch, I had to install the Template::Plugin::HtmlToText module via cpan.

QA comment:
 * small patch, that uses a T::T plugin to fix an annoying problem
 * introduces a new dependency that could be usefull in other circumstances, so it's OK
 * remove some commented/dead code ++
 * opac/opac-sendbasket.pl still passes perlcritic

passed QA

critical problem in non-english languages, worth porting it to 3.8

pushed to 3.8.x will be in 3.8.2

The patch for this major bug will require a few things for inclusion in 3.6.x:
1. A new patch that does not conflict on koha-tmpl/opac-tmpl/prog/en/modules/opac-sendbasket.tt
2. A follow-up patch that adds instructions for installing the new Perl module via CPAN and/or the debian.koha-community.org apt repository
3. A packaged version of libtemplate-plugin-htmltotext-perl for the oldstable distribution on debian.koha-community.org
4. An explanation of what the new dependency is required for so that I can put it in the release notes.

I have no time now for your points 1-3.

> 4. An explanation of what the new dependency is required for so that I
> can put it in the release notes.

With Template::Plugin::HtmlToText, it's possible to transform HTML into
Text! as suggested by the module name. It allows Koha developers to use
standard (for Koha) Template::Toolkit templating system to generate HTML,
and then, if necessary to transform this HTML into raw text. This is
required for example for email sent by Koha, which use to be in Text.
Benefits: --email templates can have a rich HTML-base layout, but still
being sent in Text ; --email templates are translatable like Koha web
pages

I think this patch introduces a bug:

+use HTML::FormatText;

this isn't mentioned in the dependency list, and doesn't appear to be used in the code. Thus, opac-sendbasket.pl breaks unless you happen to have that module installed. Can someone confirm that this is the case?

OK, it is a dependency of Template::Plugin::HtmlToText, however that doesn't declare it as a dependency in the META.yml, so the packages don't either, and so it never gets installed. Guess I'll have to redo the packages some time soon to account for that.

(In reply to comment #21)
> I think this patch introduces a bug:
> 
> +use HTML::FormatText;
> 
> this isn't mentioned in the dependency list, and doesn't appear to be used
> in the code. Thus, opac-sendbasket.pl breaks unless you happen to have that
> module installed. Can someone confirm that this is the case?

Confirmed: Can't locate HTML/FormatText.pm in @INC

Paul: When you passed this through QA, you mentioned the new dependency. But nothing was done in a followup for dependency list, etc. ?

I submitted a patch for the dependency under 7368.
But Robin is right that the code in opac-basket does not directly use FormatText. So the use statement can be deleted.
But note that the Template::Plugin::HtmlToText requires HTML::FormatText.
So it is a core Koha dependency.

Created attachment 10969 [details] [review]
Patch for HTML::FormatText

Deletes use statement only in opac-sendbasket.pl
Template::Plugin::HtmlToText does require it on its own.

QA Comments:

Bug 8062 introduced this new dependency in opac-sendbasket. But it did not call a routine of this module. 
As no new call have been added since, it's useless.

Marking as Passed QA.

Follow-up pushed
(yikes : I wanted to remove the self-signoff & put mine, but pushed to quickly...)

Follow up pushed to 3.8.x, will be in 3.8.4