CKEditor 5 support

To paraphrase @acbramley from Slack:

Just curious why webform doesn't use a format/editor config?

In Slack you wrote:

Webform is NOT using a text format/editor instead, we are loading the core/ckeditor library and then configuring the editor via https://git.drupalcode.org/project/webform/-/blob/6.1.x/js/webform.eleme...

😱 This was never supported by the CKEditor (4) module in Drupal core! This is not using the API provided by Drupal.

You chose to use the CKEditor 4 API directly (i.e. https://ckeditor.com/docs/ckeditor4/latest/guide/index.html).

For example. the Webform's HTML editor, use BR tags instead of P tags.

… that's not just any example, that is a really crucial atypical behavior 😱 You used https://ckeditor.com/docs/ckeditor4/latest/guide/dev_best_practices.html... to achieve this.

And per https://ckeditor.com/docs/ckeditor5/latest/installation/getting-started/... … I'm afraid CKEditor 5 won't support this anymore:

			<td><a href="/docs/ckeditor4/latest/api/CKEDITOR_config.html#cfg-forceEnterMode">forceEnterMode</a></td>
			<td>N/A. Se also <a href="#enterMode"><code>config.enterMode

.

… so the only potential solution I see for you is to create a custom htmlWriter plugin that still allows <p> in the editor but converts them to <br> when converting ("downcasting") to actual HTML. Fortunately we have somewhat of an example for you in Drupal core: core/modules/ckeditor5/js/ckeditor5_plugins/drupalHtmlEngine/src/*.

Perhaps it's asking if it's worth the pain? Perhaps doing things the Drupally way is the go?

***EDIT***
the issue I'm describing here occurs whether or not a wysiwyg editor is used, even on non-wysiwyg fields for labels edited with input boxes in the webform translation ui. So I thought some more and questioned the assumption below as probably incorrect which is why I'm editting my own comment here.
*** END EDIT ***

This above issue discussed by @Wimleer might explain why we're still observing this issue with +64kb long webform translations

#2836206: [Translations] Translation of big webforms broken

for some reason webforms is processing translations as a single line when saving translations with the web gui interface, however if importing with drush the correct multiline translation import is working (provided the \r\n are replaced first).

#2844452: Export configuration YAML strings as multiline

I'd like to trigger tests, I'm also going to try to test this with 9.4.8 and ckeditor5 (enable this core module)

@jrockowitz,

line 731 of your merge request
js/webform.element.html_editor.ckeditor..js: {}

this line was probably instead intended to be like this:

js/webform.element.html_editor.ckeditor.js: {}

Could this explain your trouble?

Triggered a Drupal 10 test (manually)
Triggered a Drupal 9.4 test (was automatic)

ok I ran 6.2.x-dev with this MR branch on a site with: Drupal 9.4.8,

I enabled the ckeditor5 module however the text format default remained the same when I was editing webform configurations (advanced html form items using the GUI).

I tested also the translation of webforms, didn't notice any change in behavior here.

the ckeditor 1.0.1 module (ckeditor v4) was still trying to find library files:

Not Found
The requested URL "http://f7fb44f3-5fdf-4283-ac13-7b0b4f153111.web.ahdev.cloud/modules/cont..." was not found on this server.

Not Found
The requested URL "http://f7fb44f3-5fdf-4283-ac13-7b0b4f153111.web.ahdev.cloud/modules/cont..." was not found on this server.

In Drupal 9.4.8, ckeditor version 5 is included with Drupal 9.4.8 core however ckeditor 4 is now a contrib module.

the new html_editor.element_format I couldn't confirm if this was actually working or not, I didn't notice a difference before or after using the 6.2.x-dev with the above branch.

one thing I did test is I did uninstall the ckeditor module, once I did this, gutenberg kicked in but ckeditor 5 was not visible.

I also tested a super long translation of a super long webform > 64 kilobytes in the configuration file, no change in behavior, could not save a change in any version of webform with any branch or release, the \r\n single line locales_source blob limit problem issue is caused somewhere else inside drupal (the webform module?) because I could not save a change through the GUI interface of the translated webform, only could do that using drush cim after having a clean yml file (drush webform-tidy) (drush webform-tidy very useful btw). So unfortunately I'm still looking for a solution to :

#2836206: [Translations] Translation of big webforms broken

conclusion:
More testing is needed, suggest testing these changes with Drupal 10
As for these changes on Drupal 9.4.8, I didn't notice much change at all, with that said, it didn't seem to bring up the ckeditor5 even when the ckeditor5 module was enabled. Perhaps there's a webform settings change needed for the new text format.

seems to still need work but good start so far.

@jrockowitz, may the force be with you, I'll test your new branch out whenever it comes.

@jrockowitz, I quickly tested your new branch with D9.4.8
installed the ckeditor5 core module however when I uninstalled ckeditor4 it uninstalled two text formats (kinda expected)
next time I'll try with both ckeditor5 and ckeditor4 installed.

@jrockowitz
Great work!

I saw the dependency error, I added an upstream ticket for Drupal 10 support of that dependency.

#3326466: Offering to maintain jQuery UI Checkboxradio

the related dependency found in the 1 failure needs a D10 release.

I haven't yet looked at the test code closely to see about a fix

Sorry for my silence here! 😬 But excellent progress! 🤩

#5: that is an excellent question! Sadly, the links you posted no longer work … probably because you rebased in the mean time (GitLab strikes again 😬).

#49: oh, NICE! Even better :)

If I understand #54 correctly, then it's still keeping CKEditor 4 support around, just switching it to the same "hidden text format/editor" approach.

1. +++ b/includes/webform.editor.inc
   @@ -19,6 +19,36 @@ use Drupal\Core\Config\Entity\ConfigEntityInterface;
   +function webform_filter_format_load($entities) {
   +  foreach ($entities as $entity) {
   +    if ($entity->id() === WebformHtmlEditor::DEFAULT_FILTER_FORMAT) {
   +      $route_name = \Drupal::routeMatch()->getRouteName();
   +      $filter_format = \Drupal::routeMatch()->getParameter('filter_format');
   +      if ($route_name === 'filter.admin_overview') {
   +        // Set status to FALSE which hides the default webform filter format.
   +        $entity->set('status',  FALSE);
   +      }
   +      elseif ($filter_format === WebformHtmlEditor::DEFAULT_FILTER_FORMAT) {
   +        // Throw not found exception to prevent the default webform
   +        // filter format from being edited.
   +        throw new NotFoundHttpException();
   +      }
   +    }
   +  }
   +}
   

   😳🤔 Route-based runtime config entity altering … I am not quite sure if this is actually free of side effects …

   AFAIK the correct/supported way to do this is through \Drupal\Core\Config\ConfigFactoryOverrideInterface.

   Or … maybe better yet, I'm wondering if we can work around this at the \Drupal\filter\FilterFormatListBuilder level by using access control logic 🤔

   That class determines which FilterFormat config entities to list using \Drupal\Core\Entity\EntityListBuilder::getEntityIds().

   I checked to see if one could use hook_query_alter() to apply this restriction, but that is not possible, because \Drupal\Core\Config\Entity\Query\Query::execute() does not use the DB query API 😬

   So I suggest subclassing FilterFormatListBuilder, overriding getEntityIds() and then overriding the "list_builder" = "Drupal\filter\FilterFormatListBuilder" entity type annotation.

   (I doubt anybody else has the same need … http://grep.xnddx.ru/search?text=FilterFormatListBuilder&filename= finds 2 matches, but they're just copy/paste leftovers 👍)

2. +++ b/src/Element/WebformHtmlEditor.php
   @@ -14,7 +16,12 @@ use Drupal\webform\Utility\WebformXss;
   +  const DEFAULT_FILTER_FORMAT = 'webform_default';
   
   @@ -279,4 +259,85 @@ class WebformHtmlEditor extends FormElement {
   +    // Remove the 'webform' default text format from allowed formats.
   +    // This is needed because the webform default text format DOES NOT filter HTML.
   

   🤔 Why not do this instead?

   function webform_entity_filter_format_access(EntityInterface $entity, $operation, AccountInterface $account) {
     if ($entity->id() === 'webform_default') {
       return AccessResult::forbidden('This text format can only be used by Webform.');
     }
     // No opinion.
     return AccessResult::neutral();
   }
   

P.S.: seems like this is the contrib use case that would justify adding FilterFormat::isLocked(), which 5 config entity types in Drupal core already have!

a.k.a USER1, entity access checks are disabled.

IIRC, AccessResult::forbidden() applies always, including to user 1.

IIRC, the only thing that is special cased is that user 1 automatically gets all permissions, but that doesn't mean everything is necessarily accessible. See \Drupal\user\Entity\User::hasPermission().

A quick check of \Drupal\Core\Entity\EntityAccessControlHandler::access() appears to confirm this. But I can definitely be wrong.