-
Notifications
You must be signed in to change notification settings - Fork 671
AO3-4590 Fixing display of parentheses after RTL language titles in notification emails #4102
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
AO3-4590 Fixing display of parentheses after RTL language titles in notification emails #4102
Conversation
sarken
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the pull request! I agree we can't fix it everywhere at once, but it looks like the issue mentions a couple of emails that are affected and are not currently covered by this pull request:
- Subscription emails (batch_subscription_notification.text.erb & batch_subscription_notification.html.erb)
- Prompt fill emails (prompter_notification.html.erb & prompter_notification.text.erb)
Could you update those as well?
|
I'm not sure how I missed those, thank you for pointing them out. I'll add them. |
sarken
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! Unfortunately, after some research and discussion, we don't think this is the best way to tackle this issue.
For the HTML emails, it looks like the bdi tag doesn't have great support:
- https://www.caniemail.com/search/?s=bdi
- https://www.pinpointe.com/blog/email-campaign-html-and-css-support
And with the text emails, there are a couple of potential issues:
- While a LTR mark might work for English emails, we're in the process of translating emails, and we'll probably need a RTL mark for the opposite case (e.g., Hebrew emails with English titles).
- Using invisible characters is difficult to maintain, particularly with 40 language teams who will be handling strings like
"%{work.title}" (%{word_count} words).
If you have any ideas for a different way to handle this, we'd love to hear them! However, we're going to close this pull request for now, until we decide on an approach that will work for us.
| A gift work has been posted for you<% if @collection %> in the "<%= @collection.title %>" collection (<%= collection_url(@collection) %>)<% end %> at the Archive of Our Own! | ||
|
|
||
| "<%= @work.title.html_safe %>" (<%= @work.word_count %> words) | ||
| "<%= @work.title.html_safe %>" (<%= @work.word_count %> words) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be using the RTL mark (U+200F) while the other emails use the LTR mark (U+200E).
Pull Request Checklist
as the first thing in your pull request title (e.g.
AO3-1234 Fix thing)until they are reviewed and merged before creating new pull requests.
Issue
https://otwarchive.atlassian.net/browse/AO3-4590
Purpose
This PR adds bdi (bi-directional isolation) tags to the HTML for notification emails, and a Unicode left-to-right mark to the text form of notification emails to prevent titles in right-to-left languages from causing weakly directional characters (such as numbers and parentheses) from displaying incorrectly.
This does not fix all possible locations where this could be a problem -- I think fixing this everywhere on the site probably requires doing something tricky with Rails' string interpolation -- but it does fix the places where the error was initially reported in the original ticket. We can use the same fix in other places, as it comes up.
Testing Instructions
This can be tested by generating an update notification email for a work with a title written in a right-to-left language like Hebrew or Arabic, and checking that the word count displayed after the title appears correct. This should be repeated for an account that gets HTML emails and an account that gets text emails.
References
None
Credit
What name and pronouns should we use to credit you in the Archive of Our Own's Release Notes?
Daroc Alden, they/them