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

[PM-17590] fix chrome translation bug by escaping $ #13103

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

audreyality
Copy link
Member

@audreyality audreyality commented Jan 28, 2025

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-17590

📔 Objective

Fix a translation bug affecting Chrome browser.

Chrome removes invalid interpolation keys in its messages.json translations. This caused the generator on chromium browsers to render !@#$%^&* as !@#^&*. Unfortunately, the I18nService used by the web and desktop code renders $$ when properly escaping the i18n string. Thus, the solution is to hard-code the value.

Angular string interpolation is used as the fix because the entity escape sequence for @ looks like line noise.

This PR also removes unused translation keys as to avoid confusion in the future.

📸 Screenshots

Before:
image

After:
image

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

@audreyality audreyality requested a review from a team as a code owner January 28, 2025 15:19
@audreyality audreyality changed the title fix chrome translation bug by escaping $ [PM-17590] fix chrome translation bug by escaping $ Jan 28, 2025

This comment was marked as resolved.

This comment was marked as resolved.

@djsmith85 djsmith85 linked an issue Jan 28, 2025 that may be closed by this pull request
1 task
@audreyality audreyality marked this pull request as draft January 28, 2025 15:51
@audreyality audreyality force-pushed the tools/pm-17590/fix-special-characters-label branch from c1588cf to af6d315 Compare January 28, 2025 16:01
@audreyality audreyality marked this pull request as ready for review January 28, 2025 16:03
@audreyality audreyality requested a review from a team as a code owner January 28, 2025 16:03
@audreyality audreyality requested a review from JimmyVo16 January 28, 2025 16:03
JimmyVo16
JimmyVo16 previously approved these changes Jan 28, 2025
@audreyality audreyality marked this pull request as draft January 28, 2025 16:09
@audreyality
Copy link
Member Author

Fun fact: we can't localize this text because the i18n method used by the browser doesn't match the implementation used by web. This needs to be hard-coded in the view.

@audreyality audreyality marked this pull request as ready for review January 28, 2025 16:17
@audreyality audreyality requested a review from JimmyVo16 January 28, 2025 16:17
@audreyality audreyality added the needs-qa Marks a PR as requiring QA approval label Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-qa Marks a PR as requiring QA approval
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Misleading symbols option in Password Generator tab
3 participants