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

Exclude I18n reserved keys from html escaping #1953

Merged
merged 5 commits into from
Jan 8, 2024

Conversation

nickcoyne
Copy link
Contributor

What are you trying to accomplish?

With html translation keys, reserved keys are currently being made html safe, thus converting values like true to "true". This is disabling some of these keys.

What approach did you choose and why?

The existing code tries to match reserved keys using the ::I18n.reserved_keys_pattern, however they will never match, as the pattern is expecting interpolated strings with the translation value. It will match %{raise}, but not raise, which is what we want. So these reserved key values are all subsequently html escaped.

This PR uses the I18n::RESERVED_KEYS constant directly, and excludes those keys from options that will be considered for escaping.

Anything you want to highlight for special attention from reviewers?

The key that I chose for testing was :raise. This meant that I needed a missing translation, so I've added another locale to the I18n.available_locales. This also matched the original scenario in my app that caused me to discover the issue.

@nickcoyne nickcoyne requested a review from elia as a code owner January 5, 2024 00:52
@boardfish
Copy link
Collaborator

👋 Just a quick nudge to say #1308 means you'll need to rebase this branch for checks to pass.

@nickcoyne
Copy link
Contributor Author

👋 Just a quick nudge to say #1308 means you'll need to rebase this branch for checks to pass.

Thanks, done!

Copy link
Member

@joelhawksley joelhawksley left a comment

Choose a reason for hiding this comment

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

👏🏻

@joelhawksley joelhawksley enabled auto-merge (squash) January 8, 2024 16:03
@joelhawksley joelhawksley merged commit 5a55ad5 into ViewComponent:main Jan 8, 2024
39 checks passed
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.

3 participants