Verified Commit bcf5a915 authored by Dave Long's avatar Dave Long
Browse files

Issue #3396628 by Wim Leers, smustgrave, lauriii, xjm, catch: Fix <ol start> →... 

Issue #3396628 by Wim Leers, smustgrave, lauriii, xjm, catch: Fix <ol start> →  native CKEditor 5 functionality and fix bug in SourceEditingRedundantTagsConstraintValidator that allowed it to slip by
parent 156533fe

core/modules/ckeditor5/ckeditor5.module

+34 −0
Original line number Diff line number Diff line
@@ -663,6 +663,40 @@ function ckeditor5_editor_presave(EditorInterface $editor) { 
      ];

    }



    // @see ckeditor5_post_update_list_start_reversed()

    if (in_array('numberedList', $settings['toolbar']['items'], TRUE) && array_key_exists('ckeditor5_sourceEditing', $settings['plugins'])) {

      $source_edited = HTMLRestrictions::fromString(implode(' ', $settings['plugins']['ckeditor5_sourceEditing']['allowed_tags']));

      $format_restrictions = HTMLRestrictions::fromTextFormat($editor->getFilterFormat());



      // If <ol start> is not allowed through Source Editing (the only way it

      // could possibly be supported until now), and it is not an unrestricted

      // text format (such as "Full HTML"), then set the new "startIndex"

      // setting for the List plugin to false.

      // Except … that this update path was added too late, and many sites have

      // in the meantime edited their text editor configuration through the UI,

      // in which case they may already have set it. If that is the case: do not

      // override it.

      $ol_start = HTMLRestrictions::fromString('<ol start>');

      if (!array_key_exists('ckeditor5_list', $settings['plugins']) || !array_key_exists('startIndex', $settings['plugins']['ckeditor5_list']['properties'])) {

        $settings['plugins']['ckeditor5_list']['properties']['startIndex'] = $ol_start->diff($source_edited)

          ->allowsNothing() || $format_restrictions->isUnrestricted();

      }

      // Same for <ol reversed> and "reversed".

      $ol_reversed = HTMLRestrictions::fromString('<ol reversed>');

      if (!array_key_exists('ckeditor5_list', $settings['plugins']) || !array_key_exists('reversed', $settings['plugins']['ckeditor5_list']['properties'])) {

        $settings['plugins']['ckeditor5_list']['properties']['reversed'] = $ol_reversed->diff($source_edited)

          ->allowsNothing() || $format_restrictions->isUnrestricted();

      }

      // Match the sort order in ListPlugin::defaultConfiguration().

      ksort($settings['plugins']['ckeditor5_list']['properties']);



      // Update the Source Editing configuration too.

      $settings['plugins']['ckeditor5_sourceEditing']['allowed_tags'] = $source_edited

        ->diff($ol_start)

        ->diff($ol_reversed)

        ->toCKEditor5ElementsArray();

    }



    $editor->setSettings($settings);

  }

}

core/modules/ckeditor5/ckeditor5.post_update.php

+19 −0
Original line number Diff line number Diff line
@@ -119,7 +119,26 @@ function ckeditor5_post_update_list_multiblock(&$sandbox = []) { 
      return FALSE;

    }

    $settings = $editor->getSettings();



    // @see ckeditor5_editor_presave()

    return array_key_exists('ckeditor5_list', $settings['plugins']);

  });

}



/**

 * Updates Text Editors using CKEditor 5 to native List "start" functionality.

 */

function ckeditor5_post_update_list_start_reversed(&$sandbox = []) {

  $config_entity_updater = \Drupal::classResolver(ConfigEntityUpdater::class);

  $config_entity_updater->update($sandbox, 'editor', function (Editor $editor): bool {

    // Only try to update editors using CKEditor 5.

    if ($editor->getEditor() !== 'ckeditor5') {

      return FALSE;

    }

    $settings = $editor->getSettings();



    // @see ckeditor5_editor_presave()

    return in_array('numberedList', $settings['toolbar']['items'], TRUE)

      && array_key_exists('ckeditor5_sourceEditing', $settings['plugins']);

  });

}

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

+7 −0
Original line number Diff line number Diff line
@@ -25,6 +25,13 @@ class SourceEditingRedundantTagsConstraint extends Constraint { 
   */

  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 element is added that an enabled plugin optionally supports.

   *

   * @var string

   */

  public $enabledPluginsOptionalMessage = 'The following @element_type(s) can optionally be supported by enabled plugins and should not be added to the Source Editing "Manually editable HTML tags" field: %overlapping_tags.';



  /**

   * When a Source Editing element is added that a disabled plugin supports.

   *

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

+24 −16
Original line number Diff line number Diff line
@@ -45,6 +45,8 @@ public function validate($value, Constraint $constraint) { 


    // An array of tags enabled by every plugin other than Source Editing.

    $enabled_plugin_elements = new HTMLRestrictions($this->pluginManager->getProvidedElements(array_keys($other_enabled_plugins), $text_editor, FALSE));

    $enabled_plugin_elements_optional = (new HTMLRestrictions($this->pluginManager->getProvidedElements(array_keys($other_enabled_plugins))))

      ->diff($enabled_plugin_elements);

    $disabled_plugin_elements = new HTMLRestrictions($this->pluginManager->getProvidedElements(array_keys($enableable_disabled_plugins), $text_editor, FALSE));

    $enabled_plugin_plain_tags = new HTMLRestrictions($this->pluginManager->getProvidedElements(array_keys($other_enabled_plugins), $text_editor, FALSE, TRUE));

    $disabled_plugin_plain_tags = new HTMLRestrictions($this->pluginManager->getProvidedElements(array_keys($enableable_disabled_plugins), $text_editor, FALSE, TRUE));
@@ -65,6 +67,7 @@ public function validate($value, Constraint $constraint) { 
    }



    $enabled_plugin_overlap = $enabled_plugin_elements->intersect($source_enabled_element);

    $enabled_plugin_optional_overlap = $enabled_plugin_elements_optional->intersect($source_enabled_element);

    $disabled_plugin_overlap = $disabled_plugin_elements

      // Merge the enabled plugins' elements, to allow wildcards to be resolved.

      ->merge($enabled_plugin_elements)
@@ -73,34 +76,39 @@ public function validate($value, Constraint $constraint) { 
      // 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;

    foreach ([$enabled_plugin_overlap, $enabled_plugin_optional_overlap, $disabled_plugin_overlap] as $overlap) {

      $checking_enabled = $overlap === $enabled_plugin_overlap || $overlap === $enabled_plugin_optional_overlap;

      if (!$overlap->allowsNothing()) {

        $plugins_to_check_against = $checking_enabled ? $other_enabled_plugins : $enableable_disabled_plugins;

        $plain_tags_to_check_against = $checking_enabled ? $enabled_plugin_plain_tags : $disabled_plugin_plain_tags;

        $tags_plugin_report = $this->pluginsSupplyingTagsMessage($overlap, $plugins_to_check_against, $enabled_plugin_elements);

        $message = $checking_enabled ? $constraint->enabledPluginsMessage : $constraint->availablePluginsMessage;

        $message = match($overlap) {

          $enabled_plugin_overlap => $constraint->enabledPluginsMessage,

          $enabled_plugin_optional_overlap => $constraint->enabledPluginsOptionalMessage,

          $disabled_plugin_overlap => $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];

        $is_attr_overlap = self::tagHasAttributeRestrictions($overlap, $overlap_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 bar>`).

        if ($is_attr_overlap && !$source_enabled_element->diff($overlap)->allowsNothing()) {

        // If one or more attributes (and all of the allowed attribute values)

        // of the HTML elements being configured to be edited via the Source

        // Editing plugin is supported by a CKEditor 5 plugin, complain. But if

        // an attribute overlap is detected due to a wildcard attribute, then do

        // not generate a violation message.

        // For example:

        // - value `<ol start foo>` triggers a violation because `<ol start>` is

        //   supported by the `ckeditor5_list` plugin

        // - value `<img data-*>` does NOT trigger a violation because only

        //   concrete `data-`-attributes are supported by the

        //   `ckeditor5_imageUpload`, `ckeditor5_imageCaption` and

        //   `ckeditor5_imageAlign` plugins

        if ($is_attr_overlap && $source_enabled_element->diff($overlap)->getAllowedElements(FALSE) == $source_enabled_element->getAllowedElements(FALSE)) {

          continue;

        }



        // If there is overlap, but the plain tag is not supported in the

        // overlap, exit this iteration without generating a violation message.

        // Essentially when assessing a particular value (for example `<span>`),

core/modules/ckeditor5/tests/fixtures/update/ckeditor5-3396628.php

0 → 100644
+51 −0
Original line number Diff line number Diff line 
<?php



/**

 * @file

 * Test fixture.

 */



use Drupal\Core\Database\Database;

use Drupal\Core\Serialization\Yaml;



$connection = Database::getConnection();



$format_list_ol_start = Yaml::decode(file_get_contents(__DIR__ . '/filter.format.test_format_list_ol_start.yml'));

$format_list_ol_start_post_3261599 = Yaml::decode(file_get_contents(__DIR__ . '/filter.format.test_format_list_ol_start_post_3261599.yml'));

$connection->insert('config')

  ->fields([

    'collection',

    'name',

    'data',

  ])

  ->values([

    'collection' => '',

    'name' => 'filter.format.test_format_list_ol_start',

    'data' => serialize($format_list_ol_start),

  ])

  ->values([

    'collection' => '',

    'name' => 'filter.format.test_format_list_ol_start_post_3261599',

    'data' => serialize($format_list_ol_start_post_3261599),

  ])

  ->execute();



$editor_list_ol_start = Yaml::decode(file_get_contents(__DIR__ . '/editor.editor.test_format_list_ol_start.yml'));

$editor_list_ol_start_post_3261599 = Yaml::decode(file_get_contents(__DIR__ . '/editor.editor.test_format_list_ol_start_post_3261599.yml'));

$connection->insert('config')

  ->fields([

    'collection',

    'name',

    'data',

  ])

  ->values([

    'collection' => '',

    'name' => 'editor.editor.test_format_list_ol_start',

    'data' => serialize($editor_list_ol_start),

  ])

  ->values([

    'collection' => '',

    'name' => 'editor.editor.test_format_list_ol_start_post_3261599',

    'data' => serialize($editor_list_ol_start_post_3261599),

  ])

  ->execute();