Issue #3268307 by lauriii, Wim Leers: $block wildcard resolves into a superset...

Issue #3268307 by lauriii, Wim Leers: $block wildcard resolves into a superset of the actual $block tags

core/modules/ckeditor5/ckeditor5.api.php

@@ -119,11 +119,12 @@
  *   make it discoverable.
  * - drupal.elements: A list of elements and attributes the plugin allows use of
  *   within CKEditor 5. This uses the same syntax as the 'filter_html' plugin
- *   with an additional special keyword: '<$block>' . Using
- *   '<$block [attribute(s)]>`  will permit the provided attributes in all block
- *   level tags that are explicitly enabled in any plugin. i.e. if only '<p>',
- *   '<h3>' and '<h2>' tags are allowed, then '<$block data-something>'  will
- *   allow the 'data-something' attribute for '<p>', '<h3>' and '<h2>' tags.
+ *   with an additional special keyword: '<$text-container>' . Using
+ *   '<$text-container [attribute(s)]>` will permit the provided
+ *   attributes in all CKEditor 5's `$block` text container tags that are
+ *   explicitly enabled in any plugin. i.e. if only '<p>', '<h3>' and '<h2>'
+ *   tags are allowed, then '<$text-container data-something>' will allow the
+ *   'data-something' attribute for '<p>', '<h3>' and '<h2>' tags.
  * - drupal.toolbar_items: List of toolbar items the plugin provides. Keyed by a
  *   machine name and the value being a pair defining the label:
  *   @code

core/modules/ckeditor5/ckeditor5.ckeditor5.yml

@@ -355,7 +355,7 @@ ckeditor5_alignment:
       alignment:
         label: Text alignment
     elements:
-      - <$block class="text-align-left text-align-center text-align-right text-align-justify">
+      - <$text-container class="text-align-left text-align-center text-align-right text-align-justify">
 
 ckeditor5_alignment.left:
   ckeditor5: *alignment_ckeditor5_section
@@ -365,7 +365,7 @@ ckeditor5_alignment.left:
       "alignment:left":
         label: Align left
     elements:
-      - <$block class="text-align-left">
+      - <$text-container class="text-align-left">
     <<: *alignment_drupal_section
 
 ckeditor5_alignment.center:
@@ -376,7 +376,7 @@ ckeditor5_alignment.center:
       "alignment:center":
         label: Align center
     elements:
-      - <$block class="text-align-center">
+      - <$text-container class="text-align-center">
     <<: *alignment_drupal_section
 
 ckeditor5_alignment.right:
@@ -387,7 +387,7 @@ ckeditor5_alignment.right:
       "alignment:right":
         label: Align right
     elements:
-      - <$block class="text-align-right">
+      - <$text-container class="text-align-right">
     <<: *alignment_drupal_section
 
 ckeditor5_alignment.justify:
@@ -398,7 +398,7 @@ ckeditor5_alignment.justify:
       "alignment:justify":
         label: Justify
     elements:
-      - <$block class="text-align-justify">
+      - <$text-container class="text-align-justify">
     <<: *alignment_drupal_section
 
 ckeditor5_removeFormat:

core/modules/ckeditor5/src/HTMLRestrictions.php

@@ -9,7 +9,6 @@
 use Drupal\filter\FilterFormatInterface;
 use Drupal\filter\Plugin\Filter\FilterHtml;
 use Drupal\filter\Plugin\FilterInterface;
-use Masterminds\HTML5\Elements;
 
 /**
  * Represents a set of HTML restrictions.
@@ -70,7 +69,7 @@ final class HTMLRestrictions {
    * @var string[]
    */
   private const WILDCARD_ELEMENT_METHODS = [
-    '$block' => 'getBlockElementList',
+    '$text-container' => 'getTextContainerElementList',
   ];
 
   /**
@@ -338,8 +337,8 @@ private static function fromObjectWithHtmlRestrictions(object $object): HTMLRest
    * @see ::toCKEditor5ElementsArray()
    */
   public static function fromString(string $elements_string): HTMLRestrictions {
-    // Preprocess wildcard tags: convert `<$block>` to
-    // `<__preprocessed-wildcard-block__>`.
+    // Preprocess wildcard tags: convert `<$text-container>` to
+    // `<__preprocessed-wildcard-text-container__>`.
     // Note: unknown wildcard tags will trigger a validation error in
     // ::validateAllowedRestrictionsPhase1().
     $replaced_wildcard_tags = [];
@@ -361,8 +360,8 @@ public static function fromString(string $elements_string): HTMLRestrictions {
     // @todo remove this in https://www.drupal.org/project/drupal/issues/3226368
     unset($allowed_elements['__zqh6vxfbk3cg__']);
 
-    // Postprocess tag wildcards: convert `<__preprocessed-wildcard-block__>` to
-    // `<$block>`.
+    // Postprocess tag wildcards: convert
+    // `<__preprocessed-wildcard-text-container__>` to `<$text-container>`.
     foreach ($replaced_wildcard_tags as $processed => $original) {
       if (isset($allowed_elements[$processed])) {
         $allowed_elements[$original] = $allowed_elements[$processed];
@@ -772,7 +771,7 @@ public function merge(HTMLRestrictions $other): HTMLRestrictions {
    */
   private static function applyOperation(HTMLRestrictions $a, HTMLRestrictions $b, string $operation_method_name): HTMLRestrictions {
     // 1. Operation applied to wildcard tags that exist in both operands.
-    // For example: <$block id> in both operands.
+    // For example: <$text-container id> in both operands.
     $a_wildcard = $a->getWildcardSubset();
     $b_wildcard = $b->getWildcardSubset();
     $wildcard_op_result = $a_wildcard->$operation_method_name($b_wildcard);
@@ -784,7 +783,8 @@ private static function applyOperation(HTMLRestrictions $a, HTMLRestrictions $b,
 
     // 2. Operation applied with wildcard tags resolved into concrete tags.
     // For example: <p class="text-align-center"> in the first operand and
-    // <$block class="text-align-center"> in the second operand.
+    // <$text-container class="text-align-center"> in the second
+    // operand.
     $a_concrete = self::resolveWildcards($a);
     $b_concrete = self::resolveWildcards($b);
     $concrete_op_result = $a_concrete->$operation_method_name($b_concrete);
@@ -901,15 +901,16 @@ private static function resolveWildcards(HTMLRestrictions $r): HTMLRestrictions
     // let ::merge() pick the most permissive one.
     // This is necessary because resolving wildcards may result in concrete tags
     // becoming either more permissive:
-    // - if $r is `<p> <$block class="foo">`
+    // - if $r is `<p> <$text-container class="foo">`
     // - then $naive will be `<p class="foo">`
-    // - merging them yields `<p class="foo"> <$block class="foo">`
+    // - merging them yields `<p class="foo"> <$text-container class="foo">`
     // - diffing the wildcard subsets yields just `<p class="foo">`
     // Or it could result in concrete tags being unaffected by the resolved
     // wildcards:
-    // - if $r is `<p class> <$block class="foo">`
+    // - if $r is `<p class> <$text-container class="foo">`
     // - then $naive will be `<p class="foo">`
-    // - merging them yields `<p class> <$block class="foo">` again
+    // - merging them yields `<p class> <$text-container class="foo">`
+    //   again
     // - diffing the wildcard subsets yields just `<p class>`
     return $r->merge($naive_resolution)->doDiff($r->getWildcardSubset());
   }
@@ -996,12 +997,7 @@ public function toGeneralHtmlSupportConfig(): array {
     $allowed = [];
     // Resolve any remaining wildcards based on Drupal's assumptions on
     // wildcards to ensure all HTML tags that Drupal thinks are supported are
-    // truly supported by CKEditor 5. For example: the <$block> wildcard does
-    // NOT correspond to block-level HTML tags, but to CKEditor 5 elements that
-    // behave like blocks. Knowing the list of concrete HTML tags this maps to
-    // is impossible without executing JavaScript, which PHP cannot do. By
-    // generating this GHS configuration, we can guarantee that Drupal's only
-    // possible interpretation also actually works.
+    // truly supported by CKEditor 5.
     $elements = self::resolveWildcards($this)->getAllowedElements();
     foreach ($elements as $tag => $attributes) {
       $to_allow = ['name' => $tag];
@@ -1048,25 +1044,51 @@ public function toGeneralHtmlSupportConfig(): array {
   }
 
   /**
-   * Gets a list of block-level elements.
+   * Gets a list of CKEditor 5's `$block` text container elements.
+   *
+   * This is a hard coded list of known elements that CKEditor 5 uses as
+   * `$block` text container elements. The elements listed here are registered
+   * with `inheritAllFrom: "$block"` to the CKEditor 5 schema. This list
+   * corresponds to the `$text-container` wildcard in Drupal configuration.
+   *
+   *
+   * This group of elements is special because they allow text as an immediate
+   * child node. These elements are also allowed to be used for text styles that
+   * must be applied to the wrapper instead of inline to the text, such as text
+   * alignment.
+   *
+   * This list is highly opinionated. It is based on decisions made upstream in
+   * CKEditor 5. For example, `<blockquote>` is not considered as a `$block`
+   * text container, meaning that text inside `<blockquote>` needs to always be
+   * wrapped by an element that is `$block` text container such as `<p>`. This
+   * list also excludes some special case text container elements like
+   * `<caption>` that allow containing text directly inside the element, yet do
+   * not fully implement the `$block` text container interface.
+   *
+   * It is acceptable to list the elements here because the list of elements is
+   * not likely to change often. If the list changed, an upgrade path would be
+   * required anyway. In most cases, missing elements would only impact new
+   * functionality shipped in upstream.
+   *
+   * @see https://ckeditor.com/docs/ckeditor5/latest/framework/guides/deep-dive/schema.html#generic-items
    *
    * @return string[]
    *   An array of block-level element tags.
    */
-  private static function getBlockElementList(): array {
-    return array_filter(array_keys(Elements::$html5), function (string $element): bool {
-      return Elements::isA($element, Elements::BLOCK_TAG);
-    });
+  private static function getTextContainerElementList(): array {
+    return [
+      'div', 'p', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'pre', 'li',
+    ];
   }
 
   /**
    * Computes the tags that match the provided wildcard.
    *
    * A wildcard tag in element config is a way of representing multiple tags
-   * with a single item, such as `<$block>` to represent all block tags. Each
-   * wildcard should have a corresponding callback method listed in
-   * WILDCARD_ELEMENT_METHODS that returns the set of tags represented by the
-   * wildcard.
+   * with a single item, such as `<$text-container>` to represent CKEditor 5's
+   * `$block` text container tags. Each wildcard should have a corresponding
+   * callback method listed in WILDCARD_ELEMENT_METHODS that returns the set of
+   * tags represented by the wildcard.
    *
    * @param string $wildcard
    *   The wildcard that represents multiple tags.

core/modules/ckeditor5/src/Plugin/CKEditor5PluginManager.php

@@ -179,8 +179,8 @@ public function getEnabledDefinitions(EditorInterface $editor): array {
       $restrictions = new HTMLRestrictions($this->getProvidedElements(array_keys($definitions), $editor, FALSE));
       if ($restrictions->getWildcardSubset()->isEmpty()) {
         // This is only reached if arbitrary HTML is not enabled. If wildcard
-        // tags (such as $block) are present, they need to be resolved via the
-        // wildcardHtmlSupport plugin.
+        // tags (such as $text-container) are present, they need to
+        // be resolved via the wildcardHtmlSupport plugin.
         // @see \Drupal\ckeditor5\Plugin\CKEditor5PluginManager::getCKEditor5PluginConfig()
         unset($definitions['ckeditor5_wildcardHtmlSupport']);
       }

core/modules/ckeditor5/tests/modules/ckeditor5_plugin_elements_test/ckeditor5_plugin_elements_test.ckeditor5.yml

-# cspell:ignore everyblock justblockquote
-ckeditor5_plugin_elements_test_blockquoteCombo:
+# cspell:ignore everytextcontainer justheading
+ckeditor5_plugin_elements_test_headingCombo:
   ckeditor5:
     plugins: []
   drupal:
     label: TEST — block quote combo
     elements:
-      - <blockquote data-justblockquote>
-      - <$block data-everyblock>
+      - <h1 data-justheading>
+      - <$text-container data-everytextcontainer>
 
 ckeditor5_plugin_elements_test_headingsWithOtherAttributes:
   ckeditor5:

core/modules/ckeditor5/tests/src/FunctionalJavascript/SourceEditingTest.php

@@ -200,15 +200,17 @@ public function providerAllowingExtraAttributes(): array {
         '<a class>',
       ],
 
-      // Edge case: $block wildcard with additional attribute.
-      '<$block data-llama>' => [
+      // Edge case: $text-container wildcard with additional
+      // attribute.
+      '<$text-container data-llama>' => [
         '<div class="llama" data-llama="🦙"><p data-llama="🦙">The <a href="https://example.com/pirate">pirate</a> is <a href="https://example.com/irate">irate</a>.</p></div>',
-        '<$block data-llama>',
+        '<$text-container data-llama>',
       ],
-      // Edge case: $block wildcard with stricter attribute constrain.
-      '<$block class="not-llama">' => [
+      // Edge case: $text-container wildcard with stricter attribute
+      // constrain.
+      '<$text-container class="not-llama">' => [
         '<div class="llama"><p>The <a href="https://example.com/pirate">pirate</a> is <a href="https://example.com/irate">irate</a>.</p></div>',
-        '<$block class="not-llama">',
+        '<$text-container class="not-llama">',
       ],
 
       // Edge case: wildcard attribute names:

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

@@ -15,7 +15,7 @@
 use Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag;
 use Symfony\Component\Yaml\Yaml;
 
-// cspell:ignore layercake everyblock justblockquote
+// cspell:ignore layercake everytextcontainer justheading
 
 /**
  * Tests different ways of enabling CKEditor 5 plugins.
@@ -1303,26 +1303,24 @@ public function providerTestProvidedElements(): array {
         ],
         'expected_readable_string' => '<p class="text-align-left text-align-center text-align-right text-align-justify"> <h2 class> <h3 class> <h4 class> <h5 class> <h6 class> <h1 class>',
       ],
-      'blockquote combo' => [
+      'heading text container combo' => [
         'plugins' => [
-          'ckeditor5_plugin_elements_test_blockquoteCombo',
+          'ckeditor5_plugin_elements_test_headingCombo',
           'ckeditor5_paragraph',
         ],
         'text_editor_settings' => [
-          'plugins' => [
-            'ckeditor5_heading' => Heading::DEFAULT_CONFIGURATION,
-          ],
+          'plugins' => [],
         ],
         'expected_elements' => [
           'p' => [
-            'data-everyblock' => TRUE,
+            'data-everytextcontainer' => TRUE,
           ],
-          'blockquote' => [
-            'data-justblockquote' => TRUE,
-            'data-everyblock' => TRUE,
+          'h1' => [
+            'data-justheading' => TRUE,
+            'data-everytextcontainer' => TRUE,
           ],
         ],
-        'expected_readable_string' => '<p data-everyblock> <blockquote data-justblockquote data-everyblock>',
+        'expected_readable_string' => '<p data-everytextcontainer> <h1 data-justheading data-everytextcontainer>',
       ],
       'headings plus headings with attributes' => [
         'plugins' => [

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

@@ -604,7 +604,7 @@ public function provider() {
         // Note that aligning left and right is being added, on top of what the
         // original format allowed: center and justify.
         // Note that aligning left/center/right/justify is possible on *all*
-        // allowed block-level HTML5 tags.
+        // allowed CKEditor 5 `$block` text container tags.
         // @todo When https://www.drupal.org/project/drupal/issues/3259367
         //   lands, none of the tags below should appear.
         '<h2 class="text-align-center text-align-justify">',
@@ -612,11 +612,7 @@ public function provider() {
         '<h4 class="text-align-center text-align-justify">',
         '<h5 class="text-align-center text-align-justify">',
         '<h6 class="text-align-center text-align-justify">',
-        '<dl class="text-align-center text-align-justify">',
-        '<dd class="text-align-center text-align-justify">',
-        '<blockquote class="text-align-center text-align-justify">',
-        '<ul class="text-align-center text-align-justify">',
-        '<ol class="text-align-center text-align-justify">',
+        '<li class="text-align-center text-align-justify">',
         $basic_html_test_case['expected_superset'],
       ]),
       'expected_fundamental_compatibility_violations' => $basic_html_test_case['expected_fundamental_compatibility_violations'],

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

@@ -844,7 +844,7 @@ public function providerPair(): array {
         ],
         'plugins' => [
           'ckeditor5_sourceEditing' => [
-            'allowed_tags' => ['<$block data-llama>'],
+            'allowed_tags' => ['<$text-container data-llama>'],
           ],
         ],
       ],

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

@@ -111,9 +111,9 @@ public function providerGhsConfiguration(): array {
         ],
         ['link', 'blockQuote'],
       ],
-      '<$block> minimal configuration' => [
+      '<$text-container> minimal configuration' => [
         '<p data-llama> <br>',
-        ['<$block data-llama>'],
+        ['<$text-container data-llama>'],
         [
           [
             'name' => 'p',
@@ -126,9 +126,9 @@ public function providerGhsConfiguration(): array {
           ],
         ],
       ],
-      '<$block> from multiple plugins' => [
+      '<$text-container> from multiple plugins' => [
         '<p data-llama class="text-align-left text-align-center text-align-right text-align-justify"> <br>',
-        ['<$block data-llama>'],
+        ['<$text-container data-llama>'],
         [
           [
             'name' => 'p',
@@ -147,9 +147,9 @@ public function providerGhsConfiguration(): array {
         ],
         ['alignment'],
       ],
-      '<$block> with attribute from multiple plugins' => [
+      '<$text-container> with attribute from multiple plugins' => [
         '<p data-llama class"> <br>',
-        ['<$block data-llama>', '<p class>'],
+        ['<$text-container data-llama>', '<p class>'],
         [
           [
             'name' => 'p',
@@ -172,9 +172,9 @@ public function providerGhsConfiguration(): array {
         ],
         ['alignment'],
       ],
-      '<$block> realistic configuration' => [
-        '<p data-llama> <br> <a href> <blockquote data-llama> <div data-llama> <mark> <abbr title>',
-        ['<$block data-llama>', '<div>', '<mark>', '<abbr title>'],
+      '<$text-container> realistic configuration' => [
+        '<p data-llama> <br> <a href> <blockquote> <div data-llama> <mark> <abbr title>',
+        ['<$text-container data-llama>', '<div>', '<mark>', '<abbr title>'],
         [
           [
             'name' => 'div',
@@ -209,15 +209,6 @@ public function providerGhsConfiguration(): array {
               ],
             ],
           ],
-          [
-            'name' => 'blockquote',
-            'attributes' => [
-              [
-                'key' => 'data-llama',
-                'value' => TRUE,
-              ],
-            ],
-          ],
         ],
         ['link', 'blockQuote'],
       ],