[DX] Is the use of `get_t()` in the output of `theme_form_element_label()` really necessary?

[klonos]

Starting this as a question for now.

So the output/return of the theme_form_element_label() function is this (lines wrapped here for readability):

return ' <label' . backdrop_attributes($attributes) . '>' . $t('!title !required', array(
  '!title' => $title,
  '!required' => $required,
)) . "</label>\n";

Notice how the only things passed through $t() (which is basically a call to $t = get_t();) are variables/replacements and not actual translatable text:

• !title is mapped to the $title variable, which is previously defined as $title = filter_xss_admin($element['#title']);. But element '#title's are always passed through t() anyway.

• !required is mapped to $required, which is basically theme_form_required_marker(), which in turn is this:

  function theme_form_required_marker($variables) {
  // This is also used in the installer, pre-database setup.
  $t = get_t();
  $attributes = array(
    'class' => 'form-required',
    'title' => $t('This field is required.'),
  );
  return '<abbr' . backdrop_attributes($attributes) . '>*</abbr>';
  }

  So again, the title attribute which is essentially what needs to be translated is already passed through get_t()

So does it make sense to have the return of theme_form_element_label() as it is now (variables that are passed through get_t() again when they have already been passed through get_t()/t() previously), or should it simply be like so instead?:

return ' <label' . backdrop_attributes($attributes) . '>' . $title . $required . "</label>\n";

[indigoxela]

So does it make sense to have the return of theme_form_element_label()

I'd like to point you to a subtle detail: '!title' Note the exclamation mark - it's just pass-through. https://docs.backdropcms.org/api/backdrop/core%21includes%21bootstrap.inc/function/format_string/1

[klonos]

Thanks @indigoxela, but I am still not getting it. I saw the detail, but if we are passing the value through for both the $title as well as the $required variables, then what is the point of using get_t()? I just don't understand what we are trying to achieve. What am I missing?

It seems to me that if the only string that we are passing through get_t() is '!title !required', then there is no point in using get_t().

[avpaderno]

Actually, code like t('!title !required', array('!title' => $title, '!required' => $required) allows to translators, who get !title !required as text to translate, to decide in which order the placeholders are used. (I guess that some languages that are read right-to-left could use *!required !title.)

t('!title', array('!title' => $title) would be different. Translators could not replace !text with Titolo: !title, for example, which is instead the correct Italian translation for Title: !title.

[klonos]

@kiamlaluno !required is basically an asterisk inside an <abbr> HTML tag, with a specific/fixed title as an attribute, and the value of that attribute is always "This field is required.", which is already passed through get_t() previously (in theme_form_required_marker()). So it is basically an already translated version of this:

<abbr class="form-required" title="This field is required.">*</abbr>

When that markup reaches theme_form_element_label(), it is already the translated version of the English one, so in Italian it will already be something like this:

<abbr class="form-required" title="Questo campo è obbligatorio.">*</abbr>

...in Greek it will already be:

<abbr class="form-required" title="Αυτό το πεδίο είναι υποχρεωτικό.">*</abbr>

...etc.

I don't see how passing all that markup again through get_t() is useful. Are you saying that it is just in order to allow the asterisk to be placed before the title? That just feels wrong to me.

... Translators could not replace !text with Titolo: !title, for example, which is instead the correct Italian translation for Title: !title.

But we are not translating "Title: !title" here - we are only rendering the field label (so just the !title). Is this to also allow adding a prefix to the field label by using t()/get_t()? If that is the case then, that again just feels wrong to me.

[avpaderno]

!required can be an asterisk, but translators should be able to place the asterisk before or after the title, depending on what is done in their language. Actually, in right-to-left languages, with !required !title, the asterisk will be after the title.

[avpaderno]

But we are not translating "Title: !title" here - we are only rendering the field label (so just the !title). Is this to also allow adding a prefix to the field label by using t()/get_t()? If that is the case then, that again just feels wrong to me.

No, with t('!title !required') we are asking the translators to provide the correct string to use for their language (which essentially is what translating does).

Translators are already asked to translate [!site-name] !subject. Following the same reasoning, t() should not be used with '[!site-name] !subject' because the site name and the subject are just rendered.

[klonos]

... but translators should be able to place the asterisk before or after the title, depending on what is done in their language.

If that is the case, then:

• There's other places where we should be doing that (allowing to reorder chunks of text/markup per-language)
• There should be an inline comment there explaining that this is what we are doing. It certainly has tripped me.
• We should add some 'context' in order to help translators, since my understanding is that they will see the exact string '!title !required' and not know what needs to be translated (or even worse, try to translate these words). The context should be something along the lines of 'Field label and required asterisk. These replacements are not meant to be translated - only reordered if the asterisk needs to come before the label in your language.'

[klonos]

Translators are already asked to translate [!site-name] !subject. Following the same reasoning, t() should not be used with '[!site-name] !subject' because the site name and the subject are just rendered.

That specific string has punctuation characters (the square brackets), which can differ or have entirely other meaning in other languages (see https://en.wikipedia.org/wiki/Bracket#Square_brackets and https://en.wiktionary.org/wiki/%E3%80%90_%E3%80%91). I think that that's the reason that we are using t() there. One might say that it is the same with the asterisk (see https://en.wikipedia.org/wiki/Asterisk and https://en.wiktionary.org/wiki/asterisk), however, if that was the case, then shouldn't the translation for that be happening via the get_t() in theme_form_required_marker() instead?

[avpaderno]

There's other places where we should be doing that (allowing to reorder chunks of text/markup per-language)

That is correct. In fact, I have noticed places where strings used in the user interface are not translated (where the documentation does not say t() should not be used). For some of them, I already created issues (and PRs).

There should be an inline comment there explaining that this is what we are doing. It certainly has tripped me.

I am not sure about this. It is how translation works with Drupal and Backdrop.

We should add some 'context' in order to help translators

If by context you mean the 'context' array key used in the array passed as third parameter to t(), that is possible, but Backdrop core actually does not have a context for that case.

[avpaderno]

That specific string has punctuation characters

A space is sufficient to make the difference, considering that some languages put one or more spaces where English would not use any space.

As long as the string to show in the user interface does not contain a single placeholder nor is it empty, it should be passed to t(), except in those cases the documentation already says t() should not be used (for example, for the 'title' key used in hook_menu(), or the first parameter passed to watchdog()).

[klonos]

If by context you mean the 'context' array key used in the array passed as third parameter to t(), that is possible, ...

Yes, that is what I mean 👍🏼

... but Backdrop core actually does not have a context for that case.

Sorry, I don't understand this. Can you please elaborate?

[avpaderno]

... but Backdrop core actually does not have a context for that case.

Contexts are useful only when they are consistently used.
What I mean is that, if a context is introduced, that context must then be used in all places where it should be used. There should not be too-similar, context which would make deciding which one to use. (It means some planning is required before introducing a new context.)

[klonos]

Well, contexts are meant to be used in order to help translators when things are ambiguous or similar words may have different meanings. In the case of !title !required, as a new translator that doesn't know the rules (do not translate placeholders), I'd be translating these. If the rule was pointed out to me, then I'd wonder why that string is presented to me if there is nothing to translate. So in that case, having some explanation that says "if your language requires changing the order of these, do it - if not, then ignore this string" would help me and save me some time.

[avpaderno]

So in that case, having some explanation that says "if your language requires changing the order of these, do it - if not, then ignore this string" would help me and save me some time.

That is what translators can do for every string they are asked to translate. A context for that reason should be applied to every string with at least two placeholders, which means it would be a useless context.

Instead, what translators can do needs to be documented on localized.backdropcms.org, if it is not already documented. Apart from not changing the placeholders (which means not changing %placeholder to !placeholder nor %segnaposto), there is nothing else translators cannot do.