Translation of toolbar button tooltips not working when text part language plugin is enabled

not only the tooltips. also the microcopy of the button labels isn't localized - see heading and language. same for the ie11 warning message on top and the title of the source editing vertical tab. i've tested with 9.3.0-beta2

ok things might be a bit more tricky. i've additionally added another language (french) and the parts not translated differ to german - there is no consistent pattern of parts not translated across languages. not sure if it is due to a differing degree of translated strings or another reason? :/

the title of vertical headings tab isnt translated nor its content, same for source editing. but in contrast the language tab and its content is translated completely.

@rkoller These are all Drupal-owned UI strings, because you're in the CKEditor 5 toolbar configuration UI, which is Drupal-owned. If the translation for a string is not available in Drupal, then it won't be translated.
Still, we should still add test coverage for the admin UI too, to prove that it works if there are translated strings available.

Is the actual CKEditor 5 UX translated? Thanks to @Joachim Namyslo's YouTube video, I can see that this is broken! Even despite the test coverage in \Drupal\Tests\ckeditor5\FunctionalJavascript\LanguageTest::test() that tests this explicitly, this definitely does not work for you!

@Joachim Namyslo: what langcode is your browser sending? Maybe CKEditor 5 is not interpreting yours correctly. That'd be required for us to reproduce this.

Let's test German explicitly to see if something is up with that…

I wonder … if https://github.com/ckeditor/ckeditor5/issues/8510 actually fixed this. It seems very closely related.

When we update to https://github.com/ckeditor/ckeditor5/releases/tag/v31.1.0, we'll find out 🤓

(I'll only get to this in January — but you could already create a merge request or patch to get us on that version, and that'd allow you to test this ahead of time 🤞:D)

Per #8.

#3258250: Update to CKEditor5 v31.1.0 landed!

@Joachim Namyslo: could you please test this again with the latest development snapshot of Drupal core 9.3, 9.4 or 10.0 (any of those is fine!) 🙏

Thanks for checking! 🙏 We'll investigate.

My initial hunch: this only happens when changing the default language, i.e. when there is not a translation requested through language negotiation. Nope, that can't be it, because \Drupal\Tests\ckeditor5\FunctionalJavascript\LanguageTest::test() is testing exactly that! 😬

P.S.: next time just test on a single branch — they're all kept in sync functionality-wise 🤓
P.P.S.: the console log entries are for CKEditor 4, not for CKEditor 5, so should not explain this. But who knows 🤪

I was able to reproduce this when text part language plugin is enabled. Based on #11, it seems like text part language was also enabled on the environment where the issue was reported from.

The root cause of the problem is that the text part language and UI translation both use the language key for their configuration. This is not something we take into account in Drupal.editors.ckeditor5.attach(), meaning that the configuration coming from text part language is overriding the previously set language configuration which is configuring the language CKEditor should be using.

Wow, thanks for the digging, @lauriii!

+++ b/core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5LanguagePluginTest.php
@@ -0,0 +1,72 @@
+class CKEditor5LanguagePluginTest extends CKEditor5TestBase {

Why not add this to \Drupal\Tests\ckeditor5\FunctionalJavascript\LanguageTest?

Mainly I didn't want to find how to add a second test method when there is a data provider and I wanted to test only the conflict, not the translation feature at the same time.

Mainly I didn't want to find how to add a second test method when there is a data provider

It's just @dataProvider METHODNAME :)

and I wanted to test only the conflict, not the translation feature at the same time.

The test methods run separately, so there is no risk of that. Or if you meant you didn't want the tests to run in parallel … well, sadly, FunctionalJavascript tests do not support that all, so no risk there :P

…

Upon further inspection …

… the test method is identical to what we already have in \Drupal\Tests\ckeditor5\FunctionalJavascript\LanguageTest::test(), only the second parameter has a different name! So … then we should just add a new case to the existing data provider? Or am I missing something? 🤔

not identical, check the button that is clicked. It's textPartLanguage, not blockQuote :), will reroll.

extended the test data a bit, better, thanks :)

#22 Gah missed that — makes sense!

1. +++ b/core/modules/ckeditor5/tests/src/FunctionalJavascript/LanguageTest.php
   @@ -29,18 +29,20 @@ class LanguageTest extends CKEditor5TestBase {
   -    $this->assertNotEmpty($assert_session->waitForElement('css', '.ckeditor5-toolbar-item-blockQuote'));
   ...
   +    $this->assertNotEmpty($assert_session->waitForElement('css', ".ckeditor5-toolbar-item-$plugin"));
   

   Let's rename $plugin to $toolbar_item_name — because a single plugin can provide multiple toolbar items, and these are definitely toolbar items.

2. If you fix that nitpick in point 1, and also upload a failing test-only patch variation of #23, I'll instantly RTBC this :)

Here you go

RTBC assuming one red and one green patch in #25 — sadly no tests have run in the past 2 hours due to 10.0.x HEAD currently not passing

Committed 414c7a4 and pushed to 10.0.x. Also committed patch from #29 to 9.4.x and cherry-picked to 9.3.x. Thanks!