humhub / translation

Internal translation tool
https://translate.humhub.org
Apache License 2.0
17 stars 15 forks source link

Could not translate text #40

Closed luke- closed 3 years ago

luke- commented 3 years ago

When copy 1:1 original text to the translated value, following error appears: image

See also: https://translate.humhub.org/content/perma?id=19015

ArchBlood commented 3 years ago

The issue seems to be here

https://github.com/humhub/humhub-modules-twofa/blob/master/views/config/userGoogleAuthenticatorCode.php#L18

Should look like this :thinking:

['docLinkAttrs' => ' href="https://support.google.com/accounts/answer/1066447" target="_blank"']); ?></p>
yurabakhtin commented 3 years ago

@luke- I have investigated the problem in that I created a not allowed text for translation <a{docLinkAttrs}>Google Authenticator</a>, because on save the entered translation text is processed by this code https://github.com/humhub/humhub-modules-translation/blob/master/models/TranslationLog.php#L208 $translationPurified = TranslationPurifier::process($this->translation); so the text is converted into <a>Google Authenticator</a> without expected {docLinkAttrs}.

I think to change the original text in 2FA module to new: 'Install an application that implements a time-based one-time password (TOTP) algorithm, such as <a href="{docUrl}" target="_blank">Google Authenticator</a>, and use it to scan the QR code shown below.' but please see there we need target="_blank" and I am not sure it is supported because here https://github.com/humhub/humhub-modules-translation/blob/master/models/parser/TranslationPurifier.php#L19 it seems only attribute href is supported for <a>.

luke- commented 3 years ago

@yurabakhtin Thank you for the analysis. We should probably then change the link as suggested. I would suggest we add the target argument to the Purifier, what do you think?

buddh4 commented 3 years ago

Can't we just replace the whole link element, I think the link does not need to be translated, would also improve the translation usability? Adding target to the purifier should be done nontheless, maybe just use the following config: https://github.com/humhub/humhub/blob/develop/protected/humhub/modules/content/widgets/richtext/converter/RichTextToHtmlConverter.php#L87 _blank links should also add noopener nofollow which I think is done by the prufyer automatically if you add and allow target="_blank"

yurabakhtin commented 3 years ago

@buddh4 I have replaced the whole link in PR https://github.com/humhub/humhub-modules-twofa/pull/15 because currently the attribute target is not supported yet by this module:

target_not_supported

Let me know if I should modify this module in order to support target, but I think translators should not think about such data as link attributes and maybe better to replace whole link, but of course sometimes better to know a text of a link because it may be an important part of the translatable phrase.

luke- commented 3 years ago

@yurabakhtin Thanks for the PR and yes, please add the target support to the purifier here. Unfortunately, there are a always be cases where a link within the text may required.

yurabakhtin commented 3 years ago

@luke-

please add the target support to the purifier here.

PR https://github.com/humhub/humhub-modules-translation/pull/43

@buddh4

_blank links should also add noopener nofollow which I think is done by the prufyer automatically if you add and allow target="_blank"

You are right partly because it adds noreferrer noopener for any value inside the attribute target:

target
buddh4 commented 3 years ago

In which cases is the translation of such links not possible? I think only if you require a language specific url? If you need to translate the link text you can always do something like this:

Yii::t('MyModule.base','This is a {link}', [
  'link' => Html::a(Yii::t('MyModule.base', 'Link text', ...)
])

I think we should prevent markup in the translation texts where possible, since translating parameterized translation texts can get confusing, especially since not all translators are familiar with html.

yurabakhtin commented 3 years ago

@buddh4 Yes I agree with you that firstly translators should not be familiar with html. So if it is enough I will use only {link} instead of html tags in translatable strings for all strings in my future code. But by this way translations will not be ideal for some languages like my because one word may has several forms depending on context.

buddh4 commented 3 years ago

You mean the seperation of Link text and This is a {link} is may confusing in some languages, since the translator does not know those two texts are related? I think the context in general sometimes is confusing, since a translator not always knows where the text is actually used.

yurabakhtin commented 3 years ago

@buddh4

You mean the seperation of Link text and This is a {link} is may confusing in some languages, since the translator does not know those two texts are related?

Yes.

buddh4 commented 3 years ago

Maybe we should define a best practice guideline for translation patterns for the translation categories as:

In the past we had an almost file based category seperation, which was too much seperation, then migrated to an almost base category only approach. I think we need a guideline when to use which category and when to create a new category, if for example the This is a {link} example would be used only in the module configuration, it should not be too hard to find the right place if we have a configuration translation category for this module.