From 1d5995a2562f7d36c13f7bcdd073bdfce42e4526 Mon Sep 17 00:00:00 2001 From: Lauri Eskola Date: Sat, 23 Apr 2022 17:37:28 +0300 Subject: [PATCH] Issue #3261943 by bnjmnm, lauriii, Wim Leers, andreasderijcke, ifrik: Confusing behavior after pressing "Apply changes to allowed tags" with invalid value --- .../modules/ckeditor5/ckeditor5.libraries.yml | 5 - core/modules/ckeditor5/css/filter.admin.css | 4 - .../js/ckeditor5.filter.admin.es6.js | 104 +----------------- .../ckeditor5/js/ckeditor5.filter.admin.js | 59 ---------- .../ckeditor5/src/Plugin/Editor/CKEditor5.php | 9 ++ .../ckeditor5/src/SmartDefaultSettings.php | 15 +++ ...keditor5_incompatible_filter_test.info.yml | 6 + .../Plugin/Filter/FilterIsIncompatible.php | 26 +++++ .../src/FunctionalJavascript/AdminUiTest.php | 20 ++-- .../CKEditor5AllowedTagsTest.php | 43 +++++--- .../src/Kernel/SmartDefaultSettingsTest.php | 6 +- 11 files changed, 99 insertions(+), 198 deletions(-) delete mode 100644 core/modules/ckeditor5/css/filter.admin.css create mode 100644 core/modules/ckeditor5/tests/modules/ckeditor5_incompatible_filter_test/ckeditor5_incompatible_filter_test.info.yml create mode 100644 core/modules/ckeditor5/tests/modules/ckeditor5_incompatible_filter_test/src/Plugin/Filter/FilterIsIncompatible.php diff --git a/core/modules/ckeditor5/ckeditor5.libraries.yml b/core/modules/ckeditor5/ckeditor5.libraries.yml index 0c580790d6d..6e86ee030fb 100644 --- a/core/modules/ckeditor5/ckeditor5.libraries.yml +++ b/core/modules/ckeditor5/ckeditor5.libraries.yml @@ -91,16 +91,11 @@ drupal.ckeditor5.mediaAlign: drupal.ckeditor5.filter.admin: js: js/ckeditor5.filter.admin.js: {} - css: - theme: - css/filter.admin.css: { } dependencies: - core/drupal - - core/drupal.message - core/once - core/drupal.ajax - core/drupalSettings - - core/drupal.message admin: js: diff --git a/core/modules/ckeditor5/css/filter.admin.css b/core/modules/ckeditor5/css/filter.admin.css deleted file mode 100644 index dc9c54cbb3b..00000000000 --- a/core/modules/ckeditor5/css/filter.admin.css +++ /dev/null @@ -1,4 +0,0 @@ -.ckeditor5-filter-attention { - padding: 4px; - outline: 1px red dashed; -} diff --git a/core/modules/ckeditor5/js/ckeditor5.filter.admin.es6.js b/core/modules/ckeditor5/js/ckeditor5.filter.admin.es6.js index 64cf95a6f44..6775bbf7631 100644 --- a/core/modules/ckeditor5/js/ckeditor5.filter.admin.es6.js +++ b/core/modules/ckeditor5/js/ckeditor5.filter.admin.es6.js @@ -2,7 +2,6 @@ * @file * Provides Text Editor UI improvements specific to CKEditor 5. */ - ((Drupal, once) => { Drupal.behaviors.allowedTagsListener = { attach: function attach(context) { @@ -21,113 +20,12 @@ 'drupal-ajax', '[data-drupal-selector="filter-format-edit-form"] [disabled], [data-drupal-selector="filter-format-add-form"] [disabled]', ) + // eslint-disable-next-line max-nested-callbacks .forEach((disabledElement) => { disabledElement.removeAttribute('disabled'); }); }); }); - once( - 'allowed-tags-listener', - context.querySelector( - '[data-drupal-selector="edit-filters-filter-html-settings-allowed-html"]', - ), - ).forEach((textarea) => { - const editorSelect = document.querySelector( - '[data-drupal-selector="edit-editor-editor"]', - ); - const filterCheckbox = document.querySelector( - '[data-drupal-selector="edit-filters-filter-html-status"]', - ); - const formSubmit = document.querySelector( - '[data-drupal-selector="edit-actions-submit"]', - ); - const wrapper = textarea.closest('div'); - - const resetChanges = () => { - const updateButtonContainer = document.querySelector( - '[data-ckeditor5-allowed-tags-info]', - ); - if (updateButtonContainer) { - updateButtonContainer.remove(); - } - - const allowedTagsDisabledHelp = document.querySelector( - '[data-ckeditor5-allowed-tags-disabled-help]', - ); - if (allowedTagsDisabledHelp) { - allowedTagsDisabledHelp.remove(); - } - - formSubmit.removeAttribute('disabled'); - wrapper.classList.remove('ckeditor5-filter-attention'); - }; - - resetChanges(); - - const addAllowedTagsUpdateButton = () => { - resetChanges(); - - if ( - editorSelect.value === 'ckeditor5' && - filterCheckbox && - filterCheckbox.checked - ) { - if (!textarea.hasAttribute('readonly')) { - wrapper.classList.add('ckeditor5-filter-attention'); - - const container = document.createElement('div'); - container.setAttribute('data-ckeditor5-allowed-tags-info', true); - const description = document.createElement('p'); - - description.innerText = Drupal.t( - 'Switching to CKEditor 5 requires, at minimum, the tags "


". After switching to CKEditor 5, this field will be read-only, and will be updated based on which CKEditor 5 plugins are enabled. When switching to CKEditor 5 from an existing text format with content, we recommend documenting what tags are in use and then enabling the CKEditor 5 plugins that support them.', - ); - - const updateButton = document.createElement('button'); - updateButton.setAttribute( - 'name', - 'update-ckeditor5-allowed-tags', - ); - updateButton.innerText = Drupal.t( - 'Apply changes to allowed tags.', - ); - updateButton.addEventListener('click', () => { - editorSelect.dispatchEvent(new CustomEvent('change')); - setTimeout(() => { - resetChanges(); - }); - }); - - container.appendChild(description); - container.appendChild(updateButton); - - wrapper.appendChild(container); - - // In this very specific use case, submitting the filter form must - // be prevented. - // - CKEditor 5 is the selected editor, but it's not yet accepted - // by the form because it's not passing the validation restraint - // requiring CKEditor 5 compatible "Allowed Tags". This validator, - // by necessity fires before CKEditor 5 is registered with the - // form. - // - The registering of an editor with the form typically occurs - // when a change - formSubmit.setAttribute('disabled', true); - const formSubmitHelp = document.createElement('p'); - formSubmitHelp.setAttribute( - 'data-ckeditor5-allowed-tags-disabled-help', - true, - ); - formSubmitHelp.textContent = Drupal.t( - 'This form is not submittable when the editor is set to CKEditor 5 unless the "Limit allowed HTML tags and correct faulty HTML" filter\'s "Allowed HTML tags" field includes the tags required by CKEditor 5', - ); - formSubmit.parentNode.append(formSubmitHelp); - } - } - }; - editorSelect.addEventListener('change', addAllowedTagsUpdateButton); - filterCheckbox.addEventListener('change', addAllowedTagsUpdateButton); - }); }, }; diff --git a/core/modules/ckeditor5/js/ckeditor5.filter.admin.js b/core/modules/ckeditor5/js/ckeditor5.filter.admin.js index f278999e3a9..9feaff4a8cf 100644 --- a/core/modules/ckeditor5/js/ckeditor5.filter.admin.js +++ b/core/modules/ckeditor5/js/ckeditor5.filter.admin.js @@ -15,65 +15,6 @@ }); }); }); - once('allowed-tags-listener', context.querySelector('[data-drupal-selector="edit-filters-filter-html-settings-allowed-html"]')).forEach(textarea => { - const editorSelect = document.querySelector('[data-drupal-selector="edit-editor-editor"]'); - const filterCheckbox = document.querySelector('[data-drupal-selector="edit-filters-filter-html-status"]'); - const formSubmit = document.querySelector('[data-drupal-selector="edit-actions-submit"]'); - const wrapper = textarea.closest('div'); - - const resetChanges = () => { - const updateButtonContainer = document.querySelector('[data-ckeditor5-allowed-tags-info]'); - - if (updateButtonContainer) { - updateButtonContainer.remove(); - } - - const allowedTagsDisabledHelp = document.querySelector('[data-ckeditor5-allowed-tags-disabled-help]'); - - if (allowedTagsDisabledHelp) { - allowedTagsDisabledHelp.remove(); - } - - formSubmit.removeAttribute('disabled'); - wrapper.classList.remove('ckeditor5-filter-attention'); - }; - - resetChanges(); - - const addAllowedTagsUpdateButton = () => { - resetChanges(); - - if (editorSelect.value === 'ckeditor5' && filterCheckbox && filterCheckbox.checked) { - if (!textarea.hasAttribute('readonly')) { - wrapper.classList.add('ckeditor5-filter-attention'); - const container = document.createElement('div'); - container.setAttribute('data-ckeditor5-allowed-tags-info', true); - const description = document.createElement('p'); - description.innerText = Drupal.t('Switching to CKEditor 5 requires, at minimum, the tags "


". After switching to CKEditor 5, this field will be read-only, and will be updated based on which CKEditor 5 plugins are enabled. When switching to CKEditor 5 from an existing text format with content, we recommend documenting what tags are in use and then enabling the CKEditor 5 plugins that support them.'); - const updateButton = document.createElement('button'); - updateButton.setAttribute('name', 'update-ckeditor5-allowed-tags'); - updateButton.innerText = Drupal.t('Apply changes to allowed tags.'); - updateButton.addEventListener('click', () => { - editorSelect.dispatchEvent(new CustomEvent('change')); - setTimeout(() => { - resetChanges(); - }); - }); - container.appendChild(description); - container.appendChild(updateButton); - wrapper.appendChild(container); - formSubmit.setAttribute('disabled', true); - const formSubmitHelp = document.createElement('p'); - formSubmitHelp.setAttribute('data-ckeditor5-allowed-tags-disabled-help', true); - formSubmitHelp.textContent = Drupal.t('This form is not submittable when the editor is set to CKEditor 5 unless the "Limit allowed HTML tags and correct faulty HTML" filter\'s "Allowed HTML tags" field includes the tags required by CKEditor 5'); - formSubmit.parentNode.append(formSubmitHelp); - } - } - }; - - editorSelect.addEventListener('change', addAllowedTagsUpdateButton); - filterCheckbox.addEventListener('change', addAllowedTagsUpdateButton); - }); } }; const originalAjaxEventResponse = Drupal.Ajax.prototype.eventResponse; diff --git a/core/modules/ckeditor5/src/Plugin/Editor/CKEditor5.php b/core/modules/ckeditor5/src/Plugin/Editor/CKEditor5.php index d72a62e6a8d..1547a97da10 100644 --- a/core/modules/ckeditor5/src/Plugin/Editor/CKEditor5.php +++ b/core/modules/ckeditor5/src/Plugin/Editor/CKEditor5.php @@ -524,7 +524,16 @@ class CKEditor5 extends EditorBase implements ContainerFactoryPluginInterface { ]); $submitted_filter_format = CKEditor5::getSubmittedFilterFormat($form_state); $fundamental_incompatibilities = CKEditor5::validatePair($minimal_ckeditor5_editor, $submitted_filter_format, FALSE); + foreach ($fundamental_incompatibilities as $violation) { + // If the violation uses the nonAllowedElementsMessage template, it can + // be skipped because this is a violation that automatically fixed + // within SmartDefaultSettings, but SmartDefaultSettings does not + // execute until this validator passes. + if ($violation->getMessageTemplate() === $violation->getConstraint()->nonAllowedElementsMessage) { + continue; + } + // @codingStandardsIgnoreLine $form_state->setErrorByName('editor][editor', t($violation->getMessageTemplate(), $violation->getParameters())); } diff --git a/core/modules/ckeditor5/src/SmartDefaultSettings.php b/core/modules/ckeditor5/src/SmartDefaultSettings.php index 11b0d159370..05a6b67ea83 100644 --- a/core/modules/ckeditor5/src/SmartDefaultSettings.php +++ b/core/modules/ckeditor5/src/SmartDefaultSettings.php @@ -168,6 +168,21 @@ final class SmartDefaultSettings { } } + if ($editor->getFilterFormat()->filters('filter_html')->status) { + $filter_html_restrictions = HTMLRestrictions::fromTextFormat($editor->getFilterFormat()); + $fundamental = new HTMLRestrictions($this->pluginManager->getProvidedElements([ + 'ckeditor5_essentials', + 'ckeditor5_paragraph', + ])); + $missing_tags = $fundamental->diff($filter_html_restrictions); + if (!$missing_tags->isEmpty()) { + $editor->getFilterFormat()->setFilterConfig('filter_html', $filter_html_restrictions->merge($fundamental)->getAllowedElements()); + $messages[MessengerInterface::TYPE_STATUS][] = $this->t("The following tag(s) were added to Limit allowed HTML tags and correct faulty HTML, because they are needed to provide fundamental CKEditor 5 functionality : @missing_tags.", [ + '@missing_tags' => $missing_tags->toFilterHtmlAllowedTagsString(), + ]); + } + } + // Finally: for all enabled plugins, find the ones that are configurable, // and add their default settings. For enabled plugins with element subsets, // compute the appropriate settings to achieve the subset that matches the diff --git a/core/modules/ckeditor5/tests/modules/ckeditor5_incompatible_filter_test/ckeditor5_incompatible_filter_test.info.yml b/core/modules/ckeditor5/tests/modules/ckeditor5_incompatible_filter_test/ckeditor5_incompatible_filter_test.info.yml new file mode 100644 index 00000000000..f279515767d --- /dev/null +++ b/core/modules/ckeditor5/tests/modules/ckeditor5_incompatible_filter_test/ckeditor5_incompatible_filter_test.info.yml @@ -0,0 +1,6 @@ +name: CKEditor 5 Incompatible Filter Test +type: module +description: "Provides a filter incompatible with CKEditor 5" +package: Testing +dependencies: + - drupal:ckeditor5 diff --git a/core/modules/ckeditor5/tests/modules/ckeditor5_incompatible_filter_test/src/Plugin/Filter/FilterIsIncompatible.php b/core/modules/ckeditor5/tests/modules/ckeditor5_incompatible_filter_test/src/Plugin/Filter/FilterIsIncompatible.php new file mode 100644 index 00000000000..e2f23131c82 --- /dev/null +++ b/core/modules/ckeditor5/tests/modules/ckeditor5_incompatible_filter_test/src/Plugin/Filter/FilterIsIncompatible.php @@ -0,0 +1,26 @@ +waitForText('Machine name'); $page->checkField('roles[authenticated]'); - // Enable a filter that is incompatible with CKEditor 5 if not configured - // correctly, so validation is triggered when attempting to switch. + // Enable a filter that is incompatible with CKEditor 5, so validation is + // triggered when attempting to switch. + $incompatible_filter_name = 'filters[filter_incompatible][status]'; $number_ajax_instances_before = $this->getSession()->evaluateScript('Drupal.ajax.instances.length'); - $this->assertTrue($page->hasUncheckedField('filters[filter_html][status]')); - $page->checkField('filters[filter_html][status]'); + $this->assertTrue($page->hasUncheckedField($incompatible_filter_name)); + $page->checkField($incompatible_filter_name); $this->assertEmpty($assert_session->waitForElement('css', '.ajax-progress-throbber')); $assert_session->assertWaitOnAjaxRequest(); $number_ajax_instances_after = $this->getSession()->evaluateScript('Drupal.ajax.instances.length'); @@ -81,18 +83,20 @@ class AdminUiTest extends CKEditor5TestBase { $page->selectFieldOption('editor[editor]', 'ckeditor5'); $assert_session->assertWaitOnAjaxRequest(); + $filter_warning = 'CKEditor 5 only works with HTML-based text formats. The "A TYPE_MARKUP_LANGUAGE filter incompatible with CKEditor 5" (filter_incompatible) filter implies this text format is not HTML anymore.'; + // The presence of this validation error message confirms the AJAX callback // was invoked. - $assert_session->pageTextContains('CKEditor 5 needs at least the

and
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.'); + $assert_session->pageTextContains($filter_warning); // 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_html][status]')); - $page->uncheckField('filters[filter_html][status]'); + $this->assertTrue($page->hasCheckedField($incompatible_filter_name)); + $page->uncheckField($incompatible_filter_name); $this->assertNotEmpty($assert_session->waitForElement('css', '.ajax-progress-throbber')); $assert_session->assertWaitOnAjaxRequest(); - $assert_session->pageTextNotContains('CKEditor 5 needs at least the

and
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.'); + $assert_session->pageTextNotContains($filter_warning); } /** diff --git a/core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5AllowedTagsTest.php b/core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5AllowedTagsTest.php index 1922cf36df8..f6202f63baa 100644 --- a/core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5AllowedTagsTest.php +++ b/core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5AllowedTagsTest.php @@ -25,6 +25,7 @@ class CKEditor5AllowedTagsTest extends CKEditor5TestBase { 'ckeditor5', 'media', 'media_library', + 'ckeditor5_incompatible_filter_test', ]; /** @@ -41,6 +42,13 @@ class CKEditor5AllowedTagsTest extends CKEditor5TestBase { */ protected $defaultElementsWhenUpdatingNotCkeditor5 = '