From ccce80147c2ba3c33a33c83d77020278792b4784 Mon Sep 17 00:00:00 2001 From: catch <catch@35733.no-reply.drupal.org> Date: Mon, 11 Jul 2022 11:37:07 +0900 Subject: [PATCH] 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 --- core/modules/ckeditor/ckeditor.module | 87 +++++++++++++++++++ .../modules/ckeditor/ckeditor.post_update.php | 41 +++++++++ .../ckeditor/src/Plugin/Editor/CKEditor.php | 20 ++++- .../src/Functional/CKEditorAdminTest.php | 22 +++-- .../CKEditorStylesComboAdminTest.php | 19 +++- .../CKEditorStylesComboTranslationTest.php | 7 ++ ...EditorUpdateOmitDisabledPluginSettings.php | 58 +++++++++++++ .../src/Kernel/SmartDefaultSettingsTest.php | 46 ---------- .../install/editor.editor.basic_html.yml | 6 +- .../install/editor.editor.full_html.yml | 6 +- .../install/editor.editor.basic_html.yml | 4 +- .../install/editor.editor.full_html.yml | 4 +- 12 files changed, 247 insertions(+), 73 deletions(-) create mode 100644 core/modules/ckeditor/ckeditor.post_update.php create mode 100644 core/modules/ckeditor/tests/src/Functional/Update/CKEditorUpdateOmitDisabledPluginSettings.php diff --git a/core/modules/ckeditor/ckeditor.module b/core/modules/ckeditor/ckeditor.module index 5105e1732363..babe27555e1a 100644 --- a/core/modules/ckeditor/ckeditor.module +++ b/core/modules/ckeditor/ckeditor.module @@ -10,8 +10,11 @@ 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); + } +} diff --git a/core/modules/ckeditor/ckeditor.post_update.php b/core/modules/ckeditor/ckeditor.post_update.php new file mode 100644 index 000000000000..224b5280d290 --- /dev/null +++ b/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; + }); +} diff --git a/core/modules/ckeditor/src/Plugin/Editor/CKEditor.php b/core/modules/ckeditor/src/Plugin/Editor/CKEditor.php index 9d36a8e9667c..0e43f969e499 100644 --- a/core/modules/ckeditor/src/Plugin/Editor/CKEditor.php +++ b/core/modules/ckeditor/src/Plugin/Editor/CKEditor.php @@ -166,7 +166,7 @@ public function getDefaultSettings() { ], ], ], - 'plugins' => ['language' => ['language_list' => 'un']], + 'plugins' => [], ]; } @@ -287,6 +287,24 @@ public function submitConfigurationForm(array &$form, FormStateInterface $form_s 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); } /** diff --git a/core/modules/ckeditor/tests/src/Functional/CKEditorAdminTest.php b/core/modules/ckeditor/tests/src/Functional/CKEditorAdminTest.php index 686d7da6d6b4..ded1d54c79f1 100644 --- a/core/modules/ckeditor/tests/src/Functional/CKEditorAdminTest.php +++ b/core/modules/ckeditor/tests/src/Functional/CKEditorAdminTest.php @@ -108,7 +108,7 @@ public function testExistingFormat() { ], ], ], - 'plugins' => ['language' => ['language_list' => 'un']], + 'plugins' => [], ]; $this->assertEquals($expected_default_settings, $ckeditor->getDefaultSettings()); @@ -136,22 +136,27 @@ public function testExistingFormat() { $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 @@ public function testExistingFormat() { // 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 @@ public function testExistingFormat() { // 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 @@ public function testNewFormat() { // 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.'); diff --git a/core/modules/ckeditor/tests/src/Functional/CKEditorStylesComboAdminTest.php b/core/modules/ckeditor/tests/src/Functional/CKEditorStylesComboAdminTest.php index 44444d089de4..10ef5c316392 100644 --- a/core/modules/ckeditor/tests/src/Functional/CKEditorStylesComboAdminTest.php +++ b/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 @@ protected function setUp(): void { '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 @@ protected function setUp(): void { * 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.'); diff --git a/core/modules/ckeditor/tests/src/Functional/CKEditorStylesComboTranslationTest.php b/core/modules/ckeditor/tests/src/Functional/CKEditorStylesComboTranslationTest.php index b79d0fe81b30..0e4cba82e1b2 100644 --- a/core/modules/ckeditor/tests/src/Functional/CKEditorStylesComboTranslationTest.php +++ b/core/modules/ckeditor/tests/src/Functional/CKEditorStylesComboTranslationTest.php @@ -51,9 +51,16 @@ protected function setUp(): void { '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(); diff --git a/core/modules/ckeditor/tests/src/Functional/Update/CKEditorUpdateOmitDisabledPluginSettings.php b/core/modules/ckeditor/tests/src/Functional/Update/CKEditorUpdateOmitDisabledPluginSettings.php new file mode 100644 index 000000000000..7ef124828282 --- /dev/null +++ b/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']); + } + +} diff --git a/core/modules/ckeditor5/tests/src/Kernel/SmartDefaultSettingsTest.php b/core/modules/ckeditor5/tests/src/Kernel/SmartDefaultSettingsTest.php index 408ff20c9e65..f7aeaf253d7c 100644 --- a/core/modules/ckeditor5/tests/src/Kernel/SmartDefaultSettingsTest.php +++ b/core/modules/ckeditor5/tests/src/Kernel/SmartDefaultSettingsTest.php @@ -297,32 +297,6 @@ protected function setUp(): void { ], ])->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 @@ public function provider() { ], ]; - 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' => [], diff --git a/core/profiles/demo_umami/config/install/editor.editor.basic_html.yml b/core/profiles/demo_umami/config/install/editor.editor.basic_html.yml index 4ae2bffdcc41..62138b8768ef 100644 --- a/core/profiles/demo_umami/config/install/editor.editor.basic_html.yml +++ b/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 diff --git a/core/profiles/demo_umami/config/install/editor.editor.full_html.yml b/core/profiles/demo_umami/config/install/editor.editor.full_html.yml index c0f60d273c44..0f40656b7c51 100644 --- a/core/profiles/demo_umami/config/install/editor.editor.full_html.yml +++ b/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 diff --git a/core/profiles/standard/config/install/editor.editor.basic_html.yml b/core/profiles/standard/config/install/editor.editor.basic_html.yml index 966cec6a19fb..cafc88e22c91 100644 --- a/core/profiles/standard/config/install/editor.editor.basic_html.yml +++ b/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 diff --git a/core/profiles/standard/config/install/editor.editor.full_html.yml b/core/profiles/standard/config/install/editor.editor.full_html.yml index f5dd7bcc170b..a423369d1f5c 100644 --- a/core/profiles/standard/config/install/editor.editor.full_html.yml +++ b/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 -- GitLab