| Summary: | TT tags breaking translation script when used to build a JS string | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
| Component: | I18N/L10N | Assignee: | Bugs List <koha-bugs> |
| Status: | NEW --- | QA Contact: | Testopia <testopia> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | dcook, f.demians, jonathan.druart, julian.maurice |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | All | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Comma delimited list of Sponsors: | |
| Crowdfunding goal: | 0 | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Bug Depends on: | 38147 | ||
| Bug Blocks: | |||
|
Description
Jonathan Druart
2024-10-21 09:14:01 UTC
Find possible suspects:
git grep "_(" **/*.tt|grep '\[%'
This one is confirmed at least:
koha-tmpl/intranet-tmpl/prog/en/modules/admin/identity_provider_domains.tt: var result = '<a class="btn btn-default btn-xs" role="button" href="/cgi-bin/koha/admin/identity_providers.pl?
domain_ops=1&identity_provider_id=[%- identity_provider_id | html -%]&op=edit_form&identity_provider_domain_id='+ encodeURIComponent(row.identity_provider_domain_id) +'"><i class="fa-solid fa-pencil" aria-hidden="true"></i> '+_("Edit")+'</a>'+"\n";
(In reply to Jonathan Druart from comment #0) > So the solution I found is to use an intermediate JS variable: > let bar = "[% bar | uri %]"; > str = '<a foo="%s" bar="%s">%s</a>'.format(foo, bar, _("Click me")) Another option would be something like: let link = document.createElement('a'); link.textContent = _("Click me"); link.setAttribute('foo',foo); link.setAttribute('bar',bar); str = link.outerHTML; -- While it's more verbose, it's safe from XSS and should be translation friendly. In the quoted version, both "foo" and "bar" are potentially vulnerable to XSS if they contain unsanitized user input. In any case, is this translation issue a new behaviour or has this problem existed previously? (In reply to David Cook from comment #4) > In any case, is this translation issue a new behaviour or has this problem > existed previously? From Mattermost: """ Issue with the translation script that has been around for years. We reject the structure (TT tags inside HTML tags) from the QA script, but this is a new structure that must be rejected OR we fix the translation script, but IIRC it is not trivial. """ (In reply to David Cook from comment #3) > In the quoted version, both "foo" and "bar" are potentially vulnerable to > XSS if they contain unsanitized user input. We should call escape_str on all JS vars, yes. But it's not the point of the bug report. (In reply to Jonathan Druart from comment #5) > From Mattermost: > """ > Issue with the translation script that has been around for years. We reject > the structure (TT tags inside HTML tags) from the QA script, but this is a > new structure that must be rejected Yeah I still don't understand what you're saying there. What's the "new structure"? (In reply to Jonathan Druart from comment #6) > (In reply to David Cook from comment #3) > > In the quoted version, both "foo" and "bar" are potentially vulnerable to > > XSS if they contain unsanitized user input. > > We should call escape_str on all JS vars, yes. But it's not the point of the > bug report. I didn't say it was. Just pointing out the robustness of the alternative I suggested. (In reply to David Cook from comment #7) > (In reply to Jonathan Druart from comment #5) > > From Mattermost: > > """ > > Issue with the translation script that has been around for years. We reject > > the structure (TT tags inside HTML tags) from the QA script, but this is a > > new structure that must be rejected > > Yeah I still don't understand what you're saying there. What's the "new > structure"? The one we knew that was problematic, when building HTML nodes: <a href="#" [% IF required %]required[% END %]>link</a> We catch it with the QA script, and there is a test: xt/tt_valid.t The new one, discovered here, when building nodes using JS. (In reply to Jonathan Druart from comment #9) > The one we knew that was problematic, when building HTML nodes: > <a href="#" [% IF required %]required[% END %]>link</a> > We catch it with the QA script, and there is a test: xt/tt_valid.t > > The new one, discovered here, when building nodes using JS. Ok I think I understand better now. Thanks. (That is, it's an old problem in the translation process, but it's a new problem since we're only noticing it now.) It seems to me that we should decide on a new method (or methods) and add them as coding guidelines? If possible, catch in QA script, although I don't know how easy that would be. koha-tmpl/intranet-tmpl/prog/en/modules/authorities/authorities.tt
113 var content = "<h3>" + _("Authority number") + ": [% authid | html %]</h3><hr>";
this one is correctly picked.
So I've tried this:
var content = "<h3>" + _("Authority number") + ": [% authid | html %]</h3><hr>";
content += "<h3>[% authid | html %] " + _("Authority number") + "</h3><hr>";
The second one is not picked, certainly because the TT tag is before.
In this occurrences, only "Send email" is not picked
koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/detail.tt
1623 $("#elasticPreview .modal-body").html("<h1>"+_("Une erreur s'est produite !")+"</h1><h2><em>"+_("Erreur 404")+"</em></h2><ul><li>"+_("Un lien interne dans l'interface professionnel le est cassé et la page n'existe pas")+"</li></ul><h3>"+_("Quoi d'autre ?")+"</h3><ul style='margin-bottom: 1em; padding-bottom: 1em; border-bottom: 1px solid #CCC;'><li>"+_("Utilisez la barre de menu princip ale pour vous déplacer vers une autre partie de Koha.")+"</li><li>"+_("Pour signaler un lien cassé ou tout autre problème, veuillez prendre contact avec l'administrateur Koha.")+" <a href='mailto:[% Koha.Pref erence("KohaAdminEmailAddress") | uri %]'>"+_("Send email")+"</a></li></ul>");
Also "Cancel":
koha-tmpl/intranet-tmpl/prog/en/modules/admin/background_jobs.tt: result += '<a class="btn btn-default btn-xs submit-form-link" role="button" href="#" data-action="/cgi-bin/koha/admin/background_jobs.pl" data-method="post" data-op="cud-cancel" data-confirmation-msg="[% t('Are you sure you want to cancel this job?') | html %]" data-id="'+ encodeURIComponent(row.job_id) +'"><i class="fa fa-trash-can" aria-hidden="true"></i> '+_("Cancel")+'</a>';
I think we have all the occurrences with this command: `git grep -P '\[%.*_\(' **/*.tt **/*.inc`, but there are false positives.
"Summary" is not picked
koha-tmpl/intranet-tmpl/prog/en/modules/pos/register.tt: var result = '<a class="btn btn-default btn-xs" role="button" data-bs-toggle="modal" data-cashup="'+encodeURIComponent(row.cashup_id)+'" data-register="[% register.description | html %]" href="#cashupSummaryModal"><i class="fa-solid fa-pencil" aria-hidden="true"></i> '+_("Summary")+'</a>\n';
However this one is not a problem:
koha-tmpl/intranet-tmpl/prog/en/modules/tools/manage-marc-import.tt: + diff_url.format( [% import_batch_id | html %], aData['import_record_id'], item.candidate_match_id, item.record_type) + '">' + _("View") + '</a></li>');
|