Verified Commit e8ed42cc authored by Alex Pott's avatar Alex Pott
Browse files

Issue #3273312 by Wim Leers, Dom., ifrik, mpp, seanB, lauriii: Upgrading from... 

Issue #3273312 by Wim Leers, Dom., ifrik, mpp, seanB, lauriii: Upgrading from CKEditor 4 for a text format that has FilterInterface::TYPE_MARKUP_LANGUAGE filters enabled
parent 28bdfa26

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

+12 −0
Original line number Diff line number Diff line
@@ -8,6 +8,8 @@ 
use Drupal\Core\DependencyInjection\ContainerInjectionInterface;

use Drupal\editor\EditorInterface;

use Drupal\filter\FilterFormatInterface;

use Drupal\filter\Plugin\Filter\FilterAutoP;

use Drupal\filter\Plugin\Filter\FilterUrl;

use Drupal\filter\Plugin\FilterInterface;

use Symfony\Component\Validator\Constraint;

use Symfony\Component\Validator\ConstraintValidator;
@@ -74,6 +76,13 @@ public function validate($toolbar_item, Constraint $constraint) { 
  /**

   * Checks no TYPE_MARKUP_LANGUAGE filters are present.

   *

   * Two TYPE_MARKUP_LANGUAGE filters are exempted:

   * - filter_autop: pointless but harmless to have enabled

   * - filter_url: not recommended but also harmless to have enabled

   *

   * These two commonly enabled filters with a long history in Drupal are

   * considered to be acceptable to have enabled.

   *

   * @param \Drupal\filter\FilterFormatInterface $text_format

   *   The text format to validate.

   * @param \Drupal\ckeditor5\Plugin\Validation\Constraint\FundamentalCompatibilityConstraint $constraint
@@ -85,6 +94,9 @@ private function checkNoMarkupFilters(FilterFormatInterface $text_format, Fundam 
      FilterInterface::TYPE_MARKUP_LANGUAGE

    );

    foreach ($markup_filters as $markup_filter) {

      if ($markup_filter instanceof FilterAutoP || $markup_filter instanceof FilterUrl) {

        continue;

      }

      $this->context->buildViolation($constraint->noMarkupFiltersMessage)

        ->setParameter('%filter_label', (string) $markup_filter->getLabel())

        ->setParameter('%filter_plugin_id', $markup_filter->getPluginId())

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

+26 −15
Original line number Diff line number Diff line
@@ -68,11 +68,11 @@ public function testSettingsOnlyFireAjaxWithCkeditor5() { 
    $assert_session->waitForText('Machine name');

    $page->checkField('roles[authenticated]');



    // Enable a filter that is incompatible with CKEditor 5, so validation is

    // triggered when attempting to switch.

    // Enable a filter that is incompatible with CKEditor 5 if not configured

    // correctly, so validation is triggered when attempting to switch.

    $number_ajax_instances_before = $this->getSession()->evaluateScript('Drupal.ajax.instances.length');

    $this->assertTrue($page->hasUncheckedField('filters[filter_autop][status]'));

    $page->checkField('filters[filter_autop][status]');

    $this->assertTrue($page->hasUncheckedField('filters[filter_html][status]'));

    $page->checkField('filters[filter_html][status]');

    $this->assertEmpty($assert_session->waitForElement('css', '.ajax-progress-throbber'));

    $assert_session->assertWaitOnAjaxRequest();

    $number_ajax_instances_after = $this->getSession()->evaluateScript('Drupal.ajax.instances.length');
@@ -83,16 +83,16 @@ public function testSettingsOnlyFireAjaxWithCkeditor5() { 


    // The presence of this validation error message confirms the AJAX callback

    // was invoked.

    $assert_session->pageTextContains('CKEditor 5 only works with HTML-based text formats');

    $assert_session->pageTextContains('CKEditor 5 needs at least the <p> and <br> tags to be allowed to be able to function. They are not allowed by the "Limit allowed HTML tags and correct faulty HTML" (filter_html) filter.');



    // Disable the incompatible filter. This should trigger another AJAX rebuild

    // which will include the removal of the validation error as the issue has

    // been corrected.

    $this->assertTrue($page->hasCheckedField('filters[filter_autop][status]'));

    $page->uncheckField('filters[filter_autop][status]');

    $this->assertTrue($page->hasCheckedField('filters[filter_html][status]'));

    $page->uncheckField('filters[filter_html][status]');

    $this->assertNotEmpty($assert_session->waitForElement('css', '.ajax-progress-throbber'));

    $assert_session->assertWaitOnAjaxRequest();

    $assert_session->pageTextNotContains('CKEditor 5 only works with HTML-based text formats');

    $assert_session->pageTextNotContains('CKEditor 5 needs at least the <p> and <br> tags to be allowed to be able to function. They are not allowed by the "Limit allowed HTML tags and correct faulty HTML" (filter_html) filter.');

  }



  /**
@@ -149,25 +149,36 @@ public function testMessagesDoNotAccumulate(): void { 
    $this->addNewTextFormat($page, $assert_session);

    $this->drupalGet('admin/config/content/formats/manage/ckeditor5');



    // Add the source editing plugin to the CKEditor 5 toolbar.

    $this->assertNotEmpty($assert_session->waitForElement('css', '.ckeditor5-toolbar-item-sourceEditing'));

    $this->triggerKeyUp('.ckeditor5-toolbar-item-sourceEditing', 'ArrowDown');

    $assert_session->assertWaitOnAjaxRequest();



    $find_validation_error_messages = function () use ($page): array {

      return $page->findAll('css', '[role=alert]:contains("CKEditor 5 only works with HTML-based text formats.")');

      return $page->findAll('css', '[role=alert]:contains("The following tag(s) are already supported by enabled plugins and should not be added to the Source Editing "Manually editable HTML tags" field: Bold (<strong>).")');

    };



    // No validation errors when we start.

    $this->assertCount(0, $find_validation_error_messages());



    // Enable a filter which is not compatible with CKEditor 5, to trigger a

    // Configure Source Editing to allow editing `<strong>` to trigger

    // validation error.

    $page->checkField('Convert URLs into links');

    $assert_session->waitForText('Source editing');

    $page->find('css', '[href^="#edit-editor-settings-plugins-ckeditor5-sourceediting"]')->click();

    $assert_session->assertWaitOnAjaxRequest();

    $assert_session->waitForText('Manually editable HTML tags');

    $source_edit_tags_field = $assert_session->fieldExists('editor[settings][plugins][ckeditor5_sourceEditing][allowed_tags]');

    $source_edit_tags_field->setValue('<strong>');

    $assert_session->assertWaitOnAjaxRequest();

    $this->assertCount(1, $find_validation_error_messages());



    // Disable it: validation messages should be gone.

    $page->uncheckField('Convert URLs into links');

    // Revert Source Editing it: validation messages should be gone.

    $source_edit_tags_field->setValue('');

    $assert_session->assertWaitOnAjaxRequest();

    $this->assertCount(0, $find_validation_error_messages());



    // Re-enable it: validation messages should be back.

    $page->checkField('Convert URLs into links');

    // Add `<strong>` again: validation messages should be back.

    $source_edit_tags_field->setValue('<strong>');

    $assert_session->assertWaitOnAjaxRequest();

    $this->assertCount(1, $find_validation_error_messages());

  }

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

+2 −11
Original line number Diff line number Diff line
@@ -668,10 +668,7 @@ public function provider() { 


    yield "restricted_html can be switched to CKEditor 5 after dropping the two markup-creating filters (3 upgrade messages)" => [

      'format_id' => 'restricted_html',

      'filters_to_drop' => [

        'filter_autop' => TRUE,

        'filter_url' => TRUE,

      ],

      'filters_to_drop' => [],

      'expected_ckeditor5_settings' => [

        'toolbar' => [

          'items' => [
@@ -726,19 +723,13 @@ public function provider() { 
      ],

      'expected_superset' => '<br> <p>',

      'expected_fundamental_compatibility_violations' => [

        '' => [

          0 => 'CKEditor 5 only works with HTML-based text formats. The "<em class="placeholder">Convert line breaks into HTML (i.e. &lt;code&gt;&amp;lt;br&amp;gt;&lt;/code&gt; and &lt;code&gt;&amp;lt;p&amp;gt;&lt;/code&gt;)</em>" (<em class="placeholder">filter_autop</em>) filter implies this text format is not HTML anymore.',

          1 => 'CKEditor 5 only works with HTML-based text formats. The "<em class="placeholder">Convert URLs into links</em>" (<em class="placeholder">filter_url</em>) filter implies this text format is not HTML anymore.',

        ],

        '' => 'CKEditor 5 needs at least the &lt;p&gt; and &lt;br&gt; tags to be allowed to be able to function. They are not allowed by the "<em class="placeholder">Limit allowed HTML tags and correct faulty HTML</em>" (<em class="placeholder">filter_html</em>) filter.',

      ],

      'expected_messages' => [

        'The following plugins were enabled to support tags that are allowed by this text format: <em class="placeholder">Link (for tags: &lt;a&gt;) Block quote (for tags: &lt;blockquote&gt;) Code (for tags: &lt;code&gt;) List (for tags: &lt;ul&gt;&lt;ol&gt;&lt;li&gt;)</em>.',

        'The following tags were permitted by this format\'s filter configuration, but no plugin was available that supports them. To ensure the tags remain supported by this text format, the following were added to the Source Editing plugin\'s <em>Manually editable HTML tags</em>: &lt;cite&gt; &lt;dl&gt; &lt;dt&gt; &lt;dd&gt;.',

        'This format\'s HTML filters includes plugins that support the following tags, but not some of their attributes. To ensure these attributes remain supported by this text format, the following were added to the Source Editing plugin\'s <em>Manually editable HTML tags</em>: &lt;a hreflang&gt; &lt;blockquote cite&gt; &lt;ul type&gt; &lt;ol start type&gt; &lt;h2 id&gt; &lt;h3 id&gt; &lt;h4 id&gt; &lt;h5 id&gt; &lt;h6 id&gt;.',

      ],

      'expected_post_filter_drop_fundamental_compatibility_violations' => [

        '' => 'CKEditor 5 needs at least the &lt;p&gt; and &lt;br&gt; tags to be allowed to be able to function. They are not allowed by the "<em class="placeholder">Limit allowed HTML tags and correct faulty HTML</em>" (<em class="placeholder">filter_html</em>) filter.',

      ],

    ];



    yield "full_html can be switched to CKEditor 5 (no upgrade messages)" => [

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

+7 −22
Original line number Diff line number Diff line
@@ -367,7 +367,7 @@ public function testPair(array $ckeditor5_settings, array $editor_image_upload_s 
  public function providerPair(): array {

    // cspell:ignore donk

    $data = [];

    $data['INVALID: non-HTML format: filter_autop'] = [

    $data['VALID: legacy format: filter_autop'] = [

      'settings' => [

        'toolbar' => [

          'items' => [
@@ -388,11 +388,9 @@ public function providerPair(): array { 
          'settings' => [],

        ],

      ],

      'violations' => [

        '' => 'CKEditor 5 only works with HTML-based text formats. The "<em class="placeholder">Convert line breaks into HTML (i.e. &lt;code&gt;&amp;lt;br&amp;gt;&lt;/code&gt; and &lt;code&gt;&amp;lt;p&amp;gt;&lt;/code&gt;)</em>" (<em class="placeholder">filter_autop</em>) filter implies this text format is not HTML anymore.',

      ],

      'violations' => [],

    ];

    $data['INVALID: non-HTML format: filter_autop + filter_url'] = [

    $data['VALID: legacy HTML format: filter_autop + filter_url'] = [

      'settings' => [

        'toolbar' => [

          'items' => [
@@ -422,14 +420,9 @@ public function providerPair(): array { 
          ],

        ],

      ],

      'violations' => [

        '' => [

          'CKEditor 5 only works with HTML-based text formats. The "<em class="placeholder">Convert URLs into links</em>" (<em class="placeholder">filter_url</em>) filter implies this text format is not HTML anymore.',

          'CKEditor 5 only works with HTML-based text formats. The "<em class="placeholder">Convert line breaks into HTML (i.e. &lt;code&gt;&amp;lt;br&amp;gt;&lt;/code&gt; and &lt;code&gt;&amp;lt;p&amp;gt;&lt;/code&gt;)</em>" (<em class="placeholder">filter_autop</em>) filter implies this text format is not HTML anymore.',

        ],

      ],

      'violations' => [],

    ];

    $data['INVALID: non-HTML format: filter_autop + filter_url (different order)'] = [

    $data['VALID: legacy HTML format: filter_autop + filter_url (different order)'] = [

      'settings' => [

        'toolbar' => [

          'items' => [
@@ -459,12 +452,7 @@ public function providerPair(): array { 
          ],

        ],

      ],

      'violations' => [

        '' => [

          'CKEditor 5 only works with HTML-based text formats. The "<em class="placeholder">Convert line breaks into HTML (i.e. &lt;code&gt;&amp;lt;br&amp;gt;&lt;/code&gt; and &lt;code&gt;&amp;lt;p&amp;gt;&lt;/code&gt;)</em>" (<em class="placeholder">filter_autop</em>) filter implies this text format is not HTML anymore.',

          'CKEditor 5 only works with HTML-based text formats. The "<em class="placeholder">Convert URLs into links</em>" (<em class="placeholder">filter_url</em>) filter implies this text format is not HTML anymore.',

        ],

      ],

      'violations' => [],

    ];

    $data['INVALID: forbidden tags'] = [

      'settings' => [
@@ -506,10 +494,7 @@ public function providerPair(): array { 
      ],

      'filters' => $restricted_html_format_filters,

      'violations' => [

        '' => [

          'CKEditor 5 only works with HTML-based text formats. The "<em class="placeholder">Convert line breaks into HTML (i.e. &lt;code&gt;&amp;lt;br&amp;gt;&lt;/code&gt; and &lt;code&gt;&amp;lt;p&amp;gt;&lt;/code&gt;)</em>" (<em class="placeholder">filter_autop</em>) filter implies this text format is not HTML anymore.',

          'CKEditor 5 only works with HTML-based text formats. The "<em class="placeholder">Convert URLs into links</em>" (<em class="placeholder">filter_url</em>) filter implies this text format is not HTML anymore.',

        ],

        '' => 'CKEditor 5 needs at least the &lt;p&gt; and &lt;br&gt; tags to be allowed to be able to function. They are not allowed by the "<em class="placeholder">Limit allowed HTML tags and correct faulty HTML</em>" (<em class="placeholder">filter_html</em>) filter.',

      ],

    ];

    $data['INVALID: the modified restricted_html text format (with filter_autop and filter_url removed)'] = [