Drupal
/
drupal
mirror of https://git.drupalcode.org/project/drupal.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Issue #3261943 by bnjmnm, lauriii, Wim Leers, andreasderijcke, ifrik: Confusing behavior after pressing "Apply changes to allowed tags" with invalid value

merge-requests/2161/head
Lauri Eskola 2022-04-23 17:37:28 +03:00
parent c3a73e9df4
commit 1d5995a256
No known key found for this signature in database
GPG Key ID: 382FC0F5B0DF53F8
11 changed files with 99 additions and 198 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
core/modules/ckeditor5/ckeditor5.libraries.yml
View File

@ -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:

4
core/modules/ckeditor5/css/filter.admin.css
View File

@ -1,4 +0,0 @@
.ckeditor5-filter-attention {
padding: 4px;
outline: 1px red dashed;
}

104
core/modules/ckeditor5/js/ckeditor5.filter.admin.es6.js
View File

@ -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 "<p> <br>". 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);
});
},
};

59
core/modules/ckeditor5/js/ckeditor5.filter.admin.js
View File

@ -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 "<p> <br>". 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;

9
core/modules/ckeditor5/src/Plugin/Editor/CKEditor5.php
View File

@ -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()));
}

15
core/modules/ckeditor5/src/SmartDefaultSettings.php
View File

@ -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 <em>Limit allowed HTML tags and correct faulty HTML</em>, 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

6
core/modules/ckeditor5/tests/modules/ckeditor5_incompatible_filter_test/ckeditor5_incompatible_filter_test.info.yml Normal file
View File

@ -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

26
core/modules/ckeditor5/tests/modules/ckeditor5_incompatible_filter_test/src/Plugin/Filter/FilterIsIncompatible.php Normal file
View File

@ -0,0 +1,26 @@
<?php
namespace Drupal\ckeditor5_incompatible_filter_test\Plugin\Filter;
use Drupal\filter\FilterProcessResult;
use Drupal\filter\Plugin\FilterBase;
/**
* Provides a filter incompatible with CKEditor 5.
*
* @Filter(
* id = "filter_incompatible",
* title = @Translation("A TYPE_MARKUP_LANGUAGE filter incompatible with CKEditor 5"),
* type = Drupal\filter\Plugin\FilterInterface::TYPE_MARKUP_LANGUAGE
* )
*/
class FilterIsIncompatible extends FilterBase {
/**
* {@inheritdoc}
*/
public function process($text, $langcode) {
return new FilterProcessResult($text);
}
}

20
core/modules/ckeditor5/tests/src/FunctionalJavascript/AdminUiTest.php
View File

@ -18,6 +18,7 @@ class AdminUiTest extends CKEditor5TestBase {
protected static $modules = [
'media_library',
'ckeditor',
'ckeditor5_incompatible_filter_test',
];
/**
@ -68,11 +69,12 @@ class AdminUiTest extends CKEditor5TestBase {
$assert_session->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 <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.');
$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 <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.');
$assert_session->pageTextNotContains($filter_warning);
}
/**

43
core/modules/ckeditor5/tests/src/FunctionalJavascript/CKEditor5AllowedTagsTest.php
View File

@ -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 = '<a href hreflang> <em> <strong> <cite> <blockquote cite> <code> <ul type> <ol start type> <li> <dl> <dt> <dd> <h2 id> <h3 id> <h4 id> <h5 id> <h6 id> <img src alt data-entity-type data-entity-uuid>';
/**
* The expected allowed elements after updating to CKEditor5.
*
* @var string
*/
protected $defaultElementsAfterUpdatingToCkeditor5 = '<br> <p> <h2 id> <h3 id> <h4 id> <h5 id> <h6 id> <cite> <dl> <dt> <dd> <a hreflang href> <blockquote cite> <ul type> <ol start type> <img src alt data-entity-type data-entity-uuid> <strong> <em> <code> <li>';
/**
* Test enabling CKEditor 5 in a way that triggers validation.
*/
@ -48,25 +56,26 @@ class CKEditor5AllowedTagsTest extends CKEditor5TestBase {
$page = $this->getSession()->getPage();
$assert_session = $this->assertSession();
$incompatible_filter_name = 'filters[filter_incompatible][status]';
$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.';
$this->createNewTextFormat($page, $assert_session, 'ckeditor');
$page->selectFieldOption('editor[editor]', 'ckeditor');
$assert_session->assertWaitOnAjaxRequest();
$page->checkField('filters[filter_html][status]');
$page->checkField($incompatible_filter_name);
$assert_session->assertWaitOnAjaxRequest();
$page->selectFieldOption('editor[editor]', 'ckeditor5');
$assert_session->assertWaitOnAjaxRequest();
$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.');
$assert_session->pageTextContains($filter_warning);
// Add the tags that must be included in the html filter for CKEditor 5
// defaults.
$allowed_html_field = $assert_session->fieldExists('filters[filter_html][settings][allowed_html]');
$allowed_html_field->setValue('<p> <br>');
// Disable the incompatible filter.
$page->uncheckField($incompatible_filter_name);
// Confirm there are no longer any warnings.
$assert_session->waitForElementRemoved('css', '[data-drupal-messages] [role="alert"]');
$page->pressButton('update-ckeditor5-allowed-tags');
$assert_session->assertWaitOnAjaxRequest();
// Confirm the text format can be saved.
$this->saveNewTextFormat($page, $assert_session);
}
@ -153,16 +162,9 @@ class CKEditor5AllowedTagsTest extends CKEditor5TestBase {
$page->selectFieldOption('editor[editor]', 'ckeditor5');
$assert_session->assertWaitOnAjaxRequest();
$this->assertTrue($page->find('css', '[name="editor[editor]"]')->hasClass('error'));
$this->assertNotEmpty($assert_session->waitForElement('css', '[data-ckeditor5-allowed-tags-info]'));
$this->assertHtmlEsqueFieldValueEquals('filters[filter_html][settings][allowed_html]', $this->defaultElementsWhenUpdatingNotCkeditor5);
$assert_session->pageTextContains('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 : <br> <p>');
$this->assertHtmlEsqueFieldValueEquals('filters[filter_html][settings][allowed_html]', $this->defaultElementsAfterUpdatingToCkeditor5);
$allowed_html_field = $assert_session->fieldExists('filters[filter_html][settings][allowed_html]');
$allowed_html_field->setValue('<p> <br>');
$page->pressButton('update-ckeditor5-allowed-tags');
$assert_session->assertWaitOnAjaxRequest();
$this->assertFalse($page->find('css', '[name="editor[editor]"]')->hasClass('error'));
$page->pressButton('Save configuration');
$assert_session->pageTextContains('The Image upload toolbar item requires image uploads to be enabled.');
@ -172,6 +174,15 @@ class CKEditor5AllowedTagsTest extends CKEditor5TestBase {
$page->checkField('editor[settings][plugins][ckeditor5_imageUpload][status]');
$assert_session->assertWaitOnAjaxRequest();
$page->pressButton('Save configuration');
$this->assertSession()->pageTextContains('The following attribute(s) are already supported by enabled plugins and should not be added to the Source Editing "Manually editable HTML tags" field: Image (<img src alt data-entity-uuid data-entity-type>)');
$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_value = $source_edit_tags_field->getValue();
$source_edit_tags_field->setValue(str_replace('<img src alt data-entity-type data-entity-uuid>', '', $source_edit_tags_field_value));
$assert_session->assertWaitOnAjaxRequest();
$page->pressButton('Save configuration');
$assert_session->pageTextContains('The text format ckeditor has been updated');
}

6
core/modules/ckeditor5/tests/src/Kernel/SmartDefaultSettingsTest.php
View File

@ -817,11 +817,10 @@ class SmartDefaultSettingsTest extends KernelTestBase {
'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;.',
'The following tag(s) were added to <em>Limit allowed HTML tags and correct faulty HTML</em>, because they are needed to provide fundamental CKEditor 5 functionality : &lt;br&gt; &lt;p&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.',
],
'expected_post_filter_drop_fundamental_compatibility_violations' => [],
];
yield "full_html can be switched to CKEditor 5 (no upgrade messages)" => [
@ -934,6 +933,7 @@ class SmartDefaultSettingsTest extends KernelTestBase {
'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=&quot;1 A I&quot;&gt; &lt;h2 id=&quot;jump-*&quot;&gt; &lt;h3 id&gt; &lt;h4 id&gt; &lt;h5 id&gt; &lt;h6 id&gt;.',
'The following tag(s) were added to <em>Limit allowed HTML tags and correct faulty HTML</em>, because they are needed to provide fundamental CKEditor 5 functionality : &lt;br&gt; &lt;p&gt;.',
],
],
];