Problem/Motivation

A lot of new features were recently added, which is great!

However, new texts were added with the HTML tag <br/>, which causes a challenge with the translation system. I get five errors when I install the module with Drush:

 [error]  Import of string "Hvis aktiveret, vil Toolbar Search feltet få fokus, når brugeren taster "Alt + a".<br/>Slå den fra, hvis der er en konflikt med en eksisterende genvej." was skipped because of disallowed or malformed HTML. 
 [error]  Import of string "Admin Toolbar-modulet giver en bedre brugeroplevelse med Drupals Toolbar.<br/>Det er en rullemenu, der giver hurtigere adgang til alle administrationssider på en mere effektiv måde, med færre klik og mindre rulning.<br/><br/>Følgende indstillinger giver for det meste avanceret konfiguration af værktøjslinjens JavaScript-adfærd: Sticky og hoverIntent." was skipped because of disallowed or malformed HTML. 
 [error]  Import of string "Skjul eller vis Toolbar, når brugeren taster "Alt + p".<br/>Deaktivér hvis der er en konflikt med en eksisterende genvej." was skipped because of disallowed or malformed HTML. 
 [error]  Import of string "Giver en mere behagelig brugeroplevelse, hvor kun menu links som der pauses over udvides, for derved at undgå utilsigtet udfoldning.<br/>Slå @hoverintent_source_link fra, hvis du i stedet ønsker at bruge modulets standard JavaScript." was skipped because of disallowed or malformed HTML. 
 [error]  Import of string "Sætter hoverIntent timeout aktivering (trin på 250 ms).<br/>Jo højere værdien er, desto længere tid vil menuen forblive vist, efter musen har forladt den (standard: 500ms)." was skipped because of disallowed or malformed HTML.

Steps to reproduce

Install the module in another language than English, with translations such as Danish, and see errors.

Proposed resolution

Replace <br/> with either <br> or <br />. Both are used in Drupal core:

  • $ grep '<br>' web/sites/default/files/translations/drupal-10.4.7.da.po | wc -l
6
  • $ grep '<br />' web/sites/default/files/translations/drupal-10.4.7.da.po | wc -l
9

Remaining tasks

  1. Replace <br/> in translatable strings.
  2. Update translations.

User interface changes

API changes

Data model changes

Issue fork admin_toolbar-3527462

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

ressa created an issue. See original summary.

ressa’s picture

ressa
he/him
commented
Issue summary: View changes

ressa opened merge request !155

ressa’s picture

ressa
he/him
commented
Issue summary: View changes

dydave made their first commit to this issue’s fork.

dydave’s picture

dydave commented
Status: Active » Needs review

Thanks a lot @ressa, once again for the all the help testing and reporting in issues! 🙏

Sorry for the late reply on this, but I've been handling other issues in the queue as well 😅

Thanks a lot for catching this issue, documenting it so well and for providing the solution in the merge request.
I have done a search in the code base and could not find any other occurrences for <br/> after applying the patch.

I tested the display of the all the forms and the admin_toolbar_links_access_filter error message and they all displayed fine as expected (no change visually).

I haven't tested the string translations, but if we follow the Core standards, as you suggested, there shouldn't be any issues 🤞

I have added a quick commit with very minor text edits:
https://git.drupalcode.org/project/admin_toolbar/-/merge_requests/155/di...
Mostly to roll-in the changes from:
#3526123-4: <thead> element and CKEditor toolbar hang, when Toolbar is hidden and #3304810-72: Toggle away Admin Toolbar completely

Would you see any more textual elements that should be adjusted/improved, before releasing a 3.6.1 patch version?

Otherwise, whenever we could have your confirmation on the latest text changes, the MR should be good to be merged 👍

Over to you: Needs review!

Thanks very much in advance! 🙂

ressa’s picture

ressa
he/him
commented
Assigned: ressa » Unassigned
Status: Needs review » Reviewed & tested by the community

You're welcome @dydave, thanks for taking a look at it. And definitely no worries, I have seen all the activity in the other issues! ... and we all have a limited number of hours, for volunteering our free time to Open Source.

Great extra additions to the MR, and good call to fix those details mentioned in other issues -- it now says "Toolbar sticky" everywhere 🙂 I also think using <br> should be safe, but the final test will be releasing it, and see if the translation systems accepts it, though I can't see why it shouldn't ...

I don't see any more textual elements that needs adjustment or to be improved on, before releasing a 3.6.1 patch version, it looks great now!

  • dydave committed b244ce07 on 3.x authored by ressa 
    Issue #3527462 by ressa, dydave: Fixed import of module's translated...
dydave’s picture

dydave commented
Status: Reviewed & tested by the community » Fixed

Super nice! As always!

Thanks a lot @ressa! 🙏

Following your confirmation, I went ahead and merged the changes above at #8 🥳

That's one more issue to be release in the upcoming 3.6.1 patch version 👍

Let's keep working on the other few issues so we could create a new stable release.
Thanks again very much @ressa for the great help! 🙂

ressa’s picture

ressa
he/him
commented

After updating the Danish translations to use <br> a while ago, I finally got around to trying to get the latest translations, and I am happy to report back that Admin Toolbar is now 100% translated into Danish, https://localize.drupal.org/translate/projects/admin_toolbar 🚀

I caught a single lingering untranslated string in the interface, which is "milliseconds" here:

$ grep -rinoE '.{0,10}millis.{0,10}' .
./src/Form/AdminToolbarSettingsForm.php:136:ffix' => 'milliseconds',

It just needs to get wrapped in t(), and all is well: 

'#field_suffix' => $this->t('milliseconds'),
dydave’s picture

dydave commented

Super good catch @ressa! As usual! 🙏

Indeed, it appears we missed this string 😅
milliseconds
https://git.drupalcode.org/project/admin_toolbar/-/blob/3.6.1/src/Form/A...

Let's try to add this change to the next merge request that could get merged 👌

Great job on the translations! 🥳
Thanks a lot for the feedback as well on the localization work, I hope this could serve as documentation for others to find their way through the localization system on Drupal.org.
I've followed your link myself and submitted more than 20 French translations suggestions 👍
We've got a bit of work, but looks like we're not the worst 😅

Thanks again for all the great help keeping the module well maintained @ressa!

ressa’s picture

ressa
he/him
commented

Thanks @dydave for a fast and positive answer as always! Great idea to include this in the merge request.

I recently worked with the deployment part of translations, and was finding that translating block titles and field names ("configuration") required too many manual steps, as opposed to interface translation, which automatically registers all t()-wrapped (PHP) or |trans-ending (Twig) strings, and can become part of deploy process, all contained in a single po-file.

So I checked out the options, and initially planned to use https://www.drupal.org/project/config_translation_po/. But it still wasn't great to add another slightly working step. So I instead experimented with adding |trans in the relevant Twig templates.

This turned out to work really well, and 99% percent of all translations are in a single .po file, one for each language. Only Site slogan, and some meta tag settings are still in language specific configuration files.

I updated the documentation pages in case you are interested:

Great that the French version got some more translations! Translating can take time, but https://www.deepl.com/ works pretty well (Tip: Sometimes it DeepL gets confused, but adding line breaks before and after tag signs (< and >), and then restore line breaks before inserting in the localize.drupal.org field sometimes helps) so I also made these two 100% translated in Danish :)

https://localize.drupal.org/translate/projects/token
https://localize.drupal.org/translate/projects/pathauto

... and I see French is also doing well there, kudos!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.