Issue #3227822 by lauriii, Wim Leers: [GHS] Ensure GHS works with our custom plugins, to allow adding additional attributes

core/modules/ckeditor5/js/ckeditor5_plugins/drupalMedia/src/drupalmediageneralhtmlsupport.js

@@ -1,5 +1,5 @@
 /* eslint-disable import/no-extraneous-dependencies */
-// cSpell:words conversionutils datafilter
+// cSpell:words conversionutils datafilter eventinfo downcastdispatcher generalhtmlsupport
 import { Plugin } from 'ckeditor5/src/core';
 import { setViewAttributes } from '@ckeditor/ckeditor5-html-support/src/conversionutils';
 
@@ -38,6 +38,8 @@ function viewToModelDrupalMediaAttributeConverter(dataFilter) {
         const viewMediaElement = data.viewItem;
         const viewContainerElement = viewMediaElement.parent;
 
+        preserveElementAttributes(viewMediaElement, 'htmlAttributes');
+
         if (viewContainerElement.is('element', 'a')) {
           preserveLinkAttributes(viewContainerElement);
         }
@@ -70,6 +72,26 @@ function getDescendantElement(writer, containerElement, elementName) {
   }
 }
 
+/**
+ * Model to view converter for the Drupal Media wrapper attributes.
+ *
+ * @param {module:utils/eventinfo~EventInfo} evt
+ *   An object containing information about the fired event.
+ * @param {Object} data
+ *   Additional information about the change.
+ * @param {module:engine/conversion/downcastdispatcher~DowncastDispatcher} conversionApi
+ *   Conversion interface to be used by the callback.
+ */
+function modelToDataAttributeConverter(evt, data, conversionApi) {
+  if (!conversionApi.consumable.consume(data.item, evt.name)) {
+    return;
+  }
+
+  const viewElement = conversionApi.mapper.toViewElement(data.item);
+
+  setViewAttributes(conversionApi.writer, data.attributeNewValue, viewElement);
+}
+
 /**
  * Model to editing view attribute converter.
  *
@@ -77,7 +99,7 @@ function getDescendantElement(writer, containerElement, elementName) {
  *   A function that adds an event listener to downcastDispatcher.
  */
 function modelToEditingViewAttributeConverter() {
-  return (dispatcher) =>
+  return (dispatcher) => {
     dispatcher.on(
       'attribute:linkHref:drupalMedia',
       (evt, data, conversionApi) => {
@@ -105,6 +127,16 @@ function modelToEditingViewAttributeConverter() {
       },
       { priority: 'low' },
     );
+
+    // Render arbitrary attributes on the CKEditor 5 widget wrapper until
+    // arbitrary attributes are included as part of the server rendered preview.
+    // @see https://www.drupal.org/project/drupal/issues/3231337
+    dispatcher.on(
+      'attribute:htmlAttributes:drupalMedia',
+      modelToDataAttributeConverter,
+      { priority: 'low' },
+    );
+  };
 }
 
 /**
@@ -114,7 +146,7 @@ function modelToEditingViewAttributeConverter() {
  *   function that adds an event listener to downcastDispatcher.
  */
 function modelToDataViewAttributeConverter() {
-  return (dispatcher) =>
+  return (dispatcher) => {
     dispatcher.on(
       'attribute:linkHref:drupalMedia',
       (evt, data, conversionApi) => {
@@ -137,6 +169,13 @@ function modelToDataViewAttributeConverter() {
       },
       { priority: 'low' },
     );
+
+    dispatcher.on(
+      'attribute:htmlAttributes:drupalMedia',
+      modelToDataAttributeConverter,
+      { priority: 'low' },
+    );
+  };
 }
 
 /**
@@ -148,29 +187,58 @@ export default class DrupalMediaGeneralHtmlSupport extends Plugin {
   /**
    * @inheritdoc
    */
-  init() {
+  constructor(editor) {
-    const { editor } = this;
+    super(editor);
 
     // This plugin is only needed if General HTML Support plugin is loaded.
     if (!editor.plugins.has('GeneralHtmlSupport')) {
       return;
     }
+    // This plugin works only if `DataFilter` and `DataSchema` plugins are
+    // loaded. These plugins are dependencies of `GeneralHtmlSupport` meaning
+    // that these should be available always when `GeneralHtmlSupport` is
+    // enabled.
+    if (
+      !editor.plugins.has('DataFilter') ||
+      !editor.plugins.has('DataSchema')
+    ) {
+      console.error(
+        'DataFilter and DataSchema plugins are required for Drupal Media to integrate with General HTML Support plugin.',
+      );
+    }
 
     const { schema } = editor.model;
     const { conversion } = editor;
-    const dataFilter = editor.plugins.get('DataFilter');
+    const dataFilter = this.editor.plugins.get('DataFilter');
+    const dataSchema = this.editor.plugins.get('DataSchema');
 
-    schema.extend('drupalMedia', {
+    // This needs to be initialized in ::constructor() to ensure this runs
-      allowAttributes: ['htmlLinkAttributes'],
+    // before the General HTML Support has been initialized.
+    // @see module:html-support/generalhtmlsupport~GeneralHtmlSupport
+    dataSchema.registerBlockElement({
+      model: 'drupalMedia',
+      view: 'drupal-media',
     });
 
-    conversion
+    dataFilter.on('register:drupal-media', (evt, definition) => {
-      .for('upcast')
+      if (definition.model !== 'drupalMedia') {
-      .add(viewToModelDrupalMediaAttributeConverter(dataFilter));
+        return;
-    conversion
+      }
-      .for('editingDowncast')
+
-      .add(modelToEditingViewAttributeConverter());
+      schema.extend('drupalMedia', {
-    conversion.for('dataDowncast').add(modelToDataViewAttributeConverter());
+        allowAttributes: ['htmlLinkAttributes', 'htmlAttributes'],
+      });
+
+      conversion
+        .for('upcast')
+        .add(viewToModelDrupalMediaAttributeConverter(dataFilter));
+      conversion
+        .for('editingDowncast')
+        .add(modelToEditingViewAttributeConverter());
+      conversion.for('dataDowncast').add(modelToDataViewAttributeConverter());
+
+      evt.stop();
+    });
   }
 
   /**

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

@@ -127,7 +127,7 @@ class CKEditor5Test extends CKEditor5TestBase {
     $image_uuid = $uploaded_image->uuid();
     $image_url = $this->container->get('file_url_generator')->generateString($uploaded_image->getFileUri());
     $this->drupalGet('node/1');
-    $assert_session->elementExists('xpath', sprintf('//img[@alt="</em> Kittens & llamas are cute" and @data-entity-uuid="%s" and @data-entity-type="file"]', $image_uuid));
+    $this->assertNotEmpty($assert_session->waitForElement('xpath', sprintf('//img[@alt="</em> Kittens & llamas are cute" and @data-entity-uuid="%s" and @data-entity-type="file"]', $image_uuid)));
 
     // Drupal CKEditor 5 integrations overrides the CKEditor 5 HTML writer to
     // escape ampersand characters (&) and the angle brackets (< and >). This is

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

@@ -0,0 +1,164 @@
+<?php
+
+namespace Drupal\Tests\ckeditor5\FunctionalJavascript;
+
+use Drupal\ckeditor5\Plugin\Editor\CKEditor5;
+use Drupal\editor\Entity\Editor;
+use Drupal\filter\Entity\FilterFormat;
+use Drupal\FunctionalJavascriptTests\WebDriverTestBase;
+use Drupal\Tests\ckeditor5\Traits\CKEditor5TestTrait;
+use Symfony\Component\Validator\ConstraintViolation;
+
+/**
+ * Tests emphasis in CKEditor 5.
+ *
+ * CKEditor's use of <i> is converted to <em> in Drupal, so additional coverage
+ * is provided here to verify successful conversion.
+ *
+ * @group ckeditor5
+ * @internal
+ */
+class EmphasisTest extends WebDriverTestBase {
+  use CKEditor5TestTrait;
+
+  /**
+   * The user to use during testing.
+   *
+   * @var \Drupal\user\UserInterface
+   */
+  protected $adminUser;
+
+  /**
+   * A host entity with a body field to use the <em> tag in.
+   *
+   * @var \Drupal\node\NodeInterface
+   */
+  protected $host;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = [
+    'ckeditor5',
+    'node',
+    'text',
+  ];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $defaultTheme = 'stark';
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp(): void {
+    parent::setUp();
+
+    FilterFormat::create([
+      'format' => 'test_format',
+      'name' => 'Test format',
+      'filters' => [
+        'filter_html' => [
+          'status' => TRUE,
+          'settings' => [
+            'allowed_html' => '<p> <br> <em>',
+          ],
+        ],
+      ],
+    ])->save();
+    Editor::create([
+      'editor' => 'ckeditor5',
+      'format' => 'test_format',
+      'settings' => [
+        'toolbar' => [
+          'items' => [
+            'italic',
+            'sourceEditing',
+          ],
+        ],
+        'plugins' => [
+          'ckeditor5_sourceEditing' => [
+            'allowed_tags' => [],
+          ],
+        ],
+      ],
+    ])->save();
+    $this->assertSame([], array_map(
+      function (ConstraintViolation $v) {
+        return (string) $v->getMessage();
+      },
+      iterator_to_array(CKEditor5::validatePair(
+        Editor::load('test_format'),
+        FilterFormat::load('test_format')
+      ))
+    ));
+    $this->adminUser = $this->drupalCreateUser([
+      'use text format test_format',
+      'bypass node access',
+    ]);
+
+    $this->drupalCreateContentType(['type' => 'blog']);
+    $this->host = $this->createNode([
+      'type' => 'blog',
+      'title' => 'Animals with strange names',
+      'body' => [
+        'value' => '<p>This is a <em>test!</em></p>',
+        'format' => 'test_format',
+      ],
+    ]);
+    $this->host->save();
+
+    $this->drupalLogin($this->adminUser);
+  }
+
+  /**
+   * Ensures that CKEditor italic model is converted to em.
+   */
+  public function testEmphasis() {
+    $page = $this->getSession()->getPage();
+    $assert_session = $this->assertSession();
+
+    $this->drupalGet($this->host->toUrl('edit-form'));
+    $this->waitForEditor();
+
+    $emphasis_element = $assert_session->waitForElementVisible('css', '.ck-content p em');
+    $this->assertEquals('test!', $emphasis_element->getText());
+
+    $xpath = new \DOMXPath($this->getEditorDataAsDom());
+    $emphasis_source = $xpath->query('//p/em');
+    $this->assertNotEmpty($emphasis_source);
+    $this->assertEquals('test!', $emphasis_source[0]->textContent);
+    $page->pressButton('Save');
+
+    $assert_session->responseContains('<p>This is a <em>test!</em></p>');
+  }
+
+  /**
+   * Tests that arbitrary attributes are allowed via GHS.
+   */
+  public function testEmphasisArbitraryHtml() {
+    $assert_session = $this->assertSession();
+    $editor = Editor::load('test_format');
+    $settings = $editor->getSettings();
+
+    // Allow the data-foo attribute in img via GHS.
+    $settings['plugins']['ckeditor5_sourceEditing']['allowed_tags'] = ['<em data-foo>'];
+    $editor->setSettings($settings);
+    $editor->save();
+
+    // Add data-foo use to an existing em tag.
+    $original_value = $this->host->body->value;
+    $this->host->body->value = str_replace('<em>', '<em data-foo="bar">', $original_value);
+    $this->host->save();
+    $this->drupalGet($this->host->toUrl('edit-form'));
+    $this->waitForEditor();
+
+    $emphasis_element = $assert_session->waitForElementVisible('css', '.ck-content p em');
+    $this->assertEquals('bar', $emphasis_element->getAttribute('data-foo'));
+
+    $xpath = new \DOMXPath($this->getEditorDataAsDom());
+    $this->assertNotEmpty($xpath->query('//em[@data-foo="bar"]'));
+  }
+
+}

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

@@ -140,6 +140,47 @@ class ImageTest extends CKEditor5TestBase {
     $this->drupalLogin($this->adminUser);
   }
 
+  /**
+   * Tests that arbitrary attributes are allowed via GHS.
+   *
+   * @dataProvider providerLinkability
+   */
+  public function testImageArbitraryHtml(string $image_type, bool $unrestricted) {
+    $editor = Editor::load('test_format');
+    $settings = $editor->getSettings();
+
+    // Allow the data-foo attribute in img via GHS.
+    $settings['plugins']['ckeditor5_sourceEditing']['allowed_tags'] = ['<img data-foo>'];
+    $editor->setSettings($settings);
+    $editor->save();
+
+    // Disable filter_html.
+    if ($unrestricted) {
+      FilterFormat::load('test_format')
+        ->setFilterConfig('filter_html', ['status' => FALSE])
+        ->save();
+    }
+
+    // Make the test content have either a block image or an inline image.
+    $img_tag = '<img data-foo="bar" alt="drupalimage test image" data-entity-type="file" data-entity-uuid="' . $this->file->uuid() . '" src="' . $this->file->createFileUrl() . '" />';
+    $this->host->body->value .= $image_type === 'block'
+      ? $img_tag
+      : "<p>$img_tag</p>";
+    $this->host->save();
+
+    $expected_widget_selector = $image_type === 'block' ? 'image img' : 'image-inline';
+
+    $this->drupalGet($this->host->toUrl('edit-form'));
+    $this->waitForEditor();
+
+    $drupalimage = $this->assertSession()->waitForElementVisible('css', ".ck-content .ck-widget.$expected_widget_selector");
+    $this->assertNotEmpty($drupalimage);
+    $this->assertEquals('bar', $drupalimage->getAttribute('data-foo'));
+
+    $xpath = new \DOMXPath($this->getEditorDataAsDom());
+    $this->assertNotEmpty($xpath->query('//img[@data-foo="bar"]'));
+  }
+
   /**
    * Tests linkability of the image CKEditor widget.
    *

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

@@ -188,6 +188,34 @@ class MediaTest extends WebDriverTestBase {
     $assert_session->elementExists('css', '.ck-widget.drupal-media');
   }
 
+  /**
+   * Tests that arbitrary attributes are allowed via GHS.
+   */
+  public function testMediaArbitraryHtml() {
+    $assert_session = $this->assertSession();
+
+    $editor = Editor::load('test_format');
+    $settings = $editor->getSettings();
+
+    // Allow the data-foo attribute in drupal-media via GHS.
+    $settings['plugins']['ckeditor5_sourceEditing']['allowed_tags'] = ['<drupal-media data-foo>'];
+    $editor->setSettings($settings);
+    $editor->save();
+
+    // Add data-foo use to an existing drupal-media tag.
+    $original_value = $this->host->body->value;
+    $this->host->body->value = str_replace('drupal-media', 'drupal-media data-foo="bar" ', $original_value);
+    $this->host->save();
+    $this->drupalGet($this->host->toUrl('edit-form'));
+
+    // Confirm data-foo is present in the upcasted drupal-media.
+    $upcasted_media = $assert_session->waitForElementVisible('css', '.ck-widget.drupal-media');
+    $this->assertEquals('bar', $upcasted_media->getAttribute('data-foo'));
+
+    // Confirm data-foo is not stripped from source.
+    $this->assertSourceAttributeSame('data-foo', 'bar');
+  }
+
   /**
    * Tests that failed media embed preview requests inform the end user.
    */