The word "Second" needs translation context

Can you give an example so we understand why this is a bug?

Let's see if we can get a novice contributor to work on this. Adding information to the issue summary.

Oh, for translation! The original report did not specify.

Adding more information to the issue summary.

Also a question: is it possible to add string context to YML files? For instance, I found the label 'Second' in
core/modules/layout_discovery/layout_discovery.layouts.yml
core/modules/layout_builder/layout_builder.layouts.yml (several instances)
and it would need a context in both of those files.

As for contexts in translations, I tried to revive an older issue and wrote a bit about that here:
https://www.drupal.org/project/drupal/issues/1035716
Unfortunately there were no responses, so I dropped it.
There is already a context:timeperiod here:
https://localize.drupal.org/translate/languages/de/translate?project&sta...
That is for the specific module Timeperiod, but I wonder if we couldn't use that as well to save an additional context and prevent bloating the list of contexts further. We also have some "date" contexts already.
Otherwise we need to figure out whether a broad context like "time" is fine for "second" or whether we would need something more specific. Additionally, there is the question whether to apply the context to all units of time (like "hour" and "day") or just strings that are obviously ambiguous.
As for the layouts, in my opinion those are fine. Because they clearly use "second" not as the unit of time, but as a number. Though clarifying that, might still be useful for some.

To answer one of your questions: Context should only be added to words or phrases that might be ambiguous to translate. So there is no reason to add it to "hour" and "day".

As for the layouts, the problem is that the word "Second" will be run through the t() system from those layout labels. We need the context on all uses of the word "Second" to disambiguate each one as to whether it's the time unit or the numeric order word. Otherwise, the layout region might end up being called "The time unit of 1/60th of the minute" instead of "The second part of the layout". Which could be very weird in many languages. As if in English the regions of those layouts were called something like "First", "Hour", "Third". ?!?

We just added context to workflow transition/state labels, so that's definitely possible, see #3037906: Add translation contexts to workflow schema.

> To answer one of your questions: Context should only be added to words or phrases that might be ambiguous to translate. So there is no reason to add it to "hour" and "day".

That's the theory, but it's not so easy to know whether or not a word might require different translations, developers are often not aware, how could they. Typically this is a problem with single-word translations, so it might actually not be a stupid idea to recommend adding a context to them. But we would need to have some rules/documentation on how to structure those contexts, and it would IMHO also make sense to have a word with context fall back to the same word without context if there's no translation, discussed that a while ago with Gabor. As maybe only one language out of 10 (or 50?) will need to translate them differently..

Correction of typos + adding clarificaion

Amending own typo :-}

Good clarification! It sounds like bare words of "First" and "Third" etc., if they exist in Core, also then need context -- and not just for German -- many non-English languages have gender. I imagine that those layout files have those in them, since they seem to have the word "Second" as a region name...

One clarification... To add context to a YAML config file, it gets added to the corresponding schema. Example from #3037906: Add translation contexts to workflow schema:

# PHP Date format string that is translatable.
date_format:
 type: string
 label: 'Date format'
 translatable: true
 translation context: 'PHP date format'

So, if there is a string that needs translation context in a file that is in config/optional or config/install, you have to find the definition of the data schema in config/schema (which could be in this module or another project, or even in the core/config directory!), then add the translation context there.

But in this case, it isn't config, it is a layout YML file. So the only way to add translation context there is to find the definition of the translatable keys that is in the POTX project and add it there. Here is the file:
https://git.drupalcode.org/project/potx/blob/7.x-3.x/yaml_translation_pa...

It doesn't look like the layout files are even in that file though... so as far as I can tell, layout files do not generate translatable text for the POTX module, and their strings will not get onto localize.drupal.org currently.

Anyway... leaving this comment as it may be relevant for some of the child issues.

I also created #3048032: Layout labels are not extracted for translation from layout.yml files to deal with the layout files, since that seems to be a sort of different/bigger issue.

What context are we going with here?

My suggestion is "ordinal", as in: first, second, third, etc. And "calendar" or "datetime", as in: "hours, minutes, second".

Tagging for Amsterdam 2019

Blocks #3047243: Alpha & Oscar ordinals need translation context

This issue has been sitting for 8 months while we try to decide on a good context.

The latest suggestion was #21... which seems good, except that for ordinal numbers "first" second" etc. we actually also need to know what they are applying to, because in many languages these adjectives also need gender.

So instead of a context that just says "ordinal", we actually need a specific context for each one. So for instance if it is "second" referring to the second footer region, then the context probably needs to be "ordinal for footer region" or something like that. The point is, we can have multiple words 'second' that are ordinal numbers, and depending on where they are being used, they might need different translations in different languages.

I think "datetime" for "second" when it is a unit of time is probably fine.

Also updating the issue summary about the other ordinal numbers.

Adding some notes to the issue summary about the Novice task.

additional note.

find . -type f -not -path "*/vendor/*" -not -path "*/node_modules/*" -not -path "*/tests/*" -exec grep -li ": second" {} \;
25 matches in .yml files, with 22 false positives.

find . -type f -not -path "*/vendor/*" -not -path "*/node_modules/*" -not -path "*/tests/*" -exec grep -li "['\"]second['\"]" {} \;
13 matches in .php files, with 11 false positives.

Patch attached :)

Adding additional context to regions labeled first, third, fourth, fifth as they need to be gendered in german.

patch #35 applied successfully. adding sample screenshot with this. Changing status to needs work so that any missing regions will be covered in upcoming work.

Sorry, wrong patch.

This issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 200, following Review a patch or merge require as a guide.

Interesting bug!

Looks like the solution is good to go but this will need tests please.

I am not sure what is supposed to be tested here, so remove tag. I am sure it will be added back if needed.

Converted to an MR so setting for review.

That's fair, I think the test tag was from our initial triage of 2900 issues to today. Think it's fine to not include tests.

Did search for t('Second') if there were other instances and appears to only be in tests, which don't need it.

LGTM

I wonder if in a followup we could add something to core, Coder or PHPStan that detects known words that must have a context, and add at least "Second" to this list? (and perhaps all the ordinals, given the gender issue described here)

Committed and pushed c8e47315faf to main and b0d3136822f to 11.x. Thanks!

Not eligible for further backport as this will need re-translating in all languages now.

We have an existing issue that also mentioned a phpcs check: #3108111: Require context for one word translatables, as mentioned there, I think we first need to change our context behavior so we can add contexts without breaking existing translations.