Skip to content

Conversation

@nattsw
Copy link
Contributor

@nattsw nattsw commented Feb 24, 2025

Extra context:

In the earlier implementation of topic title translations, we send the topic.title to the API, and titles are shown inline to the post as below:

Screenshot 2025-02-24 at 6 48 37 PM

For the new experimental full-topic translations, we replace topic titles in the topic title container itself above the topic:

Screenshot 2025-02-24 at 6 50 30 PM

There is a problem here - sending a topic's title for translation for the experimental method is not safe since we directly replace the fancy_title attribute.

#<Topic:0x0000000122b75f20
 title: "<h1> a title </h1> 🐱",
 fancy_title: "&lt;h1&gt; a title &lt;/h1&gt; :cat:",

This results in translated titles that include the H1 tag.

Bug

Since we're sending the raw titles, this is what happens:

Screenshot 2025-02-24 at 7 27 39 PM

This PR sends our fancy_titles for translation instead.

Additional note: I am not creating a migration to update old values, as AI translations and full topic translations are still experimental.

@nattsw nattsw force-pushed the translate-fancy-titles branch from bdaa1dd to 1b38232 Compare February 24, 2025 12:02
@nattsw nattsw marked this pull request as draft February 24, 2025 12:04
@nattsw
Copy link
Contributor Author

nattsw commented Feb 25, 2025

Unfortunately this cannot be done.

Sending fancy_title for translation is good for the new feature, but would break the older feature where the titles are escaped. It would result in double escaping for the old featuer

Screenshot 2025-02-25 at 11 17 20 AM

@nattsw nattsw closed this Feb 25, 2025
nattsw added a commit that referenced this pull request Feb 25, 2025
In the existing method of translation, we show the translated titles below the post cooked.

When using the same already-translated titles for the experimental feature, it would result in the H1 rendering into large text in the topic title.

We've been sending the `topic.title` in for translation, instead of the escaped `topic.fancy_title`, which works well for the existing method. I had considered changing our implementation to [send the fancy_titles (escaped) in for translation instead](#222), but that would result in our existing methods titles being double-escaped.

To keep compatibility between existing and experimental, I will only escape it for the experimental feature. This commit does that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants