From fb85980012ba32ca089b0b0fdccfa25ec4329211 Mon Sep 17 00:00:00 2001 From: bnjmnm <benm@umich.edu> Date: Wed, 17 Aug 2022 12:32:35 -0400 Subject: [PATCH] Issue #3294908 by nod_, Wim Leers, DanielVeza: Configuration overlaps between Styles and other CKE5 plugins --- ...tyleSensibleElementConstraintValidator.php | 19 +- ...itor5_plugin_conditions_test.ckeditor5.yml | 12 + .../tests/src/Kernel/ValidatorsTest.php | 205 +++++++++++++++++- 3 files changed, 234 insertions(+), 2 deletions(-) diff --git a/core/modules/ckeditor5/src/Plugin/Validation/Constraint/StyleSensibleElementConstraintValidator.php b/core/modules/ckeditor5/src/Plugin/Validation/Constraint/StyleSensibleElementConstraintValidator.php index 4be7d8ba1402..9c0622716a46 100644 --- a/core/modules/ckeditor5/src/Plugin/Validation/Constraint/StyleSensibleElementConstraintValidator.php +++ b/core/modules/ckeditor5/src/Plugin/Validation/Constraint/StyleSensibleElementConstraintValidator.php @@ -107,7 +107,24 @@ private static function intersectionWithClasses(HTMLRestrictions $a, HTMLRestric $tags_from_b = array_diff(array_keys($b->getConcreteSubset()->getAllowedElements()), ['*']); $a = $a->merge(new HTMLRestrictions(array_fill_keys($tags_from_b, FALSE))); $b = $b->merge(new HTMLRestrictions(array_fill_keys($tags_from_a, FALSE))); - $intersection = $a->intersect($b); + // When a plugin allows all classes on a tag, we assume there is no + // problem with having the style plugin adding classes to that element. + // When allowing all classes we don't expect a specific user experience + // so adding a class through a plugin or the style plugin is the same. + $b_without_class_wildcard = $b->getAllowedElements(); + foreach ($b_without_class_wildcard as $allowedElement => $config) { + // When all classes are allowed, remove the configuration so that + // the intersect below does not include classes. + if (!empty($config['class']) && $config['class'] === TRUE) { + unset($b_without_class_wildcard[$allowedElement]['class']); + } + // HTMLRestrictions does not accept a tag with an empty array, make sure + // to remove them here. + if (empty($b_without_class_wildcard[$allowedElement])) { + unset($b_without_class_wildcard[$allowedElement]); + } + } + $intersection = $a->intersect(new HTMLRestrictions($b_without_class_wildcard)); // Leverage the "GHS configuration" representation to easily find whether // there is an intersection for classes. Other implementations are possible. diff --git a/core/modules/ckeditor5/tests/modules/ckeditor5_plugin_conditions_test/ckeditor5_plugin_conditions_test.ckeditor5.yml b/core/modules/ckeditor5/tests/modules/ckeditor5_plugin_conditions_test/ckeditor5_plugin_conditions_test.ckeditor5.yml index 6319b3c588e0..9a4623964a2f 100644 --- a/core/modules/ckeditor5/tests/modules/ckeditor5_plugin_conditions_test/ckeditor5_plugin_conditions_test.ckeditor5.yml +++ b/core/modules/ckeditor5/tests/modules/ckeditor5_plugin_conditions_test/ckeditor5_plugin_conditions_test.ckeditor5.yml @@ -12,3 +12,15 @@ ckeditor5_plugin_conditions_test_plugins_condition: - ckeditor5_table elements: - <foo> + +ckeditor5_plugin_conditions_test_plugin_allow_all_classes_on_kbd: + ckeditor5: + plugins: {} + drupal: + label: TEST — Allow all classes on kbd + toolbar_items: + kbdAllClasses: + label: All classes on kbd (Test Plugins Condition) + elements: + - <kbd> + - <kbd class> diff --git a/core/modules/ckeditor5/tests/src/Kernel/ValidatorsTest.php b/core/modules/ckeditor5/tests/src/Kernel/ValidatorsTest.php index 9a7505a5f0ad..0dbd2c1da2db 100644 --- a/core/modules/ckeditor5/tests/src/Kernel/ValidatorsTest.php +++ b/core/modules/ckeditor5/tests/src/Kernel/ValidatorsTest.php @@ -14,7 +14,7 @@ use Drupal\Tests\SchemaCheckTestTrait; use Symfony\Component\Yaml\Yaml; -// cspell:ignore onhover +// cspell:ignore onhover baguette /** * @covers \Drupal\ckeditor5\Plugin\Validation\Constraint\ToolbarItemConstraintValidator @@ -1253,6 +1253,209 @@ public function providerPair(): array { 'settings.plugins.ckeditor5_style' => 'The <em class="placeholder">Style</em> plugin needs another plugin to create <code><blockquote></code>, for it to be able to create the following attributes: <code><blockquote class="highlighted"></code>. Enable a plugin that supports creating this tag. If none exists, you can configure the Source Editing plugin to support it.', ], ]; + $data['INVALID: Style plugin configured to add class already added by an other plugin'] = [ + 'settings' => [ + 'toolbar' => [ + 'items' => [ + 'alignment', + 'style', + ], + ], + 'plugins' => [ + 'ckeditor5_alignment' => [ + 'enabled_alignments' => ['justify'], + ], + 'ckeditor5_style' => [ + 'styles' => [ + [ + 'label' => 'Text align', + 'element' => '<p class="text-align-justify">', + ], + ], + ], + ], + ], + 'image_upload' => [ + 'status' => FALSE, + ], + 'filters' => [ + 'filter_html' => [ + 'id' => 'filter_html', + 'provider' => 'filter', + 'status' => TRUE, + 'weight' => 0, + 'settings' => [ + 'allowed_html' => '<p class="text-align-justify"> <br>', + 'filter_html_help' => TRUE, + 'filter_html_nofollow' => TRUE, + ], + ], + ], + 'violations' => [ + 'settings.plugins.ckeditor5_style.styles.0.element' => 'A style must only specify classes not supported by other plugins. The <code>text-align-justify</code> classes on <code><p></code> are already supported by the enabled <em class="placeholder">Alignment</em> plugin.', + ], + ]; + $data['VALID: Style plugin configured to add new class to an already restricted tag'] = [ + 'settings' => [ + 'toolbar' => [ + 'items' => [ + 'alignment', + 'style', + ], + ], + 'plugins' => [ + 'ckeditor5_alignment' => [ + 'enabled_alignments' => ['justify'], + ], + 'ckeditor5_style' => [ + 'styles' => [ + [ + 'label' => 'Add baguette class', + 'element' => '<p class="baguette">', + ], + ], + ], + ], + ], + 'image_upload' => [ + 'status' => FALSE, + ], + 'filters' => [ + 'filter_html' => [ + 'id' => 'filter_html', + 'provider' => 'filter', + 'status' => TRUE, + 'weight' => 0, + 'settings' => [ + 'allowed_html' => '<p class="text-align-justify baguette"> <br>', + 'filter_html_help' => TRUE, + 'filter_html_nofollow' => TRUE, + ], + ], + ], + 'violations' => [], + ]; + $data['VALID: Style plugin configured to add class to an element provided by an explicit plugin that already allows all classes'] = [ + 'settings' => [ + 'toolbar' => [ + 'items' => [ + 'kbdAllClasses', + 'style', + ], + ], + 'plugins' => [ + 'ckeditor5_style' => [ + 'styles' => [ + [ + 'label' => 'Add baguette class', + 'element' => '<kbd class="baguette">', + ], + ], + ], + ], + ], + 'image_upload' => [ + 'status' => FALSE, + ], + 'filters' => [ + 'filter_html' => [ + 'id' => 'filter_html', + 'provider' => 'filter', + 'status' => TRUE, + 'weight' => 0, + 'settings' => [ + 'allowed_html' => '<p> <br> <kbd class>', + 'filter_html_help' => TRUE, + 'filter_html_nofollow' => TRUE, + ], + ], + ], + 'violations' => [], + ]; + $data['VALID: Style plugin configured to add class to GHS-supported HTML5 tag'] = [ + 'settings' => [ + 'toolbar' => [ + 'items' => [ + 'style', + 'sourceEditing', + ], + ], + 'plugins' => [ + 'ckeditor5_sourceEditing' => [ + 'allowed_tags' => [ + '<kbd>', + ], + ], + 'ckeditor5_style' => [ + 'styles' => [ + [ + 'label' => 'Add baguette class', + 'element' => '<kbd class="baguette">', + ], + ], + ], + ], + ], + 'image_upload' => [ + 'status' => FALSE, + ], + 'filters' => [ + 'filter_html' => [ + 'id' => 'filter_html', + 'provider' => 'filter', + 'status' => TRUE, + 'weight' => 0, + 'settings' => [ + 'allowed_html' => '<p> <br> <kbd class="baguette">', + 'filter_html_help' => TRUE, + 'filter_html_nofollow' => TRUE, + ], + ], + ], + 'violations' => [], + ]; + $data['VALID: Style plugin configured to add class to GHS-supported HTML5 tag that already allows all classes'] = [ + 'settings' => [ + 'toolbar' => [ + 'items' => [ + 'style', + 'sourceEditing', + ], + ], + 'plugins' => [ + 'ckeditor5_sourceEditing' => [ + 'allowed_tags' => [ + '<bdi class>', + ], + ], + 'ckeditor5_style' => [ + 'styles' => [ + [ + 'label' => 'Bidirectional name', + 'element' => '<bdi class="name">', + ], + ], + ], + ], + ], + 'image_upload' => [ + 'status' => FALSE, + ], + 'filters' => [ + 'filter_html' => [ + 'id' => 'filter_html', + 'provider' => 'filter', + 'status' => TRUE, + 'weight' => 0, + 'settings' => [ + 'allowed_html' => '<p> <br> <bdi class>', + 'filter_html_help' => TRUE, + 'filter_html_nofollow' => TRUE, + ], + ], + ], + 'violations' => [], + ]; return $data; } -- GitLab