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

Update i18n instruction on security risks #36869

Merged
merged 4 commits into from Sep 25, 2020
Merged

Update i18n instruction on security risks #36869

merged 4 commits into from Sep 25, 2020

Conversation

hacodeorg
Copy link
Contributor

Task FND-1259:

  • Update i18n README file to raise awareness about security risks when showing translation string that contains HTML elements.
  • Also safely un-escaping a translation string which has an URL by using markdown: true.

Links

Slack discussions
From Ben 8/12/2020
From Elijah 7/16/2020

@hacodeorg hacodeorg requested a review from a team as a code owner September 23, 2020 10:58
i18n/README.md Outdated Show resolved Hide resolved
i18n/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@Hamms Hamms left a comment

Choose a reason for hiding this comment

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

One minor nit, otherwise LGTM!

@@ -10,7 +10,7 @@
.external-link-disclaimer
%h2= t('external_links.disclaimer')
%strong= t('external_links.leaving_domain')
%p!= t('external_links.disclaimer_text', support_url: 'https://support.code.org')
%p!= t('external_links.disclaimer_text', support_url: 'https://support.code.org', markdown: true)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this should probably be either

Suggested change
%p!= t('external_links.disclaimer_text', support_url: 'https://support.code.org', markdown: true)
%p!= t('external_links.disclaimer_text', support_url: 'https://support.code.org', markdown: :inline)

or

Suggested change
%p!= t('external_links.disclaimer_text', support_url: 'https://support.code.org', markdown: true)
!= t('external_links.disclaimer_text', support_url: 'https://support.code.org', markdown: true)

the difference between the two being that the regular case means the markdown will handle its own block-level elements like paragraphs and lists and whatnot, while the inline case will produce content suitable for embedding inside an existing block-level element.

@hacodeorg hacodeorg merged commit 1b9a988 into staging Sep 25, 2020
@hacodeorg hacodeorg deleted the ha/i18n-readme branch September 25, 2020 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants