Problem/Motivation

Drupal stores its style dropdown configuration in a flat list

# editor.editor.*.yml
settings:
  plugins:
    ckeditor5_style:
      styles:
        -
          label: 'Text small (sm)'
          element: '<p class="text-small">'
        -
          label: 'Text large (lg)'
          element: '<p class="text-large">'
        -
          label: 'Text red (red)'
          element: '<p class="text-red">'
        -
          label: 'Text blue (blue)'
          element: '<p class="text-blue">'

A translation for this would look like

# language/de/editor.editor.*.yml
settings:
  plugins:
    ckeditor5_style:
      styles:
        -
          label: 'Text klein (sm)'
        -
          label: 'Text groß (lg)'
        -
          label: 'Text rot (red)'
        -
          label: 'Text blau (blue)'

If an site admin adds a new style in between and not at the very end, the translated config list is now incorrect due different numeric indexes.

E.g. an site admin adds a new format in between

# editor.editor.*.yml
settings:
  plugins:
    ckeditor5_style:
      styles:
        -
          label: 'Text small (sm)'
          element: '<p class="text-small">'
        -
          label: 'Text medium (md)'
          element: '<p class="text-medium">'
        -
          label: 'Text large (lg)'
          element: '<p class="text-large">'
        -
          label: 'Text red (red)'
          element: '<p class="text-red">'
        -
          label: 'Text blue (blue)'
          element: '<p class="text-blue">'

and does not instantly translate/update the config, then the german UI misses the new entry and shows something like this in the CK5 styles dropdown UI

Text klein (sm) <-- creates p.text-small
Text groß (lg) <-- creates p.text-medium
Text rot (red) <-- creates p.text-large
Text blau (blue) <-- creates p.text-red
Text blue (blue) <-- creates p.text-blue, uses label from original language

This is extremly confusing and does not even trigger any error or warning. It just shows something completely wrong in the translated editor UI. (It becomes even more confusing if you are mixing block and inline level styles, as they are grouped in the UI, but not in the YAML.)

Proposed resolution

Do not use a flat list to store the styles to prevent incorrect translation due missing translated labels.

Data model changes

Yes, to be determined

Comments

hudri created an issue. See original summary.

hudri’s picture

Issue summary: View changes
hudri’s picture

Issue summary: View changes
hudri’s picture

Issue summary: View changes
wim leers’s picture

Title: CKeditor5 style must not use flat list to prevent incorrect translation config » Config Translation does not correctly handle sequences when new items are not appended, but inserted in existing positions
Component: ckeditor5.module » config_translation.module
Priority: Normal » Major
Issue tags: +Usability

Wow, very interesting problem!

Thanks for explaining it so very clearly. 🙏👏

However … this structure is necessary: the items can be ordered arbitrarily, and it's up to the site builder to order them in whichever way they see fit. Plus, there are other things in Drupal core/contrib that follow this very same pattern.

This seems to be a bug in the Config Translation UI 🤔

wim leers’s picture

gábor hojtsy’s picture

Title: Config Translation does not correctly handle sequences when new items are not appended, but inserted in existing positions » CKEditor 5 styles config storage is not compatible with config ovverides

Its not a bug in the config translation user interface. I would argue that this kind of data structure is not well suited for config overrides in general (the system that the config translation system is based on). Config overrides are essentially a deep array merge. The config override system has no way to tell if you just changed the data under a sequence item vs. swapped items. So it does not know if you intentionally changed that label while keeping the meaning of it the same, or you moved around items.

Numbered sequences have no other way to identify them but their number indexes, so the config override system (translation, domain overrides, environment overrides, etc). will use that numeric index to apply the override. I don't think there is an easy to way to update all of the possible overrides after you swapped values. I believe the config override system is pluggable in that you may not know exactly what overrides may be available for a key. But that is from memory, so please check if you want to go this way :)

I think the likely solution indeed is to store these under named keys, so that when the items move, the named keys are different and therefore the array deep merge will be based on an associative array rather than an indexed array and fall into the right place. It can still be a sequence. The current solution is incompatible with all types of config overrides if the order of the items are designed to change. So that would make it compatible with all of them.

Retitled for this direction but feel free to update if another direction is deemed better.

gábor hojtsy’s picture

Component: config_translation.module » ckeditor5.module
wim leers’s picture

I think the likely solution indeed is to store these under named keys, so that when the items move, the named keys are different and therefore the array deep merge will be based on an associative array rather than an indexed array and fall into the right place.

AFAICT this means that Config Translation's UI does not support anything that uses type: sequence + orderby: ~ that #2361539: Config export key order is not predictable for sequences, add orderby property to config schema + #2852557: Config export key order is not predictable, use config schema to order keys for maps added … which is also the default.

IOW: AFAICT this means Config Translation's UI only works for orderby: key and orderby: value? 😱

It may very well be impossible to support, but reality is that this implies a lot of config is not actually translatable 😅

gábor hojtsy’s picture

The config translation UI is not implicated or involved at all when you save a change to a config value (or to a config translation or other kind of override). I don't think a solution would reside with config translation's user interface. Here the problem is the base config structure is updated and the config translation UI is nowhere to be seen then :)

The config translation system has no problems with translating sequences of any kind, but when the base data of the sequence changes, the config translation (and generally all of the override) system has no information that "item 4 swapped with 5" as opposed to "both item 4 and 5 changed some data". It is impossible to tell the difference between the two on the level of the config save/override. The application level knows the difference but the config level does not have that understanding of the nature of the data. If the application level wants to do this, then it would need to manually update all of the overrides (not just translations, there could be environment based, group based, any kind of overrides) when it saves the base config, since the application layer has this understanding. The config system does not.

wim leers’s picture

Thanks for the extra context!

AFAICT that means though that there should never be something translatable inside a sequence that does not have orderby: key?

I'm more familiar with config schemas than most, yet I had no idea this was going to be a consequence! 😳

If the answer to my question is "yes", we should add validation for that to the configuration system, so that any such schema definition throws an exception in Drupal 11 and a deprecation in Drupal 10. Tagging Needs followup for that and assigning to you, @Gábor Hojtsy.


Either way, it's clear that in order to fix the problem that @hudri reported, we'll need to change the config schema + write an update hook + update the CKEditor 5 upgrade path logic + update path tests. 😅 Tagging Needs tests, Needs update path, Needs update path tests for that.

gábor hojtsy’s picture

Status: Postponed (maintainer needs more info) » Needs work

I don't know what exactly would orderby: key mean, if it means items should not be moved around abitrarily then yes that would help. Also note that if one of the middle items are removed, you may still face the same problem that the translation "does not know" that you removed a middle item and it would only see that the last item does not exist anymore and that would be removed from the config override on save of the original config (due to non-existent key in the base config). So if you consider that case too, I think a more precise guideline is that numbered sequence items are only marked translatable if items cannot be reordered or removed in the source (both of those would result in their numbers changed and thus mismatching config overrides applied in general, not just translations).

wim leers’s picture

Title: CKEditor 5 styles config storage is not compatible with config ovverides » [Style] CKEditor 5 styles config storage is not compatible with config ovverides
Assigned: gábor hojtsy » Unassigned
wim leers’s picture

Issue tags: +Configuration schema

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.