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

Mailer refactoring #2993

Merged
merged 6 commits into from Dec 6, 2020
Merged

Conversation

gravitystorm
Copy link
Collaborator

This PR contains some translation-related refactoring for the user_mailer.

It took me two attempts to get this right, since there's some subtleties about how the _html translations interact with plain text mail templates, and so I had to revisit and refactor some (unsubmitted) work that was done last week when I realised what was going on. But I've added a test that would have caught my mistake earlier.

In short, _html translations take the translation options (e.g. something like t(".foo", :username => "Jack & Jill"), html_escapes the options, interpolates them into the translation, and returns the whole thing as a SafeBuffer to avoid further html_escaping in the template. Putting that straight into an html template is fine, since it a) is already a safebuffer and b) is already escaped. But for plain text templates, we don't want any html_escaping of the input options, so we have to avoid using _html translation keys. Hence the duplication between the _html and non-_html keys.

This makes it easier to rename methods and translation keys.
Rails doesn't escape text in text/plain outputs, since before Rails 4.

See rails/rails#8235
I spent some time working on an alternative translation strategy, without realising some of the plain text implications.
Use the _html suffix to mark that we expect the translations to contain links.

Unfortunately, we can't use the _html keys for the plain text emails, since the input options (e.g. username, url) will be html-escaped,
before it is passed to the view. So we need to use non-html-suffix keys for the plaintext views, in most cases. Only when the translation
options (e.g. url) are guaranteed to not contain any escapable characters can the same translation key be shared.
@tomhughes tomhughes merged commit 9271259 into openstreetmap:master Dec 6, 2020
@woodpeck
Copy link
Contributor

woodpeck commented Dec 8, 2020

There seems to be a small regression here. Since 06 December, when I get emails through the web site, the initial text ("so-and-so has sent you a message through OpenStreetMap") is HTML escaped, so that the anchor/link around the user name now appears in all its a target="_blank" rel="noopener" style="text-decoration: none; color: #222" glory directly in the message text!

@gravitystorm
Copy link
Collaborator Author

when I get emails through the web site, the initial text ("so-and-so has sent you a message through OpenStreetMap") is HTML escaped

argh, I'll look into this.

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

3 participants