Issue #3291744 by lauriii, xjm, Abhijith S, Wim Leers, bnjmnm, catch: Ensure Editor config entities using CKEditor 4 only store plugins settings for actually enabled plugins

(cherry picked from commit ccce80147c)

core/modules/ckeditor/ckeditor.module

@@ -10,8 +10,11 @@ use Drupal\Component\Serialization\Json;
 use Drupal\Component\Utility\UrlHelper;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Routing\RouteMatchInterface;
+use Drupal\ckeditor\CKEditorPluginButtonsInterface;
+use Drupal\ckeditor\CKEditorPluginContextualInterface;
 use Drupal\Core\StringTranslation\TranslatableMarkup;
 use Drupal\editor\Entity\Editor;
+use Drupal\editor\EditorInterface;
 
 /**
  * Implements hook_help().
@@ -122,6 +125,59 @@ function _ckeditor_theme_css($theme = NULL) {
   return $css;
 }
 
+/**
+ * Gets all enabled CKEditor 4 plugins.
+ *
+ * @param \Drupal\editor\EditorInterface $editor
+ *   A text editor config entity configured to use CKEditor 4.
+ *
+ * @return string[]
+ *   The enabled CKEditor 4 plugin IDs.
+ *
+ * @internal
+ */
+function _ckeditor_get_enabled_plugins(EditorInterface $editor): array {
+  assert($editor->getEditor() === 'ckeditor');
+
+  $cke4_plugin_manager = \Drupal::service('plugin.manager.ckeditor.plugin');
+
+  // This is largely copied from the CKEditor 4 plugin manager, because it
+  // unfortunately does not provide the API this needs.
+  // @see \Drupal\ckeditor\CKEditorPluginManager::getEnabledPluginFiles()
+  $plugins = array_keys($cke4_plugin_manager->getDefinitions());
+  $toolbar_buttons = $cke4_plugin_manager->getEnabledButtons($editor);
+  $enabled_plugins = [];
+  $additional_plugins = [];
+  foreach ($plugins as $plugin_id) {
+    $plugin = $cke4_plugin_manager->createInstance($plugin_id);
+
+    $enabled = FALSE;
+    // Plugin is enabled if it provides a button that has been enabled.
+    if ($plugin instanceof CKEditorPluginButtonsInterface) {
+      $plugin_buttons = array_keys($plugin->getButtons());
+      $enabled = (count(array_intersect($toolbar_buttons, $plugin_buttons)) > 0);
+    }
+    // Otherwise plugin is enabled if it declares itself as enabled.
+    if (!$enabled && $plugin instanceof CKEditorPluginContextualInterface) {
+      $enabled = $plugin->isEnabled($editor);
+    }
+
+    if ($enabled) {
+      $enabled_plugins[$plugin_id] = $plugin_id;
+      // Check if this plugin has dependencies that should be considered
+      // enabled.
+      $additional_plugins = array_merge($additional_plugins, array_diff($plugin->getDependencies($editor), $additional_plugins));
+    }
+  }
+
+  // Add the list of dependent plugins.
+  foreach ($additional_plugins as $plugin_id) {
+    $enabled_plugins[$plugin_id] = $plugin_id;
+  }
+
+  return $enabled_plugins;
+}
+
 /**
  * Implements hook_library_info_alter().
  */
@@ -203,3 +259,34 @@ function ckeditor_filter_format_edit_form_validate($form, FormStateInterface $fo
     }
   }
 }
+
+/**
+ * Implements hook_ENTITY_TYPE_presave().
+ */
+function ckeditor_editor_presave(EditorInterface $editor) {
+  // Only try to update editors using CKEditor 4.
+  if ($editor->getEditor() !== 'ckeditor') {
+    return FALSE;
+  }
+
+  $enabled_plugins = _ckeditor_get_enabled_plugins($editor);
+
+  // Only update if the editor has plugin settings for disabled plugins.
+  $needs_update = FALSE;
+  $settings = $editor->getSettings();
+
+  // Updates are not needed if plugin settings are not defined for the editor.
+  if (!isset($settings['plugins'])) {
+    return;
+  }
+
+  foreach (array_keys($settings['plugins']) as $plugin_id) {
+    if (!in_array($plugin_id, $enabled_plugins, TRUE)) {
+      unset($settings['plugins'][$plugin_id]);
+      $needs_update = TRUE;
+    }
+  }
+  if ($needs_update) {
+    $editor->setSettings($settings);
+  }
+}

core/modules/ckeditor/ckeditor.post_update.php

@@ -0,0 +1,41 @@
+<?php
+
+/**
+ * @file
+ * Post update functions for CKEditor.
+ */
+
+use Drupal\Core\Config\Entity\ConfigEntityUpdater;
+use Drupal\editor\Entity\Editor;
+
+/**
+ * Updates Text Editors using CKEditor 4 to omit settings for disabled plugins.
+ */
+function ckeditor_post_update_omit_settings_for_disabled_plugins(&$sandbox = []) {
+  $config_entity_updater = \Drupal::classResolver(ConfigEntityUpdater::class);
+  $config_entity_updater->update($sandbox, 'editor', function (Editor $editor): bool {
+    // Only try to update editors using CKEditor 4.
+    if ($editor->getEditor() !== 'ckeditor') {
+      return FALSE;
+    }
+
+    $enabled_plugins = _ckeditor_get_enabled_plugins($editor);
+
+    // Only update if the editor has plugin settings for disabled plugins.
+    $needs_update = FALSE;
+    $settings = $editor->getSettings();
+
+    // Updates are not needed if plugin settings are not defined for the editor.
+    if (!isset($settings['plugins'])) {
+      return FALSE;
+    }
+
+    foreach (array_keys($settings['plugins']) as $plugin_id) {
+      if (!in_array($plugin_id, $enabled_plugins, TRUE)) {
+        $needs_update = TRUE;
+      }
+    }
+
+    return $needs_update;
+  });
+}

core/modules/ckeditor/src/Plugin/Editor/CKEditor.php

@@ -174,7 +174,7 @@ class CKEditor extends EditorBase implements ContainerFactoryPluginInterface {
           ],
         ],
       ],
-      'plugins' => ['language' => ['language_list' => 'un']],
+      'plugins' => [],
     ];
   }
 
@@ -295,6 +295,24 @@ class CKEditor extends EditorBase implements ContainerFactoryPluginInterface {
     if ($form_state->hasValue('plugins')) {
       $form_state->unsetValue('plugin_settings');
     }
+
+    // Ensure plugin settings are only saved for plugins that are actually
+    // enabled.
+    $about_to_be_saved_editor = Editor::create([
+      'editor' => 'ckeditor',
+      'settings' => [
+        'toolbar' => $form_state->getValue('toolbar'),
+        'plugins' => $form_state->getValue('plugins'),
+      ],
+    ]);
+    $enabled_plugins = _ckeditor_get_enabled_plugins($about_to_be_saved_editor);
+    $plugin_settings = $form_state->getValue('plugins', []);
+    foreach (array_keys($plugin_settings) as $plugin_id) {
+      if (!in_array($plugin_id, $enabled_plugins, TRUE)) {
+        unset($plugin_settings[$plugin_id]);
+      }
+    }
+    $form_state->setValue('plugins', $plugin_settings);
   }
 
   /**

core/modules/ckeditor/tests/src/Functional/CKEditorAdminTest.php

@@ -108,7 +108,7 @@ class CKEditorAdminTest extends BrowserTestBase {
           ],
         ],
       ],
-      'plugins' => ['language' => ['language_list' => 'un']],
+      'plugins' => [],
     ];
     $this->assertEquals($expected_default_settings, $ckeditor->getDefaultSettings());
 
@@ -136,22 +136,27 @@ class CKEditorAdminTest extends BrowserTestBase {
     $expected_buttons_value = json_encode($expected_default_settings['toolbar']['rows']);
     $this->assertSession()->fieldValueEquals('editor[settings][toolbar][button_groups]', $expected_buttons_value);
 
-    // Ensure the styles textarea exists and is initialized empty.
-    $this->assertSession()->fieldValueEquals('editor[settings][plugins][stylescombo][styles]', '');
-
     // Submit the form to save the selection of CKEditor as the chosen editor.
     $this->submitForm($edit, 'Save configuration');
 
     // Ensure an Editor object exists now, with the proper settings.
     $expected_settings = $expected_default_settings;
-    $expected_settings['plugins']['stylescombo']['styles'] = '';
     $editor = Editor::load('filtered_html');
     $this->assertInstanceOf(Editor::class, $editor);
     $this->assertEquals($expected_settings, $editor->getSettings(), 'The Editor config entity has the correct settings.');
 
     // Configure the Styles plugin, and ensure the updated settings are saved.
     $this->drupalGet('admin/config/content/formats/manage/filtered_html');
+
+    // Ensure the styles textarea exists and is initialized empty.
+    $this->assertSession()->fieldValueEquals('editor[settings][plugins][stylescombo][styles]', '');
+
+    $expected_settings['toolbar']['rows'][0][] = [
+      'name' => 'Styles dropdown',
+      'items' => ['Styles'],
+    ];
     $edit = [
+      'editor[settings][toolbar][button_groups]' => json_encode($expected_settings['toolbar']['rows']),
       'editor[settings][plugins][stylescombo][styles]' => "h1.title|Title\np.callout|Callout\n\n",
     ];
     $this->submitForm($edit, 'Save configuration');
@@ -164,6 +169,7 @@ class CKEditorAdminTest extends BrowserTestBase {
     // done via drag and drop, but here we can only emulate the end result of
     // that interaction). Test multiple toolbar rows and a divider within a row.
     $this->drupalGet('admin/config/content/formats/manage/filtered_html');
+    $expected_settings = $expected_default_settings;
     $expected_settings['toolbar']['rows'][0][] = [
       'name' => 'Action history',
       'items' => ['Undo', '|', 'Redo', 'JustifyCenter'],
@@ -205,7 +211,12 @@ class CKEditorAdminTest extends BrowserTestBase {
 
     // Finally, check the "Ultra llama mode" checkbox.
     $this->drupalGet('admin/config/content/formats/manage/filtered_html');
+    $expected_settings['toolbar']['rows'][0][] = [
+      'name' => 'Ultra llama mode',
+      'items' => ['Llama'],
+    ];
     $edit = [
+      'editor[settings][toolbar][button_groups]' => json_encode($expected_settings['toolbar']['rows']),
       'editor[settings][plugins][llama_contextual_and_button][ultra_llama_mode]' => '1',
     ];
     $this->submitForm($edit, 'Save configuration');
@@ -287,7 +298,6 @@ class CKEditorAdminTest extends BrowserTestBase {
 
     // Ensure an Editor object exists now, with the proper settings.
     $expected_settings = $default_settings;
-    $expected_settings['plugins']['stylescombo']['styles'] = '';
     $editor = Editor::load('amazing_format');
     $this->assertInstanceOf(Editor::class, $editor);
     $this->assertEquals($expected_settings, $editor->getSettings(), 'The Editor config entity has the correct settings.');

core/modules/ckeditor/tests/src/Functional/CKEditorStylesComboAdminTest.php

@@ -39,6 +39,13 @@ class CKEditorStylesComboAdminTest extends BrowserTestBase {
    */
   protected $format;
 
+  /**
+   * The default editor settings.
+   *
+   * @var array
+   */
+  protected $defaultSettings;
+
   /**
    * {@inheritdoc}
    */
@@ -52,9 +59,16 @@ class CKEditorStylesComboAdminTest extends BrowserTestBase {
       'filters' => [],
     ]);
     $filter_format->save();
+    $ckeditor = $this->container->get('plugin.manager.editor')->createInstance('ckeditor');
+    $this->defaultSettings = $ckeditor->getDefaultSettings();
+    $this->defaultSettings['toolbar']['rows'][0][] = [
+      'name' => 'Styles dropdown',
+      'items' => ['Styles'],
+    ];
     $editor = Editor::create([
       'format' => $this->format,
       'editor' => 'ckeditor',
+      'settings' => $this->defaultSettings,
     ]);
     $editor->save();
 
@@ -65,14 +79,11 @@ class CKEditorStylesComboAdminTest extends BrowserTestBase {
    * Tests StylesCombo settings for an existing text format.
    */
   public function testExistingFormat() {
-    $ckeditor = $this->container->get('plugin.manager.editor')->createInstance('ckeditor');
-    $default_settings = $ckeditor->getDefaultSettings();
-
     $this->drupalLogin($this->adminUser);
     $this->drupalGet('admin/config/content/formats/manage/' . $this->format);
 
     // Ensure an Editor config entity exists, with the proper settings.
-    $expected_settings = $default_settings;
+    $expected_settings = $this->defaultSettings;
     $editor = Editor::load($this->format);
     $this->assertEquals($expected_settings, $editor->getSettings(), 'The Editor config entity has the correct settings.');
 

core/modules/ckeditor/tests/src/Functional/CKEditorStylesComboTranslationTest.php

@@ -51,9 +51,16 @@ class CKEditorStylesComboTranslationTest extends BrowserTestBase {
       'filters' => [],
     ]);
     $filter_format->save();
+    $ckeditor = $this->container->get('plugin.manager.editor')->createInstance('ckeditor');
+    $settings = $ckeditor->getDefaultSettings();
+    $settings['toolbar']['rows'][0][] = [
+      'name' => 'Styles dropdown',
+      'items' => ['Styles'],
+    ];
     $editor = Editor::create([
       'format' => $this->format,
       'editor' => 'ckeditor',
+      'settings' => $settings,
     ]);
     $editor->save();
 

core/modules/ckeditor/tests/src/Functional/Update/CKEditorUpdateOmitDisabledPluginSettings.php

@@ -0,0 +1,58 @@
+<?php
+
+namespace Drupal\Tests\ckeditor\Functional\Update;
+
+use Drupal\editor\Entity\Editor;
+use Drupal\FunctionalTests\Update\UpdatePathTestBase;
+
+/**
+ * Tests the update path for CKEditor plugin settings for disabled plugins.
+ *
+ * @group Update
+ */
+class CKEditorUpdateOmitDisabledPluginSettings extends UpdatePathTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $defaultTheme = 'stark';
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setDatabaseDumpFiles() {
+    $this->databaseDumpFiles = [
+      __DIR__ . '/../../../../../system/tests/fixtures/update/drupal-9.3.0.filled.standard.php.gz',
+    ];
+  }
+
+  /**
+   * Ensure settings for disabled CKEditor 4 plugins are omitted on post update.
+   */
+  public function testUpdateUpdateOmitDisabledSettingsPostUpdate() {
+    $editor = Editor::load('basic_html');
+    $settings = $editor->getSettings();
+    $this->assertArrayHasKey('stylescombo', $settings['plugins']);
+
+    $this->runUpdates();
+
+    $editor = Editor::load('basic_html');
+    $settings = $editor->getSettings();
+    $this->assertArrayNotHasKey('stylescombo', $settings['plugins']);
+  }
+
+  /**
+   * Ensure settings for disabled CKEditor 4 plugins are omitted on entity save.
+   */
+  public function testUpdateUpdateOmitDisabledSettingsEntitySave() {
+    $editor = Editor::load('basic_html');
+    $settings = $editor->getSettings();
+    $this->assertArrayHasKey('stylescombo', $settings['plugins']);
+    $editor->save();
+
+    $editor = Editor::load('basic_html');
+    $settings = $editor->getSettings();
+    $this->assertArrayNotHasKey('stylescombo', $settings['plugins']);
+  }
+
+}

core/modules/ckeditor5/tests/src/Kernel/SmartDefaultSettingsTest.php

@@ -297,32 +297,6 @@ class SmartDefaultSettingsTest extends KernelTestBase {
       ],
     ])->save();
 
-    FilterFormat::create([
-      'format' => 'cke4_plugins_with_settings_for_disabled_plugins',
-      'name' => 'All CKEditor 4 core plugins with settings for disabled plugins',
-    ])->save();
-    Editor::create([
-      'format' => 'cke4_plugins_with_settings_for_disabled_plugins',
-      'editor' => 'ckeditor',
-      'settings' => [
-        // Empty toolbar.
-        'toolbar' => [
-          'rows' => [
-            0 => [
-              [
-                'name' => 'Only buttons without settings',
-                'items' => [
-                  'Bold',
-                ],
-              ],
-            ],
-          ],
-        ],
-        // Same plugin settings as `cke4_plugins_with_settings`.
-        'plugins' => Editor::load('cke4_plugins_with_settings')->getSettings()['plugins'],
-      ],
-    ])->save();
-
     FilterFormat::create([
       'format' => 'cke4_contrib_plugins_now_in_core',
       'name' => 'All CKEditor 4 contrib plugins now in core',
@@ -1097,26 +1071,6 @@ class SmartDefaultSettingsTest extends KernelTestBase {
       ],
     ];
 
-    yield "cke4_plugins_with_settings_for_disabled_plugins can be switched to CKEditor 5 without problems; irrelevant settings are dropped" => [
-      'format_id' => 'cke4_plugins_with_settings_for_disabled_plugins',
-      'filters_to_drop' => [],
-      'expected_ckeditor5_settings' => [
-        'toolbar' => [
-          'items' => [
-            'bold',
-          ],
-        ],
-        'plugins' => [],
-      ],
-      'expected_superset' => '',
-      'expected_fundamental_compatibility_violations' => [],
-      'expected_messages' => [
-        'warning' => [
-          'The <em class="placeholder">llama_contextual_and_button</em> plugin settings do not have a known upgrade path.',
-        ],
-      ],
-    ];
-
     yield "cke4_contrib_plugins_now_in_core can be switched to CKEditor 5 without problems" => [
       'format_id' => 'cke4_contrib_plugins_now_in_core',
       'filters_to_drop' => [],

core/profiles/demo_umami/config/install/editor.editor.basic_html.yml

@@ -39,11 +39,7 @@ settings:
           name: Tools
           items:
             - Source
-  plugins:
-    language:
-      language_list: un
-    stylescombo:
-      styles: ''
+  plugins: {}
 image_upload:
   status: false
   scheme: public

core/profiles/demo_umami/config/install/editor.editor.full_html.yml

@@ -47,11 +47,7 @@ settings:
           items:
             - ShowBlocks
             - Source
-  plugins:
-    language:
-      language_list: un
-    stylescombo:
-      styles: ''
+  plugins: {}
 image_upload:
   status: true
   scheme: public

core/profiles/standard/config/install/editor.editor.basic_html.yml

@@ -39,9 +39,7 @@ settings:
           name: Tools
           items:
             - Source
-  plugins:
-    stylescombo:
-      styles: ''
+  plugins: {}
 image_upload:
   status: true
   scheme: public

core/profiles/standard/config/install/editor.editor.full_html.yml

@@ -47,9 +47,7 @@ settings:
           items:
             - ShowBlocks
             - Source
-  plugins:
-    stylescombo:
-      styles: ''
+  plugins: {}
 image_upload:
   status: true
   scheme: public