Editor settings cannot be translated

I'm not quite sure what Add the appropriate editor config object to the text format's config translation mapper. means.

Also, there actually are translatable things even without third-party settings:

1. Go to /admin/config/content/formats/manage/basic_html
2. Drag the Styles dropdown from Available buttons to somewhere in Active toolbar
3. Now a Styles dropdown vertical tab appears under CKEditor plugin settings. Enter something like blockquote.famous|Famous quote

Now Famous quote is a config value that you actually want to translate.

Assigning to @Gábor Hojtsy for feedback.

#2546212: Entity view/form mode formatter/widget settings have no translation UI is a similar problem. Basically translation mappers in config translation map a set of config names to a route. So it can add a translate tab and several other routes to allow looking at translation status, translating the configs and deleting translations. So I think editor configs are edited on text format config pages, so you would need to alter the mapper for text formats to also have the editor entity. Again all the things that #2546212: Entity view/form mode formatter/widget settings have no translation UI is postponed on would help as well as the implementation proposed for #2546212: Entity view/form mode formatter/widget settings have no translation UI overall.

Updating status to reflect reality. Blocked on #2577761: We need a way to dynamically alter the list of config names for config mappers, hence marking as postponed. And since #2577761 was moved to 8.1, also moving this issue to 8.1.

#2577761: We need a way to dynamically alter the list of config names for config mappers is in.

So this now just needs to do something like \Drupal\config_translation_test\EventSubscriber\ConfigTranslationTestSubscriber, or #2546212-90: Entity view/form mode formatter/widget settings have no translation UI. Let's do it!

Yay, finally possible to fix :)

@Gábor Hojtsy: Is that a response to #15 or did something else happen recently? :)

Nothing recently other than some activity in #2546212: Entity view/form mode formatter/widget settings have no translation UI that prompted me to look here as well.

In spirit of https://twitter.com/berdir/status/1113529017410387970, adding one more patch to our project :)

This is very much a proof of concept, I didn't do a lot of testing and, and as the example of the stylescombo config shows, it will most likely need more config changes.

Also, as always, integrating two modules that don't depend on each other is tricky, especially with events, add a class_exists() check as workaround for that now, we really should have module dependencies on services...

forgot the proof that it works:

Absolutely no clue, the test does that? :)

Yes. I got it only working with the langcode check removed.

The mapper's language was NULL in my case, where the standard config language is German.

Alright, this now throws an error if the config file (editor.editor.[format_name]) does not exist yet, thus has not editor:

Drupal\config_translation\Exception\ConfigMapperLanguageException: A config mapper can only contain configuration for a single language. in Drupal\config_translation\ConfigNamesMapper->getLangcode() (line 405 of core/modules/config_translation/src/ConfigNamesMapper.php)

Again in a setup where German is the default config language.

Caused by ConfigNamesMapper's:

  public function getLangcodeFromConfig($config_name) {
    // Default to English if no language code was provided in the file.
    // Although it is a best practice to include a language code, if the
    // developer did not think about a multilingual use case, we fall back
    // on assuming the file is English.
    return $this->configFactory->get($config_name)->get('langcode') ?: 'en';
  }

So we need to skip filter formats not having a WYSIWYG editor.

Here a new patch checking if the filter format has an editor set.

1. The removal of the langcode check made it work on a site with German as default language for configs.
2. The check if a editor exists made it work for filter formats without editor

Reroll after #3063020: Support translation of the list of styles in CKEditor which tried to fix this too but that's not enough.

I'm adding a basic translation test,

1. +++ b/core/modules/ckeditor/tests/src/Functional/CKEditorStylesComboTranslationTest.php
   @@ -0,0 +1,82 @@
   +  /**
   +   * Modules to enable.
   +   *
   +   * @var array
   +   */
   

   🤓 {inheritdoc}

2. +++ b/core/modules/ckeditor/tests/src/Functional/CKEditorStylesComboTranslationTest.php
   @@ -0,0 +1,82 @@
   +   * A random generated format machine name.
   

   🤓 s/random/randomly/

3. +++ b/core/modules/ckeditor/tests/src/Functional/CKEditorStylesComboTranslationTest.php
   @@ -0,0 +1,82 @@
   +   * Tests translations of ckeditor styles configuration.
   

   🤓 s/ckeditor/CKEditor/

4. +++ b/core/modules/editor/src/EventSubscriber/EditorConfigTranslationSubscriber.php
   @@ -0,0 +1,63 @@
   +use function class_exists;
   

   🤯

5. +++ b/core/modules/editor/src/EventSubscriber/EditorConfigTranslationSubscriber.php
   @@ -0,0 +1,63 @@
   +class EditorConfigTranslationSubscriber implements EventSubscriberInterface {
   +
   +
   +  /**
   

   Übernit: excess whitespace.

6. +++ b/core/modules/editor/src/EventSubscriber/EditorConfigTranslationSubscriber.php
   @@ -0,0 +1,63 @@
   +  public function __construct($config_factory) {
   

   Shoudln't this be typehinted?

7. +++ b/core/modules/editor/src/EventSubscriber/EditorConfigTranslationSubscriber.php
   @@ -0,0 +1,63 @@
   +      // Only add the editor config if it exists, otherwise we assume no editor
   +      // has been set for this filter format.
   

   Nits:
   - s/editor/text editor/
   - s/filter format/text format/

Patch added for issues mentioned in #33.

Thanks, @shubham.prakash! 👍 Could you please provide an interdiff in the future? That makes reviews much easier! 🙂

Manually compared. Looks good! Two remaining remarks:

1. #33.6 has not yet been addressed.

2. +++ b/core/modules/editor/src/EventSubscriber/EditorConfigTranslationSubscriber.php
   @@ -0,0 +1,61 @@
   +      // Only add the text editor config if it exists, otherwise we assume no editor
   +      // has been set for this text format.
   

   🤓 This line now is longer than 80 cols. The last word needs to be moved to the next line.

Thanks for the review, I would make sure to provide interdiff now.

I am not sure about the type hinting as why it gave failed test cases in #34.
The commented line has been wrapped as said.

Aha, I missed that!

You forgot the use statement, and that's why it failed the way it did. Typehinting + importing the correct interface through a use statement should work fine 🙂

Once that's done, this is ready!

Hope this resolves the issue.

It did! :)

The remaining failure is now due to the newly added requirement that you specify a $defaultTheme property on your test class.

Perfect, thank you!

Committed and pushed f68812c39a to 9.0.x and 9c054814f3 to 8.9.x. Thanks!

Asked the other committers about backporting this to 8.8.x

Discussed with @catch who +1'd the backport.