Some strings need "no HTML entities" translator comments

[xibe]

This is a renewal of #10005, which was apparently my first patch to WP 6 years ago, but never got accepted. #10005 was closed as fixed, but it truth it was not (see my last comment there).

So, I'm opening a new ticket about this... and updating the description and patch from 6 years :)

Here goes!

Several strings need to have an indication warning translators against the inclusion of HTML entities within their translation, because of where the strings are user (RSS feeds, e-mails, JavaScript...).

Case in point: in French, there should be a non-breaking space ( ) before any double sign (; : ! ?). So, translators would turn "URL: %s" into "Adresse web : %s". But since it is used in e-mails, it is displayed as-is. And for strings that are used in feeds, it breaks them...

Suggestions comment: "Do not add HTML entities ( , etc): used in [context]".

Patch fixes all strings that I could find.

I do hope it is a simple enough patch, and not too close to RC, that it could make it into 4.3.

Thanks!

[xibe]

Patch for upgrade.php, feed-atom-comments.php, feed-rss2-comments.php, pluggable.php

[fxbenard]

+1 Making it for 4.3 will be a nice treat for all French translators (new comers - old elephants)

[xibe]

Note: for some strings, I chose to expand the code block.

For instance:

if ( EMPTY_TRASH_DAYS )
	$notify_message .= sprintf( __('Trash it: %s'), admin_url("comment.php?action=trash&c=$comment_id") ) . "\r\n";
else
	$notify_message .= sprintf( __('Delete it: %s'), admin_url("comment.php?action=delete&c=$comment_id") ) . "\r\n";

...is turned into:

if ( EMPTY_TRASH_DAYS ) {
	/* translators: Do not use HTML entities ( , etc.): this string is used in e-mails. */
	$notify_message .= sprintf( __('Trash it: %s'), admin_url("comment.php?action=trash&c=$comment_id") ) . "\r\n";
}
else {
	/* translators: Do not use HTML entities ( , etc.): this string is used in e-mails. */
	$notify_message .= sprintf( __('Delete it: %s'), admin_url("comment.php?action=delete&c=$comment_id") ) . "\r\n";
}

The line I add is a comment, so I'm unsure if it's necessary to add curly braces to mark the code block, but I want to play it safe.

[SergeyBiryukov]

#10005 was marked as a duplicate.

[SergeyBiryukov]

Replying to xibe:

The line I add is a comment, so I'm unsure if it's necessary to add curly braces to mark the code block, but I want to play it safe.

Good call, this usage of braces was added to the ​coding standards some time ago.

[zodiac1978]

+1

This a often a question on the i18n-table at contributor days.

Could/Should we use entities or not? - Depends on the context.

This would be good solution to add translator comments!

[xibe]

Good call, this usage of braces was added to the ​coding standards some time ago.

So I see! I'm guessing the ​PSR2 Fixer has not been run on the whole codebase (with the braces option)? But that's for another ticket...

[xibe]

A note by @nbachiyski from #10005:

We have computers on our side and we can do better than telling the translators not to use entities. In all cases we can properly escape entities if they aren't allowed. It can wait until next version, though.

Still, nothing happened in 6 years, so I'm submitting my patch idea. If anything better comes up, I'd be happy to forget it :)

[azaozz]

Replying to xibe:

Would it work if we run html_entity_decode() when using these strings? This is the case for all strings that are outputted for use in JS and as far as I know there aren't any problems with entities there.

[netweb]

Replying to xibe:

So I see! I'm guessing the ​PSR2 Fixer has not been run on the whole codebase (with the braces option)? (I know we do not use PSR-2, but the fixer could be useful in this case). But that's for another ticket...

See ​https://make.wordpress.org/core/2011/03/23/code-refactoring/, but the gist of that doc is "Code refactoring should not be done just because we can."

[xibe]

Replying to azaozz:

Would it work if we run html_entity_decode() when using these strings? This is the case for all strings that are outputted for use in JS and as far as I know there aren't any problems with entities there.

Good call! Actually, I see that the feed-atom-comments.php and feed-rss2-comments.php strings are already enclosed in ent2ncr() -- and the feeds don't break with "By: %s" translated to "Par : %s", so it seems we're good here.

For the mail related strings, I guess html_entity_decode() is indeed the better way, since it converts the entities to their applicable characters, whereas ent2ncr() "only" switches named entities to their numbered counterparts.

If so, I can take care of the patch.

[Presskopp]

We just had this issue/discussion in the german polyglots team, so we can see it's still a valid point to be patched. I'm not sure if we need comments to tell the translator to not use entities or if we'd need a software side patch to convert entities when they appear. However, I'd appreciate if this could go forward somewhere.

[La Geek]

We all give the best to receive a professional output of our translations. This should be above all also in the interest of WordPress.org. But we cannot achieve this if we are not shown when an entity is not transformed.

Please display a warning and prevent saving the translation in such cases.

This is a renewal of #10005, which was apparently my first patch to WP 6 years ago

and now additional 7 further years in total 13 years and still not supported? Really?

[galanos]

Seconding this. As I’ve learned, HTML entities sometimes must be inserted into translated strings. Making translators check the context first—HTML, JavaScript, e-mail, tattoo template, or whatnot—inevitably leads to errors and is non-inclusive by further raising the threshold of technical knowledge required of translators. Thank you!

[oglekler]

Hi @johnbillion,
can you have a look at the patch, it most likely needs a refresh, but you added needs-patch after patch was already there, so, I am wondering if you had in mind another solution instead of the one which was provided by patch.
Thank you 🙏

[audrasjb]

Given this enhancement still needs refresh and as WP 6.3 beta 1 is going to be released in a couple days, let's move this to milestone 6.4.

[oglekler]

What needs to be done:

1. Refreshing the patch because at least paths to files are changed.
2. Checking that it covers all pleases when this note should be added. It is possible that we have more of them.

Because it looks like a straight forward task, I am marking it as good first bug.

[costdev]

@oglekler As comment:18 mentions, asking translators to check the context may still lead to human error. comment:13 points to html_entity_decode() as a viable code-based solution. I believe needs-patch is correct because the previous patch's approach no longer seems to be the preferred one, and a new patch is needed.

[oglekler]

Thank you @costdev, good points 🙏

[oglekler]

We need a new patch and there is no time before Beta 1 to finish and test it, so, I am moving this ticket into the 6.5 milestone.

When translations are being worked on, translators may use html entities in their translations. In scenarios where translation doesn't happen in a browser, these can show as the raw set of characters for the entity, for instance &bnsp;. We can fix this by using the PHP function html_entity_decode inline in these scenarios to decode html entities that have potentially been used in translation input.

Trac ticket: https://core.trac.wordpress.org/ticket/33073

[benjaminknox]

I put up this PR ​https://github.com/WordPress/wordpress-develop/pull/5457 based on the attached patch by @xibe and the suggestion to use html_decode_entity function.

Do you guys think we need tests written for this?

[oglekler]

@costdev is it safe to add HTML into strings, or do we need additionally to check for allowed tags? Can you please take a look.

[swissspidy]

Asking translators to check the context may still lead to human error. comment:13 points to html_entity_decode() as a viable code-based solution. I believe needs-patch is correct because the previous patch's approach no longer seems to be the preferred one, and a new patch is needed.

IMHO we need both:

1. Translator comments so that translators do not use HTML entities (documentation)
2. Usage of html_entity_decode to fix any remaining HTML entities (hardening)

[audrasjb]

As per today's bug scrub, self assigning to help it moving towards a resolution during 6.6.

[dmsnell]

This dovetails the work in #61072 (​WordPress-develop#6387) where we're adding a spec-compliant HTML text decoder. Without wanting to stall this work, if it's coinciding I'd be interesting in avoiding new introductions of html_entity_decode() when we could rely on the new and more knowledgable function.

Maybe this is only an earmark to return to this issue after the new decoder is merged and available.

[oglekler]

We have 2 days before Beta 1 and no concrete understanding on how to proceed, so, I am moving this ticket to the next milestone for further consideration.

@audrasjb please feel free to move it back if you think it still can be tackled as part of 6.6.

[chaion07]

Thanks @xibe for reporting this. We reviewed this Ticket during a recent bug-scrub session. Here's the summary:

1. Peter ​informed of a merge conflict for the PR. He mentioned that he can fix it if this is small and easy enough otherwise it is suggested that the author can fix it.
2. Basically before commit, it needs to be certain that the edited strings aren't used in locations that do, in fact, require encoded entities (ie, HTML output rather than emails for example).
3. Peter also noted that the ​associated pull request is quite large so he thinks it could do with a lot of testing to get it towards merge.

Props to @peterwilsoncc

Cheers!

[davidbaumwald]

With 6.7 Beta 1 releasing in a few hours, this is being moved to Future Release given a lack of recent momentum towards a resolution.

If any committer feels the remaining work can be resolved in time for Beta 1 or any other specific milestone and wishes to assume ownership during that cycle, feel free to update the milestone accordingly.