Bug 18754 - Translatability: Get rid of exposed tt directives in opac-detail.tt
Summary: Translatability: Get rid of exposed tt directives in opac-detail.tt
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Marc Véron
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-06-08 04:55 UTC by Marc Véron
Modified: 2017-10-01 22:28 UTC (History)
5 users (show)

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


Attachments
Bug 18754: Translatability: Get rid of exposed tt directives in opac-detail.tt (17.92 KB, patch)
2017-06-09 13:45 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18754: Translatability: Get rid of exposed tt directives in opac-detail.tt (18.92 KB, patch)
2017-06-11 06:58 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18754: Translatability: Get rid of exposed tt directives in opac-detail.tt (19.02 KB, patch)
2017-08-02 05:09 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18754: Translatability: Get rid of exposed tt directives in opac-detail.tt (19.11 KB, patch)
2017-08-04 14:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18754: [QA Follow-up] Tiny corrections (3.26 KB, patch)
2017-08-04 14:49 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 Marc Véron 2017-06-08 04:55:54 UTC
The file opac-detail.tt exposes a lot of template directives to translation where translators should not be confronted with.

Some examples from po file are:

$[% SUBFIELD.code %] [% SUBFIELD.value %]

%s [%% INCLUDE \"openlibrary-readapi.inc\" bib = { normalized_isbn => "
"normalized_isbn, lccn => lccn, normalized_oclc => normalized_oclc } %%]

See: [% IF ( nextTitle ) %][% nextTitle |html %][% ELSE %]next biblio[% END "
"%]
Comment 1 Marc Véron 2017-06-09 13:45:28 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2017-06-11 06:58:58 UTC
Created attachment 64178 [details] [review]
Bug 18754: Translatability: Get rid of exposed tt directives in opac-detail.tt

The file opac-detail.tt exposes a lot of template directives to translation where translators should not be confronted with.

Some examples from po file are:

$[% SUBFIELD.code %] [% SUBFIELD.value %]

%s [%% INCLUDE \"openlibrary-readapi.inc\" bib = { normalized_isbn => "
"normalized_isbn, lccn => lccn, normalized_oclc => normalized_oclc } %%]

See: [% IF ( nextTitle ) %][% nextTitle |html %][% ELSE %]next biblio[% END "
"%]

To test:
- Apply patch
- Do a search in OPAC that has more than 1 results
- Go to the detail page of one of the items found
- Verify that the details display as before and that you can
  browse the results with Previous and Next
- In staff client, change OPACXSLTDetailsDisplay from 'default' to
  empty for "no xslt" and repeat steps above
- In staff client, set HTML5MediaEnabled to 'OPAC' or 'OPAC and staff client'
- Verify that media catalogued in field 856 still work
- Create a new translation for a 'language' aa-AA (perl translate create aa-AA)
- Verify that template directives ar no longer exposed in aa-AA-opac-bootstrap.po
Comment 3 Marc Véron 2017-07-20 08:10:52 UTC
Still applies on current master
Comment 4 Alex Buckley 2017-08-02 05:09:11 UTC
Created attachment 65412 [details] [review]
Bug 18754: Translatability: Get rid of exposed tt directives in opac-detail.tt

The file opac-detail.tt exposes a lot of template directives to translation where translators should not be confronted with.

Some examples from po file are:

$[% SUBFIELD.code %] [% SUBFIELD.value %]

%s [%% INCLUDE \"openlibrary-readapi.inc\" bib = { normalized_isbn => "
"normalized_isbn, lccn => lccn, normalized_oclc => normalized_oclc } %%]

See: [% IF ( nextTitle ) %][% nextTitle |html %][% ELSE %]next biblio[% END "
"%]

To test:
- Apply patch
- Do a search in OPAC that has more than 1 results
- Go to the detail page of one of the items found
- Verify that the details display as before and that you can
  browse the results with Previous and Next
- In staff client, change OPACXSLTDetailsDisplay from 'default' to
  empty for "no xslt" and repeat steps above
- In staff client, set HTML5MediaEnabled to 'OPAC' or 'OPAC and staff client'
- Verify that media catalogued in field 856 still work
- Create a new translation for a 'language' aa-AA (perl translate create aa-AA)
- Verify that template directives ar no longer exposed in aa-AA-opac-bootstrap.po

Followed test plan which works as intended

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 5 Marcel de Rooy 2017-08-04 06:17:53 UTC
I am wondering if constructions like those below should be considered as improvements to the template (recognizing translation benefits). But as always, we have to find some balance.

[% IF ( collectionvolume ) %] [% ' ; ' _ collectionvolume %] [% END %]

[% ' ( ' _ (itemloop.size || 0) _ ' )' %]

Another QA/RM opinion please.
Comment 6 Marcel de Rooy 2017-08-04 06:19:00 UTC
Given the number of hard-to-read lines, I would not mark this one as trivial btw.
Comment 7 Marc Véron 2017-08-04 07:56:56 UTC
(In reply to Marcel de Rooy from comment #6)
> Given the number of hard-to-read lines, I would not mark this one as trivial
> btw.

Who can better deal with code - translators or developers? We should avoid to expose code or code fragents to translators IMO, otherwise we risk that translators break something.
Comment 8 Owen Leonard 2017-08-04 11:42:47 UTC
I would vote for added complexity in the templates in order to make translation easier.
Comment 9 Marcel de Rooy 2017-08-04 14:31:00 UTC
Just a note about BLOCK..INCLUDE versus BLOCK..PROCESS:

The PROCESS directive is slightly faster than INCLUDE because it avoids the need to localise (i.e. copy) the variable stash before processing the template.
Comment 10 Marcel de Rooy 2017-08-04 14:49:48 UTC
Created attachment 65504 [details] [review]
Bug 18754: Translatability: Get rid of exposed tt directives in opac-detail.tt

The file opac-detail.tt exposes a lot of template directives to translation where translators should not be confronted with.

Some examples from po file are:

$[% SUBFIELD.code %] [% SUBFIELD.value %]

%s [%% INCLUDE \"openlibrary-readapi.inc\" bib = { normalized_isbn => "
"normalized_isbn, lccn => lccn, normalized_oclc => normalized_oclc } %%]

See: [% IF ( nextTitle ) %][% nextTitle |html %][% ELSE %]next biblio[% END "
"%]

To test:
- Apply patch
- Do a search in OPAC that has more than 1 results
- Go to the detail page of one of the items found
- Verify that the details display as before and that you can
  browse the results with Previous and Next
- In staff client, change OPACXSLTDetailsDisplay from 'default' to
  empty for "no xslt" and repeat steps above
- In staff client, set HTML5MediaEnabled to 'OPAC' or 'OPAC and staff client'
- Verify that media catalogued in field 856 still work
- Create a new translation for a 'language' aa-AA (perl translate create aa-AA)
- Verify that template directives ar no longer exposed in aa-AA-opac-bootstrap.po

Followed test plan which works as intended

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Marcel de Rooy 2017-08-04 14:49:53 UTC
Created attachment 65505 [details] [review]
Bug 18754: [QA Follow-up] Tiny corrections

Converted one INCLUDE directive to PROCESS; we are not changing variables here. (The PROCESS directive is slightly faster than INCLUDE because it avoids the need to localise (i.e. copy) the variable stash before processing the template.)

Removed one vim inserted letter i.

Error in [% IF ( XISBN.publicationyear ) _ ', ' _ XISBN.publicationyear %][% END %] The concatenation became part of the condition.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2017-08-04 14:51:34 UTC
(In reply to Marc Véron from comment #7)
> (In reply to Marcel de Rooy from comment #6)
> > Given the number of hard-to-read lines, I would not mark this one as trivial
> > btw.
> 
> Who can better deal with code - translators or developers? We should avoid
> to expose code or code fragents to translators IMO, otherwise we risk that
> translators break something.

(In reply to Owen Leonard from comment #8)
> I would vote for added complexity in the templates in order to make
> translation easier.

Okay. Send it to the next level.
Comment 13 Jonathan Druart 2017-08-30 19:59:35 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 14 Fridolin SOMERS 2017-09-29 13:45:24 UTC
Pushed to 17.05.x, will be in 17.05.05.
Comment 15 Katrin Fischer 2017-10-01 22:28:05 UTC
These patches have been pushed to 16.11.x and will be in 16.11.13.