Bug 20195 - Untranslatable Show/Hide title attr replacement in opac detail
Summary: Untranslatable Show/Hide title attr replacement in opac detail
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: paxed
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-02-14 10:58 UTC by paxed
Modified: 2018-06-04 20:15 UTC (History)
4 users (show)

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


Attachments
[PATCH] Bug 20195: Untranslatable Show/Hide title attr replacement (3.78 KB, patch)
2018-02-14 10:59 UTC, paxed
Details | Diff | Splinter Review
Bug 20195: Untranslatable Show/Hide title attr replacement in opac detail (3.83 KB, patch)
2018-03-12 09:16 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 20195: Untranslatable Show/Hide title attr replacement in opac detail (3.90 KB, patch)
2018-03-12 15:27 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description paxed 2018-02-14 10:58:24 UTC
opac-detail.tt contains this:

   var newtitle = $(this).attr('title').replace('Show', 'Hide')
   $(this).attr('title',newtitle);

This tries to replace "Show" in the title attribute of this element:

<a href="#" id="a_listResults" title="Show pagination list ([% indexPag %]-[% indexPagEnd %] / [% totalPag %])">Browse results</a>

Which will of course fail on other languages.
(There's also the matching code to replace "Hide" with "Show")
Comment 1 paxed 2018-02-14 10:59:46 UTC
Created attachment 71606 [details] [review]
[PATCH] Bug 20195: Untranslatable Show/Hide title attr replacement

There's javascript code in opac detail view that tries to replace
the word "Show" with "Hide" (and vice versa) in a title attribute.
In addition to those words being untranslatable, a word replacement
like that would not work properly when using other languages.

Replace the single title attribute with two translatable strings,
one for the "Show" case and one "Hide", and use the whole
string instead of trying to replace a single word.

Test plan:

1) Make sure OpacBrowseResults is on
2) in OPAC, search the catalog and go to the detail view
3) Hover the mouse over the "Browse results" text in the grey box on the
   right side. The popup text should show something like
   "Show pagination list (1-5 / 5)"
4) Click on the "Browse results"
5) Hover the mouse again over the text. The popup should show
   "Hide pagination list (1-5 / 5)"
6) Update a language xx-YY, translate the new msgids
   "Show pagination list (%s-%s / %s)" and
   "Hide pagination list (%s-%s / %s)", and install the language
7) Repeat 2-5 with that language, making sure the popup
   texts show up correctly in that language
Comment 2 Jonathan Druart 2018-02-27 14:49:08 UTC
It will not work for JS turned off, maybe we should use a generic title by default?
Comment 3 Brendan Gallagher 2018-03-12 09:16:36 UTC
Created attachment 72627 [details] [review]
Bug 20195: Untranslatable Show/Hide title attr replacement in opac detail

There's javascript code in opac detail view that tries to replace
the word "Show" with "Hide" (and vice versa) in a title attribute.
In addition to those words being untranslatable, a word replacement
like that would not work properly when using other languages.

Replace the single title attribute with two translatable strings,
one for the "Show" case and one "Hide", and use the whole
string instead of trying to replace a single word.

Test plan:

1) Make sure OpacBrowseResults is on
2) in OPAC, search the catalog and go to the detail view
3) Hover the mouse over the "Browse results" text in the grey box on the
   right side. The popup text should show something like
   "Show pagination list (1-5 / 5)"
4) Click on the "Browse results"
5) Hover the mouse again over the text. The popup should show
   "Hide pagination list (1-5 / 5)"
6) Update a language xx-YY, translate the new msgids
   "Show pagination list (%s-%s / %s)" and
   "Hide pagination list (%s-%s / %s)", and install the language
7) Repeat 2-5 with that language, making sure the popup
   texts show up correctly in that language

Signed-off-by: Pasi Kallinen <pasi.kallinen@joensuu.fi>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 4 Brendan Gallagher 2018-03-12 09:18:12 UTC
Just seeing Jonathan's comment - should be a consideration for QA team.  Perhaps a generic title would work. (this patch works though)


(In reply to Brendan Gallagher from comment #3)
> Created attachment 72627 [details] [review] [review]
> Bug 20195: Untranslatable Show/Hide title attr replacement in opac detail
> 
> There's javascript code in opac detail view that tries to replace
> the word "Show" with "Hide" (and vice versa) in a title attribute.
> In addition to those words being untranslatable, a word replacement
> like that would not work properly when using other languages.
> 
> Replace the single title attribute with two translatable strings,
> one for the "Show" case and one "Hide", and use the whole
> string instead of trying to replace a single word.
> 
> Test plan:
> 
> 1) Make sure OpacBrowseResults is on
> 2) in OPAC, search the catalog and go to the detail view
> 3) Hover the mouse over the "Browse results" text in the grey box on the
>    right side. The popup text should show something like
>    "Show pagination list (1-5 / 5)"
> 4) Click on the "Browse results"
> 5) Hover the mouse again over the text. The popup should show
>    "Hide pagination list (1-5 / 5)"
> 6) Update a language xx-YY, translate the new msgids
>    "Show pagination list (%s-%s / %s)" and
>    "Hide pagination list (%s-%s / %s)", and install the language
> 7) Repeat 2-5 with that language, making sure the popup
>    texts show up correctly in that language
> 
> Signed-off-by: Pasi Kallinen <pasi.kallinen@joensuu.fi>
> 
> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
Comment 5 Katrin Fischer 2018-03-12 15:24:36 UTC
(In reply to Jonathan Druart from comment #2)
> It will not work for JS turned off, maybe we should use a generic title by
> default?

I just tested - Browse results doesn't work without Javascript, so I think we should move forward with the fix and worry about how to improve that bit later :)
Comment 6 Katrin Fischer 2018-03-12 15:27:33 UTC
Created attachment 72707 [details] [review]
Bug 20195: Untranslatable Show/Hide title attr replacement in opac detail

There's javascript code in opac detail view that tries to replace
the word "Show" with "Hide" (and vice versa) in a title attribute.
In addition to those words being untranslatable, a word replacement
like that would not work properly when using other languages.

Replace the single title attribute with two translatable strings,
one for the "Show" case and one "Hide", and use the whole
string instead of trying to replace a single word.

Test plan:

1) Make sure OpacBrowseResults is on
2) in OPAC, search the catalog and go to the detail view
3) Hover the mouse over the "Browse results" text in the grey box on the
   right side. The popup text should show something like
   "Show pagination list (1-5 / 5)"
4) Click on the "Browse results"
5) Hover the mouse again over the text. The popup should show
   "Hide pagination list (1-5 / 5)"
6) Update a language xx-YY, translate the new msgids
   "Show pagination list (%s-%s / %s)" and
   "Hide pagination list (%s-%s / %s)", and install the language
7) Repeat 2-5 with that language, making sure the popup
   texts show up correctly in that language

Signed-off-by: Pasi Kallinen <pasi.kallinen@joensuu.fi>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Jonathan Druart 2018-03-19 18:31:11 UTC
Pushed to master for 18.05, thanks to everybody involved!