Issue #3260857 by Wim Leers, lauriii: Expand...

Issue #3260857 by Wim Leers, lauriii: Expand SourceEditingRedundantTagsConstraintValidator to also check attributes and attribute values

core/modules/ckeditor5/src/Plugin/Validation/Constraint/SourceEditingRedundantTagsConstraint.php

@@ -7,11 +7,11 @@
 use Symfony\Component\Validator\Constraint;
 
 /**
- * For disallowing Source Editing tags that are already supported by a plugin.
+ * For disallowing Source Editing elements already supported by a plugin.
  *
  * @Constraint(
  *   id = "SourceEditingRedundantTags",
- *   label = @Translation("Source editing should only use otherwise unavailable tags", context = "Validation"),
+ *   label = @Translation("Source editing should only use otherwise unavailable tags and attributes", context = "Validation"),
  * )
  *
  * @internal
@@ -19,17 +19,17 @@
 class SourceEditingRedundantTagsConstraint extends Constraint {
 
   /**
-   * When a Source Editing tag is added that an enabled plugin supports.
+   * When a Source Editing element is added that an enabled plugin supports.
    *
    * @var string
    */
-  public $enabledPluginsMessage = 'The following tag(s) are already supported by enabled plugins and should not be added to the Source Editing "Manually editable HTML tags" field: %overlapping_tags.';
+  public $enabledPluginsMessage = 'The following @element_type(s) are already supported by enabled plugins and should not be added to the Source Editing "Manually editable HTML tags" field: %overlapping_tags.';
 
   /**
-   * When a Source Editing tag is added that a disabled plugin supports.
+   * When a Source Editing element is added that a disabled plugin supports.
    *
    * @var string
    */
-  public $availablePluginsMessage = 'The following tag(s) are already supported by available plugins and should not be added to the Source Editing "Manually editable HTML tags" field. Instead, enable the following plugins to support these tags: %overlapping_tags.';
+  public $availablePluginsMessage = 'The following @element_type(s) are already supported by available plugins and should not be added to the Source Editing "Manually editable HTML tags" field. Instead, enable the following plugins to support these @element_types: %overlapping_tags.';
 
 }

core/modules/ckeditor5/src/Plugin/Validation/Constraint/SourceEditingRedundantTagsConstraintValidator.php

@@ -5,7 +5,9 @@
 namespace Drupal\ckeditor5\Plugin\Validation\Constraint;
 
 use Drupal\ckeditor5\HTMLRestrictions;
+use Drupal\ckeditor5\Plugin\CKEditor5PluginDefinition;
 use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
+use Drupal\Core\StringTranslation\StringTranslationTrait;
 use Symfony\Component\Validator\Constraint;
 use Symfony\Component\Validator\ConstraintValidator;
 use Symfony\Component\Validator\Exception\UnexpectedTypeException;
@@ -18,6 +20,7 @@
 class SourceEditingRedundantTagsConstraintValidator extends ConstraintValidator implements ContainerInjectionInterface {
 
   use PluginManagerDependentValidatorTrait;
+  use StringTranslationTrait;
   use TextEditorObjectDependentValidatorTrait;
 
   /**
@@ -37,11 +40,21 @@ public function validate($value, Constraint $constraint) {
     $text_editor = $this->createTextEditorObjectFromContext();
     $enabled_plugins = $this->pluginManager->getEnabledDefinitions($text_editor);
     $disabled_plugins = array_diff_key($this->pluginManager->getDefinitions(), $enabled_plugins);
+    // Only consider plugins that can be explicitly enabled by the user: plugins
+    // that have a toolbar item and do not have conditions. Those are the only
+    // plugins that are truly available for the site builder to enable without
+    // other consequences.
+    // In the future, we may choose to expand this, but it will require complex
+    // infrastructure to generate messages that explain which of the conditions
+    // are already fulfilled and which are not.
+    $disabled_plugins = array_filter($disabled_plugins, function (CKEditor5PluginDefinition $definition) {
+      return $definition->hasToolbarItems() && !$definition->hasConditions();
+    });
     unset($enabled_plugins['ckeditor5_sourceEditing']);
 
     // An array of tags enabled by every plugin other than Source Editing.
-    $enabled_plugin_tags = new HTMLRestrictions($this->pluginManager->getProvidedElements(array_keys($enabled_plugins)));
-    $disabled_plugin_tags = new HTMLRestrictions($this->pluginManager->getProvidedElements(array_keys($disabled_plugins)));
+    $enabled_plugin_tags = new HTMLRestrictions($this->pluginManager->getProvidedElements(array_keys($enabled_plugins), $text_editor, FALSE));
+    $disabled_plugin_tags = new HTMLRestrictions($this->pluginManager->getProvidedElements(array_keys($disabled_plugins), $text_editor, FALSE));
 
     // The single tag for which source editing is enabled, which we are checking
     // now.
@@ -57,32 +70,76 @@ public function validate($value, Constraint $constraint) {
     if (count($source_enabled_tags->getAllowedElements()) !== 1) {
       return;
     }
-    // This validation constraint currently only validates tags, not attributes;
-    // so if all attributes are allowed (TRUE) or some attributes are allowed
-    // (an array), return early. Only proceed when no attributes are allowed
-    // (FALSE).
-    // @todo Support attributes and attribute values in
-    //   https://www.drupal.org/project/drupal/issues/3260857
-    $tags = array_keys($source_enabled_tags->getAllowedElements());
-    if ($source_enabled_tags->getAllowedElements()[reset($tags)] !== FALSE) {
-      return;
-    }
 
     $enabled_plugin_overlap = $enabled_plugin_tags->intersect($source_enabled_tags);
-    $disabled_plugin_overlap = $disabled_plugin_tags->intersect($source_enabled_tags);
+    $disabled_plugin_overlap = $disabled_plugin_tags
+      // Merge the enabled plugin tags, to allow wildcards to be resolved.
+      ->merge($enabled_plugin_tags)
+      // Compute the overlap.
+      ->intersect($source_enabled_tags)
+      // Exclude the enabled plugin tags from the overlap; we merged these
+      // previously to be able to resolve wildcards.
+      ->diff($enabled_plugin_overlap);
     foreach ([$enabled_plugin_overlap, $disabled_plugin_overlap] as $overlap) {
       $checking_enabled = $overlap === $enabled_plugin_overlap;
       if (!$overlap->isEmpty()) {
         $plugins_to_check_against = $checking_enabled ? $enabled_plugins : $disabled_plugins;
-        $tags_plugin_report = $this->pluginsSupplyingTagsMessage($overlap, $plugins_to_check_against);
+        $tags_plugin_report = $this->pluginsSupplyingTagsMessage($overlap, $plugins_to_check_against, $enabled_plugin_tags);
         $message = $checking_enabled ? $constraint->enabledPluginsMessage : $constraint->availablePluginsMessage;
+
+        // Determine which element type is relevant for the violation message.
+        assert(count($overlap->getAllowedElements(FALSE)) === 1);
+        $overlap_tag = array_keys($overlap->getAllowedElements(FALSE))[0];
+        $element_type = self::tagHasAttributeRestrictions($overlap, $overlap_tag) && array_key_exists($overlap_tag, $enabled_plugin_tags->getAllowedElements())
+          ? $this->t('attribute')
+          : $this->t('tag');
+
+        // If the entirety (so not just the tag but also the attributes, and not
+        // just some of the attribute values, but all of them) of the HTML
+        // elements being configured to be edited via the Source Editing plugin
+        // is supported by a CKEditor 5 plugin, complain. But if some attribute
+        // or some attribute value is still not yet supported, do not generate a
+        // violation message.
+        // If there is overlap, but some attribute/attribute value is still not
+        // supported, exit this iteration without generating a violation
+        // message. Essentially: when assessing a particular value
+        // (for example `<foo bar baz>`), only CKEditor 5 plugins providing an
+        // exact match (`<foo bar baz>`) or a superset (`<foo bar baz qux>`) can
+        // trigger a violation, not subsets (`<foo>`).
+        if (!$source_enabled_tags->diff($overlap)->isEmpty()) {
+          continue;
+        }
+
+        // If we reach this, it means the entirety (so not just the tag but also
+        // the attributes, and not just some of the attribute values, but all of
+        // them) of the HTML elements being configured to be edited via the
+        // Source Editing plugin's 'allowed_tags' configuration is supported by
+        // a CKEditor 5 plugin. This earns a violation.
         $this->context->buildViolation($message)
+          ->setParameter('@element_type', $element_type)
           ->setParameter('%overlapping_tags', $tags_plugin_report)
           ->addViolation();
       }
     }
   }
 
+  /**
+   * Inspects whether the given tag has attribute restrictions.
+   *
+   * @param \Drupal\ckeditor5\HTMLRestrictions $r
+   *   A set of HTML restrictions to inspect.
+   * @param string $tag_name
+   *   The tag to check for attribute restrictions in $r.
+   *
+   * @return bool
+   *   TRUE if the given tag has attribute restrictions, FALSE otherwise.
+   */
+  private static function tagHasAttributeRestrictions(HTMLRestrictions $r, string $tag_name): bool {
+    $all_elements = $r->getAllowedElements(FALSE);
+    assert(isset($all_elements[$tag_name]));
+    return is_array($r->getAllowedElements(FALSE)[$tag_name]);
+  }
+
   /**
    * Creates a message listing plugins and the overlapping tags they provide.
    *
@@ -90,16 +147,34 @@ public function validate($value, Constraint $constraint) {
    *   An array of overlapping tags.
    * @param \Drupal\ckeditor5\Plugin\CKEditor5PluginDefinition[] $plugin_definitions
    *   An array of plugin definitions where overlap was found.
+   * @param \Drupal\ckeditor5\HTMLRestrictions $enabled_plugin_restrictions
+   *   The set of HTML restrictions for all already enabled CKEditor 5 plugins.
    *
    * @return string
    *   A list of plugins that provide the overlapping tags.
    */
-  private function pluginsSupplyingTagsMessage(HTMLRestrictions $overlap, array $plugin_definitions): string {
+  private function pluginsSupplyingTagsMessage(HTMLRestrictions $overlap, array $plugin_definitions, HTMLRestrictions $enabled_plugin_restrictions): string {
     $message_array = [];
     $message_string = '';
     foreach ($plugin_definitions as $definition) {
       if ($definition->hasElements()) {
         $plugin_capabilities = HTMLRestrictions::fromString(implode(' ', $definition->getElements()));
+
+        // If this plugin supports wildcards, resolve them.
+        if (!$plugin_capabilities->getWildcardSubset()->isEmpty()) {
+          $plugin_capabilities = $plugin_capabilities
+            // Resolve wildcards.
+            ->merge($enabled_plugin_restrictions)
+            ->diff($enabled_plugin_restrictions);
+        }
+
+        // Skip plugins that provide a subset, only mention the plugin that
+        // actually provides the overlap.
+        // For example: avoid listing the image alignment/captioning plugins
+        // when matching `<img src>`; only lists the main image plugin.
+        if (!$overlap->diff($plugin_capabilities)->isEmpty()) {
+          continue;
+        }
         foreach ($plugin_capabilities->intersect($overlap)->toCKEditor5ElementsArray() as $element) {
           $message_array[(string) $definition->label()][] = $element;
         }

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

@@ -334,11 +334,13 @@ public function test(string $format_id, array $filters_to_drop, array $expected_
     }
 
     // The resulting Editor config entity should be valid.
-    $typed_config = $this->typedConfig->createFromNameAndData(
-      $updated_text_editor->getConfigDependencyName(),
-      $updated_text_editor->toArray(),
-    );
-    $this->assertCount(0, $typed_config->validate());
+    $violations = $this->validatePairToViolationsArray($updated_text_editor, $text_format, FALSE);
+    // At this point, the fundamental compatibility errors do not matter, they
+    // have been checked above; whatever remains is expected.
+    if (isset($violations[''])) {
+      unset($violations['']);
+    }
+    $this->assertSame([], $violations);
 
     // If the text format has HTML restrictions, ensure that a strict superset
     // is allowed after switching to CKEditor 5.

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

@@ -647,6 +647,7 @@ public function providerPair(): array {
             'heading',
             'bold',
             'italic',
+            'link',
             'sourceEditing',
             'textPartLanguage',
           ],
@@ -666,8 +667,25 @@ public function providerPair(): array {
           ],
           'ckeditor5_sourceEditing' => [
             'allowed_tags' => [
+              // Tag-only; supported by enabled plugin.
               '<strong>',
+              // Tag-only; supported by disabled plugin.
               '<table>',
+              // Tag-only; supported by no plugin.
+              '<exotic>',
+              // Tag + attributes; all supported by enabled plugin.
+              '<span lang>',
+              // Tag + attributes; all supported by an ineligible disabled
+              // plugin (has no toolbar item, has conditions).
+              '<img src>',
+              // Tag + attributes; all supported by disabled plugin.
+              '<code class="language-*">',
+              // Tag + attributes; tag already supported by enabled plugin,
+              // attributes supported by disabled plugin
+              '<h2 class="text-align-center">',
+              // Tag + attributes; tag already supported by enabled plugin,
+              // attribute not supported by no plugin.
+              '<a hreflang>',
             ],
           ],
         ],
@@ -679,6 +697,9 @@ public function providerPair(): array {
       'violations' => [
         'settings.plugins.ckeditor5_sourceEditing.allowed_tags.0' => 'The following tag(s) are already supported by enabled plugins and should not be added to the Source Editing "Manually editable HTML tags" field: <em class="placeholder">Bold (&lt;strong&gt;)</em>.',
         'settings.plugins.ckeditor5_sourceEditing.allowed_tags.1' => 'The following tag(s) are already supported by available plugins and should not be added to the Source Editing "Manually editable HTML tags" field. Instead, enable the following plugins to support these tags: <em class="placeholder">Table (&lt;table&gt;)</em>.',
+        'settings.plugins.ckeditor5_sourceEditing.allowed_tags.3' => 'The following attribute(s) are already supported by enabled plugins and should not be added to the Source Editing "Manually editable HTML tags" field: <em class="placeholder">Language (&lt;span lang&gt;)</em>.',
+        'settings.plugins.ckeditor5_sourceEditing.allowed_tags.5' => 'The following tag(s) are already supported by available plugins and should not be added to the Source Editing "Manually editable HTML tags" field. Instead, enable the following plugins to support these tags: <em class="placeholder">Code Block (&lt;code class=&quot;language-*&quot;&gt;)</em>.',
+        'settings.plugins.ckeditor5_sourceEditing.allowed_tags.6' => 'The following attribute(s) are already supported by available plugins and should not be added to the Source Editing "Manually editable HTML tags" field. Instead, enable the following plugins to support these attributes: <em class="placeholder">Alignment (&lt;h2 class=&quot;text-align-center&quot;&gt;), Align center (&lt;h2 class=&quot;text-align-center&quot;&gt;)</em>.',
       ],
     ];
     $data['INVALID some invalid Source Editable tags provided by plugin and another available in a not enabled plugin'] = [