backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 40 forks source link

The page to import translation does not import the translation of strings like "<none>" #6625

Open avpaderno opened 3 months ago

avpaderno commented 3 months ago

To reproduce the error:

The form will show the following error.

A translation string was skipped because of disallowed or malformed HTML. See the log for details.

12581df46876e1ab63b71d9b5c047735

The error is shown because the translation provided in Italian is <nessuno> which is considered as HTML tag that does not exist.

I understand why the error is shown, but at least it should be a warning, and the translated string should be imported.
The log shows which string causes the error. In the case the error is changed to a warning, and the translated string imported, administrator users could still change the translation, if necessary.

avpaderno commented 3 months ago

The reason for which I am asking for the error to be changed in a warning is that, using the example I did in the issue summary, <none> is already used in the English text. If there is any issue, that issue is present even in a site that uses only English as site language.

A warning, which would make the person who imported the translations check the translated string is fine, but avoiding the string is imported seems excessive.

avpaderno commented 3 months ago

I changed the issue title, as the same would happen when the translation string is <none>, although that should not normally happen. (If translators decide to use the same string used in English, they would probably leave the translated string empty.)

indigoxela commented 3 months ago

I thought this over a bit and my conclusion is: core shouldn't use a string that wouldn't be valid as translation.

Currently we present something, that people can't actually translate in a way, that only changes the language. We also can't use something like &lt; btw, as that would lead to double encoding.

What we can do: replace that problematic <none> with – none –, which is easy to translate to, for example – nessuno –. (Or alternatively move the brackets out of the translatable string.)

But some more feedback from active translators might be helpful for a decision. @olafgrabienski, @robertgarrigos, or whoever feels addressed: would that make your life easier or harder? :wink:

For some context: this string is, for example, used in the number module formatter settings form to select a decimal point character or none.

olafgrabienski commented 3 months ago

When I translate such strings, I already use dashes. So, in my opinion, using dashes instead of angle brackets in the original string is a good idea.

Another example is the Label for "Any" value on the page admin/structure/views/settings, which defaults to <Any>.

avpaderno commented 3 months ago

The function that does not accept a string containing '<none>' or '<nessuno>' is locale_string_is_safe() which only accepts the following tags: <a>,<abbr>,<acronym>,<address>,<b>,<bdo>,<big>,<blockquote>,<br>,<caption>,<cite>,<code>,<col>,<colgroup>,<dd>,<del>,<dfn>,<dl>,<dt>,<em>,<h1>,<h2>,<h3>,<h4>,<h5>,<h6>,<hr>,<i>,<ins>,<kbd>,<li>,<ol>,<p>,<pre>,<q>,<samp>,<small>,<span>,<strong>,<sub>,<sup>,<table>,<tbody>,<td>,<tfoot>,<th>,<thead>,<tr>,<tt>,<ul>,<var>.

indigoxela commented 3 months ago

@kiamlaluno sure, but the accepted tags are supposed to be HTML tags (markup), but this <none> isn't. It's just to visually highlight the non-selection a bit. And that could get achieved with other characters - more appropriate ones IMO. Characters that don't need special treatment (escaping) in HTML.

avpaderno commented 3 months ago

I checked with it grep -in -P '( s| )t\((1x22|\x27).*\<[a-z]+\>' -- "*.php" "*.inc" "*.module" "*.install" which files pass strings containing words between < and > to t().

'<none>' is only used twice. '<Any>' is used once, in core/modules/views/handlers/views_handler_filter.inc, from code that uses '<Any>' only for compatibility with Drupal 7 (or older Backdrop versions).

$any_label = config_get('views.settings', 'exposed_filter_any_label') == 'old_any' ? t('<Any>') : t('- Any -');

In the other cases, the strings passed to t() contain only the allowed tags I listed in my previous comment.

I would say that, except for <Any> used for compatibility with Drupal 7 (or older Backdrop versions?), the code can be changed to avoid <none> in translatable strings.

(Yes, I know that <none> is not a valid HTML tag; that list served me to write this comment.)

indigoxela commented 3 months ago

Works for me! I tested in the sandbox with German as additional language and - none - translated.

To see this translation, there are two places.

  1. A view of files with a path field set to "Use full URL..."
  2. A number field of type decimal on a node, it's in the display settings then (Thousand marker)

Both work as expected.

I took a quick look, where this old_any comes from - it's inherited from early D8.