"Add or select media" not translated

Created a patch for translating the title of media library dialog after enabling it for text format. Attached the screenshot as well. Please review.

Tested patch in #3, the translation works now as intended.

I removed the title attribute for said dialog from ckeditor5.ckeditor5.yml as I believe that value is now not used at all, I also simplified the patch a bit.

Thank you for reporting the issue.

Next step will be to get a test assertion in place to check that it's properly translated.

I'm not too familiar with testing but should this test regarding if the translation works be done in the core media_library module instead?

In the core media_library module there is tests regarding if the library translation works as intended, the current translation test has call to waitForText('Add or select media') in both dutch and spanish, by updating them to the correct translated strings the test should fail if the strings are not translated.

Attached a patch regarding proposed change but this should perhaps be moved to a separate issue regarding the media_library module, not ckeditor5.

@ameymudras that's the test-only patch FYI that you're carrying forward.

@miikamakarainen if that's where you want to place them you should use the t() function. So we don't have to pull cspell ignroes in.

@smustgrave if I have understood things correct the test in media_library will fail when the dialog is patched so it needs updating either way as waitForText('Add or select media') will not be in english when the string is translated.

Attached an updated patch which checks if the ui dialog is translated correctly and waits for the element instead of the dialog title text.

Correct but when you use the t('Add or select media') it will check for the translated string. Or else we have to cspell ignore those specific lines as they are failing the custom commands.

Correct but when you use the t('Add or select media') it will check for the translated string. Or else we have to cspell ignore those specific lines as they are failing the custom commands.

I disagree with using t in the tests themselves. See https://www.drupal.org/project/drupal/issues/2875148 → for background info

I have uploaded the patch
Fixed CCF

But in this scenario we are testing translations

Hiding 14 as it removes the fix to the problem.

I was going to say "but CKEditor 5 plugin labels are translatable and they're also defined in that YAML file, so why don't these work?"

The answer: \Drupal\ckeditor5\Annotation\DrupalAspectsOfCKEditor5Plugin::$label has:

  /**
   * The human-readable name of the CKEditor plugin.
   *
   * @var \Drupal\Core\Annotation\Translation
   * @ingroup plugin_translatable
   */
  public $label;

Such annotations are not possible for each plugin's custom metadata…

So +1 for this.

#13: I think @smustgrave is right in #15 — see \Drupal\Tests\ckeditor5\FunctionalJavascript\LanguageTest::test() for example 😅

1. +++ b/core/modules/media_library/tests/src/FunctionalJavascript/TranslationsTest.php
   @@ -151,7 +151,9 @@ public function testMediaLibraryTranslations() {
   +    $this->assertSame('Media toevoegen of selecteren', $header_text);
   

   Just add

   // cSpell:disable-next-line
   

   above this line.

2. +++ b/core/modules/media_library/tests/src/FunctionalJavascript/TranslationsTest.php
   @@ -164,7 +166,9 @@ public function testMediaLibraryTranslations() {
   +    $this->assertSame('Añadir o seleccionar medio', $header_text);
   

   Same here.

… then this will pass tests and I'll RTBC 😊

make changes a per #16

The last submitted patch, 17: 3347212-17.patch, failed testing. View results →

Attempted to fix error patch #17

Updated the patch from #11 according to #16 (skipped #17 and #20 as they introduced a typo in the spellings).

I'm a bit worried that this patch wont pass the testing procedure somehow as the previous test results have failed and that is because the string has been in English in the testing environment. Attached screenshots from my local environment just to verify that the strings are correct and the translation part of the patch works.

If the tests fails with due to asserting the strings, could it be so that the testing environment does not have the dutch or spanish localization strings available? If so, then we cannot test that this string has been translated.

Adding accidentally stripped space which corrupted the patch in #21.

Re-roll of patch #22

#23 will not apply because it was relative to web/core, not core.

But you're right, even after enabling the locale module, the translation test is not working. In hindsight, I was wrong in #16 — because \Drupal\Tests\ckeditor5\FunctionalJavascript\LanguageTest::test() is testing CKEditor 5's UI translations, not Drupal's!

So per #2875148: BrowserTestBase: Steer new test development away from translation → , we AFAICT should simply not have any test coverage for this. Is that right, @borisson_?

If we are not to test any translations then the patch in #5 should be ok and it has passed testing. We do not need to update the media_library tests as the ui elements (in the testing environment) are unaffected by this change.

So per #2875148: BrowserTestBase: Steer new test development away from translation, we AFAICT should simply not have any test coverage for this. Is that right, @borisson_?

Yes, this was also discussed in slack, and catch agreed that there is no test coverage needed. So #5 should be correct patch here.

Re-uploading #5 🤓

This should address the failing custom commands.

Committed to 10.1.x. and backported to 10.0.x and 9.5.x

Thanks all

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