Bug 8937

Summary: Translation process removes CDATA in RSS XML
Product: Koha Reporter: Fridolin Somers <fridolin.somers>
Component: I18N/L10NAssignee: Bugs List <koha-bugs>
Status: NEW --- QA Contact:
Severity: normal    
Priority: P5 - low CC: bgkriegel, frederic, katrin.fischer, marjorie.barry-vila, victor
Version: master   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8936
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20286
Change sponsored?: --- Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 8937: Translation process removes xml prolog

Description Fridolin Somers 2012-10-17 09:06:19 UTC
The translation process seams to manage CDATA tags in JS but not in HTML directly.
This exists in RSS feed 'opac-opensearch.tt' : 
  <description><![CDATA[ Search results [% IF ( query_desc ) %]for '[% query_desc |html %]'[% END %][% IF ( limit_desc ) %] with limit(s): '[% limit_desc |html %]'[% END %] at [% LibraryName |html %]]]></description>
is translated in fr-FR :
  <description> Search results [% IF ( query_desc ) %]for '[% query_desc |html %]'[% END %][% IF ( limit_desc ) %] with limit(s): '[% limit_desc |html %]'[% END %] at [% LibraryName |html %]</description>

This is another cause of bug 8936.
Comment 1 Owen Leonard 2013-08-02 19:49:32 UTC
I notice the fr-FR version of opac-opensearch.tt is also missing the XML declaration at the top ("<?xml version='1.0' encoding='utf-8'  ?>").
Comment 2 Victor Grousset/tuxayo 2018-06-21 09:27:03 UTC
== Reproduice ==
1. opac: switch to english language
2. opac: Search for something that returns multiple results
3. open rss feed
4. See that the author name and "place hold" link show
     Optional: Right click → page source → see that multiple CDATA section are there
5. Translate & switch to another language than english
6. open rss feed
7. See that only the titles show. Nor the author name nor the "place hold" link
     This is the bug.
     Optional: Right click → page source → see that only one CDATA section is there
Comment 3 Victor Grousset/tuxayo 2018-09-03 15:18:29 UTC
Should be fixed by bug 15395

So it might be better to put the efforts in treating the cause instead of yet another symptom.
Comment 4 Katrin Fischer 2019-04-28 11:22:09 UTC
(In reply to Victor Grousset/tuxayo from comment #3)
> Should be fixed by bug 15395
> 
> So it might be better to put the efforts in treating the cause instead of
> yet another symptom.

Bug 15395 is pushed now. Would you be able to provide a fix here?
Comment 5 Victor Grousset/tuxayo 2019-05-13 15:11:12 UTC
From what I understand, it turns out that regardless of 15395, the main translation system will still process the file and remove the CDATA.

So I don't know what would be a decent way to fix this.
Comment 6 Bernardo Gonzalez Kriegel 2019-12-06 20:00:24 UTC
Created attachment 96076 [details] [review]
Bug 8937: Translation process removes xml prolog

This patch fix the missing xml prolog in translated
files, xml ot TT.

Is fixed teaching C4::TTParse not to ignore <?..?> constructs,
then teaching xgettext.pl to ignore those strings. Net result is
that they are copied in the translated file.

To test:
1) Update & install your preferred language,
(cd misc/translator/; perl translate update xx-YY; perl translate install xx-YY )

2) Compare the first lines (head -2) of:
koha-tmpl/opac-tmpl/bootstrap/en/xslt/compact.xsl
koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-opensearch.tt
koha-tmpl/intranet-tmpl/prog/en/xslt/plainMARC.xsl
and
koha-tmpl/opac-tmpl/bootstrap/xx-YY/xslt/compact.xsl
koha-tmpl/opac-tmpl/bootstrap/xx-YY/modules/opac-opensearch.tt
koha-tmpl/intranet-tmpl/prog/xx-YY/xslt/plainMARC.xsl

Check the missing prolog

3) Install this patch, repeat 1 and 2, now the prolog is present
on translated files.
Comment 7 Bernardo Gonzalez Kriegel 2019-12-07 10:42:09 UTC
(In reply to Victor Grousset/tuxayo from comment #5)
> From what I understand, it turns out that regardless of 15395, the main
> translation system will still process the file and remove the CDATA.
> 
> So I don't know what would be a decent way to fix this.

If one takes a look at the translated file, not all CDATA entries are removed.

In EN 'opac-opensearch.tt' for example there are two lines near the begining,
$ egrep -n CDATA opac-opensearch.tt
25:     <title><![CDATA[.(removed).]]></title>
28:     <description><![CDATA[.(removed).]]></description>

In the translated version of the same file we have
25: <title><![CDATA[[.(removed).]]></title>
28: <description>.(removed).</description>

CDATA is removed but in 'description', not in 'title'!

Why one and not the other? Because of C4::TTParser, based on HTML::Parser. treats some tags differently (script, style, xmp, iframe, title, textarea and plaintext), see https://metacpan.org/pod/HTML::Parser

Anyway, the important thing is that ANY text inside CDATA block is IGNORED by the translation process, whichever the tag, and I was not aware of this.

To verify this: 
go to misc/translator, edit a new file 'test.tt' and put inside the lines

<title><![CDATA[ Title ]]></title>
<description><![CDATA[ Description ]]></description>

then create a translation file form it

./tmpl_process3.pl create -i ./ -f test.tt -s test.po

'test.po' will have nothing to translate!

Change the contents of 'test.po' to

<title> Title </title>
<description> Description </description>

and repeat,

rm test.po; ./tmpl_process3.pl create -i ./ -f test.tt -s test.po

'test.po' has now strings to translate.

Other rss files, 'opac-news-rss.tt' for example, do not use CDATA at all.

Question: Can we remove CDATA blocks from this files? 

It's the only 'fix' that I can think of.
Comment 8 Bernardo Gonzalez Kriegel 2019-12-07 10:43:43 UTC
> 
> Change the contents of 'test.po' to
> 

It's 'test.tt', sorry.
Comment 9 Bernardo Gonzalez Kriegel 2020-04-22 18:47:26 UTC
Comment on attachment 96076 [details] [review]
Bug 8937: Translation process removes xml prolog

Patch moved to Bug 25257
Comment 10 Victor Grousset/tuxayo 2020-04-23 23:52:12 UTC
I guess this patch can be closed right?

*** This bug has been marked as a duplicate of bug 25257 ***
Comment 11 Bernardo Gonzalez Kriegel 2020-04-24 00:13:28 UTC
(In reply to Victor Grousset/tuxayo from comment #10)
> I guess this patch can be closed right?
> 
> *** This bug has been marked as a duplicate of bug 25257 ***

Well, that CDATA is removed is still valid.

What I said previously is that our current translation process ignores text inside CDATA, but there is Bug 25067, when it gets in I'll revisit this one.

Then, I think that this is not a duplicate of 25257.

I wrote that patch following the comment #1 on this bug, but is not a solution to the reported problem, then I never switched this to 'NS'.
I open another bug and moved my patch there as a way to make it pass, it solves other little problem.

Please leave this open.
Comment 12 Victor Grousset/tuxayo 2020-04-24 00:43:50 UTC
> I wrote that patch following the comment #1 on this bug, but is not a solution to the reported problem, then I never switched this to 'NS'.

Thanks for the clarification, indeed it's not the same thing.