-
Notifications
You must be signed in to change notification settings - Fork 671
AO3-4590 Titles in RTL languages cause display issues in emails #5230
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
base: master
Are you sure you want to change the base?
Conversation
surround work/chapter title in mailer helper with bidi isolation characters so the title doesn't affect adjacent parenthesis. test is wip.
| end | ||
| str | ||
| # FIRST-STRONG ISOLATE and POP DIRECTIONAL ISOLATE respectively, so bidi titles won't affect the parenthesis around word counts, see AO3-4590 | ||
| self.title.present? ? t(".full_chapter_title", title: "⁨#{self.title}⁩", position: self.position) : t(".chapter_header", position: self.position) |
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.
I haven't fully checked whether this works yet, but the idea is that anything like a work/chapter title variable in the .yml file that should be bidi isolated will be formatted as such in the .t call or earlyer, so that by the time the variable is being handled by the translation team it's already been isolated and won't affect anything around it.
I thought this would make it "autofill" but it didn't work like that
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.
We had a discussion about the approach here:
- For HTML emails we'd like to go with
dir = "auto"on the HTML elements that contain the title, and also addunicode-bidi:plaintextin the CSS for those elements because gmail appears to sometimes discard the dir attribute but definitely supports the CSS - For plaintext emails FSI and PDI are indeed the correct approach, but the characters should not be used anywhere where we can use markup instead (like HTML emails or the interface)
This is based on the unicode standard annex 9 section 2.7.
So it looks like confining these changes to the mailer helpers and then consistently using only the mailer helpers for work/chapter titles in emails is the right move. Note that with the recently merged subscriptions email i18n the use of the methods you modified here has changed a bit, so it'd be good to merge master and adjust for that.
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
Handling of RTL/LTR work or chapter title in a LTR/RTL localized email in the specific case of putting the title next to a word count that's in parenthesis using the
creation_link_with_word_count/creation_title_with_word_countmailer helpers. So far this helper is only used inrecipient_notificationmailers. As part of this change tho, I changedfull_chapter_titlein thechaptermodel, and that is also used inbatch_subscription_notificationmailers.Testing Instructions
How can the Archive's QA team verify that this is working as you intended? wip
If you have a Jira account with access, please update or comment on the issue
with any new or missing testing instructions instead.
You can remove this section if there are already full testing instructions in the Jira issue.
References
Considerations taken from discussion on a previous declined PR linked in the Jira issue. (#4102)
Credit
Hamham6, it/its