Drupal
/
drupal
mirror of https://git.drupalcode.org/project/drupal.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Issue #3268174 by Wim Leers, nod_, catch, lauriii: Bug in CKE 4 → 5 upgrade path "format" does not always map to "heading", it could map to "codeBlock" too, or both, or neither

merge-requests/1969/merge
catch 2022-03-15 12:13:09 +00:00
parent 718fa096fd
commit 196b68e9de
7 changed files with 145 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
core/modules/ckeditor5/src/HTMLRestrictions.php
View File

@ -53,6 +53,17 @@ final class HTMLRestrictions {
*/
private $elements;
/**
* Whether unrestricted, in other words: arbitrary HTML allowed.
*
* Used for when FilterFormatInterface::getHTMLRestrictions() returns `FALSE`,
* e.g. in case of the default "Full HTML" text format.
*
* @var bool
* @see \Drupal\filter\Plugin\FilterInterface::getHTMLRestrictions()
*/
private $unrestricted = FALSE;
/**
* Wildcard types, and the methods that return tags the wildcard represents.
*
@ -214,6 +225,15 @@ final class HTMLRestrictions {
return new self([]);
}
/**
* Whether this set of HTML restrictions is unrestricted.
*
* @return bool
*/
public function isUnrestricted(): bool {
return $this->unrestricted;
}
/**
* Whether this is the empty set of HTML restrictions.
*
@ -249,6 +269,18 @@ final class HTMLRestrictions {
return self::fromObjectWithHtmlRestrictions($text_format);
}
/**
* Constructs an unrestricted set of HTML restrictions.
*
* @return \Drupal\ckeditor5\HTMLRestrictions
*/
private static function unrestricted(): self {
// @todo Refine in https://www.drupal.org/project/drupal/issues/3231336, including adding support for all operations.
$restrictions = HTMLRestrictions::emptySet();
$restrictions->unrestricted = TRUE;
return $restrictions;
}
/**
* Constructs a set of HTML restrictions matching the given object.
*
@ -272,6 +304,11 @@ final class HTMLRestrictions {
throw new \InvalidArgumentException();
}
if ($object->getHtmlRestrictions() === FALSE) {
// @todo Refine in https://www.drupal.org/project/drupal/issues/3231336
return self::unrestricted();
}
$restrictions = $object->getHTMLRestrictions();
if (!isset($restrictions['allowed'])) {
// @todo Handle HTML restrictor filters that only set forbidden_tags

51
core/modules/ckeditor5/src/Plugin/CKEditor4To5Upgrade/Core.php
View File

@ -4,6 +4,7 @@ declare(strict_types=1);
namespace Drupal\ckeditor5\Plugin\CKEditor4To5Upgrade;
use Drupal\ckeditor5\HTMLRestrictions;
use Drupal\ckeditor5\Plugin\CKEditor4To5UpgradePluginInterface;
use Drupal\Core\Plugin\PluginBase;
use Drupal\filter\FilterFormatInterface;
@ -69,15 +70,15 @@ class Core extends PluginBase implements CKEditor4To5UpgradePluginInterface {
/**
* {@inheritdoc}
*/
public function mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem(string $cke4_button): ?string {
public function mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem(string $cke4_button, HTMLRestrictions $text_format_html_restrictions): ?array {
switch ($cke4_button) {
// @see \Drupal\ckeditor\Plugin\CKEditorPlugin\DrupalImage
case 'DrupalImage':
return 'uploadImage';
return ['uploadImage'];
// @see \Drupal\ckeditor\Plugin\CKEditorPlugin\DrupalLink
case 'DrupalLink':
return 'link';
return ['link'];
case 'DrupalUnlink':
return NULL;
@ -94,37 +95,51 @@ class Core extends PluginBase implements CKEditor4To5UpgradePluginInterface {
case 'Indent':
case 'Undo':
case 'Redo':
return lcfirst($cke4_button);
return [lcfirst($cke4_button)];
case 'Blockquote':
return 'blockQuote';
return ['blockQuote'];
case 'JustifyLeft':
return "alignment:left";
return ["alignment:left"];
case 'JustifyCenter':
return "alignment:center";
return ["alignment:center"];
case 'JustifyRight':
return "alignment:right";
return ["alignment:right"];
case 'JustifyBlock':
return "alignment:justify";
return ["alignment:justify"];
case 'HorizontalRule':
return 'horizontalLine';
return ['horizontalLine'];
case 'Format':
return 'heading';
if ($text_format_html_restrictions->isUnrestricted()) {
// When no restrictions exist, all tags possibly supported by "Format"
// in CKEditor 4 must be supported.
return ['heading'];
}
$allowed_elements = $text_format_html_restrictions->getAllowedElements();
// Check if <h*> is supported.
// Merely checking the existence of the array key is sufficient; this
// plugin does not set or need any additional attributes.
// @see \Drupal\filter\Plugin\FilterInterface::getHTMLRestrictions()
$intersect = array_intersect(['h2', 'h3', 'h4', 'h5', 'h6'], array_keys($allowed_elements));
return count($intersect) > 0 ? ['heading'] : NULL;
case 'Table':
return 'insertTable';
return ['insertTable'];
case 'Source':
return 'sourceEditing';
return ['sourceEditing'];
case 'Strike':
return 'strikethrough';
return ['strikethrough'];
case 'Cut':
case 'Copy':
@ -139,7 +154,7 @@ class Core extends PluginBase implements CKEditor4To5UpgradePluginInterface {
// @see \Drupal\ckeditor5\Plugin\CKEditor5Plugin\RemoveFormat
case 'RemoveFormat':
return 'removeFormat';
return ['removeFormat'];
// @see \Drupal\ckeditor\Plugin\CKEditorPlugin\StylesCombo
case 'Styles':
@ -148,15 +163,15 @@ class Core extends PluginBase implements CKEditor4To5UpgradePluginInterface {
// @see \Drupal\ckeditor5\Plugin\CKEditor5Plugin\specialCharacters
case 'SpecialChar':
return 'specialCharacters';
return ['specialCharacters'];
// @see \Drupal\ckeditor\Plugin\CKEditorPlugin\Language
case 'Language':
return 'textPartLanguage';
return ['textPartLanguage'];
// @see \Drupal\media_library\Plugin\CKEditorPlugin\DrupalMediaLibrary
case 'DrupalMediaLibrary':
return 'drupalMedia';
return ['drupalMedia'];
default:
throw new \OutOfBoundsException();

12
core/modules/ckeditor5/src/Plugin/CKEditor4To5UpgradePluginInterface.php
View File

@ -4,6 +4,7 @@ declare(strict_types = 1);
namespace Drupal\ckeditor5\Plugin;
use Drupal\ckeditor5\HTMLRestrictions;
use Drupal\Component\Plugin\PluginInspectionInterface;
use Drupal\filter\FilterFormatInterface;
@ -24,10 +25,13 @@ interface CKEditor4To5UpgradePluginInterface extends PluginInspectionInterface {
*
* @param string $cke4_button
* A valid CKEditor 4 button name.
* @param \Drupal\ckeditor5\HTMLRestrictions $text_format_html_restrictions
* The restrictions of the text format, if this upgrade plugin needs to
* inspect the text format's HTML restrictions to make a decision.
*
* @return string|null
* The equivalent CKEditor 5 toolbar item, or NULL if no equivalent exists.
* In either case, the button name must be added to the annotation.
* @return string[]|null
* The equivalent CKEditor 5 toolbar items, or NULL if no equivalent exists.
* In either case, the button names must be added to the annotation.
*
* @throws \OutOfBoundsException
* Thrown when this plugin does not know whether an equivalent exists.
@ -35,7 +39,7 @@ interface CKEditor4To5UpgradePluginInterface extends PluginInspectionInterface {
* @see \Drupal\ckeditor\CKEditorPluginButtonsInterface
* @see \Drupal\ckeditor5\Annotation\CKEditor4To5Upgrade
*/
public function mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem(string $cke4_button): ?string;
public function mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem(string $cke4_button, HTMLRestrictions $text_format_html_restrictions): ?array;
/**
* Maps CKEditor 4 settings to the CKEditor 5 equivalent, if needed.

12
core/modules/ckeditor5/src/Plugin/CKEditor4To5UpgradePluginManager.php
View File

@ -5,6 +5,7 @@ declare(strict_types = 1);
namespace Drupal\ckeditor5\Plugin;
use Drupal\ckeditor5\Annotation\CKEditor4To5Upgrade;
use Drupal\ckeditor5\HTMLRestrictions;
use Drupal\Component\Assertion\Inspector;
use Drupal\Core\Cache\CacheBackendInterface;
use Drupal\Core\Extension\ModuleHandlerInterface;
@ -116,9 +117,12 @@ class CKEditor4To5UpgradePluginManager extends DefaultPluginManager {
*
* @param string $cke4_button
* A valid CKEditor 4 button name.
* @param \Drupal\ckeditor5\HTMLRestrictions $text_format_html_restrictions
* The restrictions of the text format, to allow an upgrade plugin to
* inspect the text format's HTML restrictions to make a decision.
*
* @return string|null
* The equivalent CKEditor 5 toolbar item, or NULL if no equivalent exists.
* @return string[]|null
* The equivalent CKEditor 5 toolbar items, or NULL if no equivalent exists.
*
* @throws \OutOfBoundsException
* Thrown when no upgrade path exists.
@ -127,7 +131,7 @@ class CKEditor4To5UpgradePluginManager extends DefaultPluginManager {
*
* @see \Drupal\ckeditor\CKEditorPluginButtonsInterface
*/
public function mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem(string $cke4_button): ?string {
public function mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem(string $cke4_button, HTMLRestrictions $text_format_html_restrictions): ?array {
$this->validateAndBuildMaps();
if (!isset($this->cke4ButtonsMap[$cke4_button])) {
@ -136,7 +140,7 @@ class CKEditor4To5UpgradePluginManager extends DefaultPluginManager {
$plugin_id = $this->cke4ButtonsMap[$cke4_button];
try {
return $this->createInstance($plugin_id)->mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem($cke4_button);
return $this->createInstance($plugin_id)->mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem($cke4_button, $text_format_html_restrictions);
}
catch (\OutOfBoundsException $e) {
throw new \LogicException(sprintf('The "%s" CKEditor4To5Upgrade plugin claims to provide an upgrade path for the "%s" CKEditor 4 button but does not.', $plugin_id, $cke4_button));

11
core/modules/ckeditor5/src/SmartDefaultSettings.php
View File

@ -124,7 +124,7 @@ final class SmartDefaultSettings {
$old_editor = $editor->id() ? Editor::load($editor->id()) : NULL;
if ($old_editor && $old_editor->getEditor() === 'ckeditor') {
$enabled_cke4_plugins = $this->getEnabledCkeditor4Plugins($old_editor);
[$upgraded_settings, $messages] = $this->createSettingsFromCKEditor4($old_editor->getSettings(), $enabled_cke4_plugins);
[$upgraded_settings, $messages] = $this->createSettingsFromCKEditor4($old_editor->getSettings(), $enabled_cke4_plugins, HTMLRestrictions::fromTextFormat($old_editor->getFilterFormat()));
$editor->setSettings($upgraded_settings);
$editor->setImageUploadSettings($old_editor->getImageUploadSettings());
}
@ -200,6 +200,9 @@ final class SmartDefaultSettings {
* @param string[] $enabled_ckeditor4_plugins
* The list of enabled CKEditor 4 plugins: their settings will be mapped to
* the CKEditor 5 equivalents, if they have any.
* @param \Drupal\ckeditor5\HTMLRestrictions $text_format_html_restrictions
* The restrictions of the text format, to allow an upgrade plugin to
* inspect the text format's HTML restrictions to make a decision.
*
* @return array
* An array with two values:
@ -210,7 +213,7 @@ final class SmartDefaultSettings {
* Thrown when an upgrade plugin is attempting to generate plugin settings
* for a CKEditor 4 plugin upgrade path that have already been generated.
*/
private function createSettingsFromCKEditor4(array $ckeditor4_settings, array $enabled_ckeditor4_plugins): array {
private function createSettingsFromCKEditor4(array $ckeditor4_settings, array $enabled_ckeditor4_plugins, HTMLRestrictions $text_format_html_restrictions): array {
$settings = [
'toolbar' => [
'items' => [],
@ -226,7 +229,7 @@ final class SmartDefaultSettings {
$some_added = FALSE;
foreach ($group['items'] as $cke4_button) {
try {
$equivalent = $this->upgradePluginManager->mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem($cke4_button);
$equivalent = $this->upgradePluginManager->mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem($cke4_button, $text_format_html_restrictions);
}
catch (\OutOfBoundsException $e) {
$messages[] = $this->t('The CKEditor 4 button %button does not have a known upgrade path. If it allowed editing markup, then you can do so now through the Source Editing functionality.', [
@ -235,7 +238,7 @@ final class SmartDefaultSettings {
continue;
}
if ($equivalent) {
$settings['toolbar']['items'][] = $equivalent;
$settings['toolbar']['items'] = array_merge($settings['toolbar']['items'], $equivalent);
$some_added = TRUE;
}
}

18
core/modules/ckeditor5/tests/src/Kernel/CKEditor4to5UpgradeCompletenessTest.php
View File

@ -5,7 +5,9 @@ declare(strict_types = 1);
namespace Drupal\Tests\ckeditor5\Kernel;
use Drupal\ckeditor\CKEditorPluginConfigurableInterface;
use Drupal\ckeditor5\HTMLRestrictions;
use Drupal\ckeditor5\Plugin\CKEditor5PluginElementsSubsetInterface;
use Drupal\Component\Assertion\Inspector;
use Drupal\Component\Utility\NestedArray;
use Drupal\filter\Entity\FilterFormat;
use Drupal\KernelTests\KernelTestBase;
@ -89,11 +91,13 @@ class CKEditor4to5UpgradeCompletenessTest extends KernelTestBase {
$cke4_buttons = array_keys(NestedArray::mergeDeepArray($this->cke4PluginManager->getButtons()));
foreach ($cke4_buttons as $button) {
$equivalent = $this->upgradePluginManager->mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem($button);
$this->assertTrue($equivalent === NULL || is_string($equivalent));
// The returned equivalent CKEditor 5 toolbar item must exist.
$equivalent = $this->upgradePluginManager->mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem($button, HTMLRestrictions::emptySet());
$this->assertTrue($equivalent === NULL || (is_array($equivalent) && Inspector::assertAllStrings($equivalent)));
// The returned equivalent CKEditor 5 toolbar item(s) must exist.
if (is_string($equivalent)) {
$this->assertArrayHasKey($equivalent, $this->cke5PluginManager->getToolbarItems());
foreach (explode(',', $equivalent) as $equivalent_cke5_toolbar_item) {
$this->assertArrayHasKey($equivalent_cke5_toolbar_item, $this->cke5PluginManager->getToolbarItems());
}
}
}
}
@ -186,7 +190,7 @@ class CKEditor4to5UpgradeCompletenessTest extends KernelTestBase {
$this->expectException(\OutOfBoundsException::class);
$this->expectExceptionMessage('The "DrupalImage" CKEditor 4 button is already being upgraded by the "core" CKEditor4To5Upgrade plugin, the "foo" plugin is as well. This conflict needs to be resolved.');
$this->upgradePluginManager->mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem('foo');
$this->upgradePluginManager->mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem('foo', HTMLRestrictions::emptySet());
}
/**
@ -199,7 +203,7 @@ class CKEditor4to5UpgradeCompletenessTest extends KernelTestBase {
$this->expectException(\LogicException::class);
$this->expectExceptionMessage('The "foo" CKEditor4To5Upgrade plugin claims to provide an upgrade path for the "foo" CKEditor 4 button but does not.');
$this->upgradePluginManager->mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem('foo');
$this->upgradePluginManager->mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem('foo', HTMLRestrictions::emptySet());
}
/**
@ -212,7 +216,7 @@ class CKEditor4to5UpgradeCompletenessTest extends KernelTestBase {
$this->expectException(\OutOfBoundsException::class);
$this->expectExceptionMessage('The "stylescombo" CKEditor 4 plugin\'s settings are already being upgraded by the "core" CKEditor4To5Upgrade plugin, the "foo" plugin is as well. This conflict needs to be resolved.');
$this->upgradePluginManager->mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem('foo');
$this->upgradePluginManager->mapCKEditor4ToolbarButtonToCKEditor5ToolbarItem('foo', HTMLRestrictions::emptySet());
}
/**

41
core/modules/ckeditor5/tests/src/Kernel/SmartDefaultSettingsTest.php
View File

@ -101,6 +101,18 @@ class SmartDefaultSettingsTest extends KernelTestBase {
Yaml::parseFile('core/profiles/standard/config/install/editor.editor.basic_html.yml')
)->save();
$new_value = str_replace(['<h2 id> ', '<h3 id> ', '<h4 id> ', '<h5 id> ', '<h6 id> '], '', $current_value);
$basic_html_format_without_headings = $basic_html_format;
$basic_html_format_without_headings['name'] .= ' (without H*)';
$basic_html_format_without_headings['format'] = 'basic_html_without_headings';
NestedArray::setValue($basic_html_format_without_headings, $allowed_html_parents, $new_value);
FilterFormat::create($basic_html_format_without_headings)->save();
Editor::create(
['format' => 'basic_html_without_headings']
+
Yaml::parseFile('core/profiles/standard/config/install/editor.editor.basic_html.yml')
)->save();
$new_value = str_replace('<p>', '<p class="text-align-center text-align-justify">', $current_value);
$basic_html_format_with_alignable_p = $basic_html_format;
$basic_html_format_with_alignable_p['name'] .= ' (with alignable paragraph support)';
@ -507,6 +519,35 @@ class SmartDefaultSettingsTest extends KernelTestBase {
),
];
yield "basic_html_without_headings can be switched to CKEditor 5 without problems, heading configuration computed automatically" => [
'format_id' => 'basic_html_without_headings',
'filters_to_drop' => $basic_html_test_case['filters_to_drop'],
'expected_ckeditor5_settings' => [
'toolbar' => [
'items' => array_merge(
array_slice($basic_html_test_case['expected_ckeditor5_settings']['toolbar']['items'], 0, 10),
array_slice($basic_html_test_case['expected_ckeditor5_settings']['toolbar']['items'], 12),
),
],
'plugins' => [
'ckeditor5_sourceEditing' => [
'allowed_tags' => array_values(array_diff(
$basic_html_test_case['expected_ckeditor5_settings']['plugins']['ckeditor5_sourceEditing']['allowed_tags'],
['<h2 id>', '<h3 id>', '<h4 id>', '<h5 id>', '<h6 id>'],
)),
],
'ckeditor5_imageResize' => ['allow_resize' => TRUE],
'ckeditor5_language' => $basic_html_test_case['expected_ckeditor5_settings']['plugins']['ckeditor5_language'],
],
],
'expected_superset' => $basic_html_test_case['expected_superset'],
'expected_fundamental_compatibility_violations' => $basic_html_test_case['expected_fundamental_compatibility_violations'],
'expected_messages' => array_merge(
$basic_html_test_case['expected_messages'],
['This format\'s HTML filters includes plugins that support the following tags, but not some of their attributes. To ensure these attributes remain supported by this text format, the following were added to the Source Editing plugin\'s <em>Manually editable HTML tags</em>: &lt;a hreflang&gt; &lt;blockquote cite&gt; &lt;ul type&gt; &lt;ol start type&gt;.'],
),
];
yield "basic_html_with_alignable_p can be switched to CKEditor 5 without problems, align buttons added automatically" => [
'format_id' => 'basic_html_with_alignable_p',
'filters_to_drop' => $basic_html_test_case['filters_to_drop'],