Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[I18n] Fix pseudo-localization of i18n directive placeholders #26948

Conversation

LeanidShutau
Copy link
Contributor

@LeanidShutau LeanidShutau commented Dec 11, 2018

Bug
In #26274 we have added @I18N@ placeholders for html values in i18n-values attribute. Pseudo-localization localizes these placeholders, so they cannot be replaced by their original values after pseudo-localization. It breaks messages with html_ values when en-xa locale is used.

Fix
Update pseudo-localization RegExp to ignore @I18N@valueName@I18N@ sub-strings.

How to test
Enable pseudo-locale (yarn start --i18n.locale en-xa or set i18n.locale: "ex-xa" in kibana.yml).
Open Timelion -> Help:

  • without fix there will be localized @I18N@ placeholders in the text and no rendered html code;
  • with the fix there will be rendered HTML code (e.g. <strong> Next word on the first page of Timelion Help).

@LeanidShutau LeanidShutau added bug Fixes for quality problems that affect the customer experience Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Project:i18n v6.6.0 labels Dec 11, 2018
@LeanidShutau LeanidShutau self-assigned this Dec 11, 2018
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@LeanidShutau LeanidShutau added this to In progress in I18n Dec 11, 2018
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@LeanidShutau LeanidShutau moved this from In progress to Review in I18n Dec 11, 2018
Copy link
Contributor

@pavel06081991 pavel06081991 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked locally. Works as expected.

@azasypkin azasypkin requested review from Bamieh and removed request for azasypkin December 11, 2018 12:40
@LeanidShutau LeanidShutau removed the request for review from maryia-lapata December 11, 2018 12:46
Copy link
Member

@Bamieh Bamieh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you!

Do you think its possible to add 1 or 2 small unit tests for the part you added? i know the whole file is not tested but it would help to have those tests in place even if only for that small part alone.

@Bamieh
Copy link
Member

Bamieh commented Dec 13, 2018

@LeanidShutau Looks good, placeholders are only words? no /[-_.]/ are allowed?

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Member

@Bamieh Bamieh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@LeanidShutau LeanidShutau merged commit 848121f into elastic:master Dec 14, 2018
@LeanidShutau LeanidShutau deleted the fix/i18n-placeholders-pseudo-localization branch December 14, 2018 10:38
@LeanidShutau LeanidShutau moved this from Review to Backporting in I18n Dec 14, 2018
LeanidShutau added a commit to LeanidShutau/kibana that referenced this pull request Dec 14, 2018
…c#26948)

* [I18n] Fix pseudo-localization of i18n directive placeholders

* Add a unit test

* Add more tests
LeanidShutau added a commit that referenced this pull request Dec 17, 2018
#27192)

* [I18n] Fix pseudo-localization of i18n directive placeholders

* Add a unit test

* Add more tests
@LeanidShutau
Copy link
Contributor Author

6.x/6.6: 85780a2

@LeanidShutau LeanidShutau moved this from Backporting to Done in I18n Dec 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported bug Fixes for quality problems that affect the customer experience Project:i18n Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v6.6.0
Projects
I18n
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants