Add ckeditor5-code-block package and CodeBlock plugin

Ironically, the d.o code filter is brittle …

#3261600: Update to CKEditor5 v32.0.0 landed just now, we'll need to get this rebased I'm afraid 🤓

(Interesting BTW that you already had pinned this plugin to 32.0.0, and the rest was still on 31.1.0 — I think that would not always be possible.)

@nod_ discovered the current \Drupal\ckeditor5\Plugin\CKEditor4To5UpgradePluginInterface does not allow expressing this upgrade path yet!

Opened #3268174: Bug in CKE 4 → 5 upgrade path "format" does not always map to "heading", it could map to "codeBlock" too, or both, or neither for that (working on it now), that is hard-blocking this.

#8++ — that would actually allow me to add the upgrade path in #3268174: Bug in CKE 4 → 5 upgrade path "format" does not always map to "heading", it could map to "codeBlock" too, or both, or neither too :)

Test failure analysis

The tests are only failing due to

-            15 => 'code'
+            15 => 'codeBlock'

expectations all over the place.

Solution: #3231328

#3231328: SmartDefaultSettings should select the CKE5 plugin that minimizes creation of HTML restriction supersets is related to that: it wants to make SmartDefaultSettings … well … smarter … about which CKEditor 5 toolbar item/plugin it selects to fulfill a certain markup need.

Because, here's the thing:

1. the code toolbar item supports only <code>
2. the codeBlock toolbar item supports both <pre> and <code>
3. the basic_html text format does not support <pre> out of the box, so this really is literally another showcase for the exact problem that #3231328 is selecting!

Trickier still

To make matters more complex: we actually don't want only the codeBlock toolbar item in there, we want both:

1. if <pre> and <code> are supported in the original text format, we want both <code>and <pre>
2. if only <pre> is allowed, we don't want either, not even codeBlock, which is specifically for the <pre><code>…< /code>< /pre> structure, which obviously requires both tags to be allowed → this is something we currently cannot express

   @nod_ mentioned this in a meeting earlier today and I finally 100% get why he started talking about this! 🙈😅)

3. if only <code> is allowed, we want only code, not codeBlock

(See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/code.)

Conclusion

I'm not yet sure how the Trickier still section will play out in combination with #3231328: SmartDefaultSettings should select the CKE5 plugin that minimizes creation of HTML restriction supersets … so … postponing this on that issue and starting to work on that right now to hopefully unblock this ASAP.

#3231328: SmartDefaultSettings should select the CKE5 plugin that minimizes creation of HTML restriction supersets is not 100% ready yet, but it is ~80%. It's ready for review.

Note: this issue should add test coverage for the Trickier still section, specifically the only <pre> and both scenarios (the only <code> scenario is already covered), by adding something to SmartDefaultSettingsTest like $basic_html_format_with_alignable_p and adding a new test case to its ::provider()

Update: #3231328: SmartDefaultSettings should select the CKE5 plugin that minimizes creation of HTML restriction supersets is now fully ready for review :)

add a simple <pre> button

That's invalid HTML for blocks of code, see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/code#notes.

Verified that using this MR with the codeblock plugin on content that only have a <pre> tag without the corresponding <code> tag result in the <pre> tag being stripped => data loss.

😬

Tricky!

I do believe that the UX of the CodeBlock plugin covers the 99% use case. But of course we should not have any data loss problem.

What if we have a creative combo? What if we ensure that the ckeditor5_codeBlock plugin you're adding here also loads the htmlSupport.GeneralHtmlSupport plugin and has this configuration:

    config:
      htmlSupport:
        allow:
          -
            name: pre

… then it'd ensure <pre> is retained always (just like its drupal.elements metadata claims), it just won't be editable using the CodeBlock UX out of the box. That way we avoid adding a weird <pre> button with an inferior UX? (Not 100% sure about that, because of #3269059-4: [PP-1] Add a <pre> button to CKEditor 5 for the upgrade path from CKEditor 4 format dropdown).

#17: That validation error happens when the user configures a particular tag to be allowed.

What I'm suggesting is something like this patch, which means that it's just the existing ckeditor5_codeBlock plugin that does this, simply to ensure that all forms of <pre> are possible.

We used to do that for a whole bunch of tags! See https://git.drupalcode.org/project/ckeditor5/-/commit/9b1a917c502ef5bc5b..., https://git.drupalcode.org/project/ckeditor5/-/commit/393386bd2413424912..., etc.

+1 — I think not being able to create <pre> without source editing seems fine to me. The <code>-less <pre> use case is a minority. Even more so because Basic HTML and \Drupal\filter\Plugin\Filter\FilterHtml both have <code> enabled by default!

The MR looks great to me — it will pass as soon as those two blockers land.

Marking this as explicitly blocked because there's no other work to be done here for now. 👍

You beat me to it :D

Argh! Interesting problem 😅 That is once again where your remark from #10's Trickier still, point 2 comes in …

I think I agree with your conclusion — which fortunately means this won't be blocked after all. 👍🤞

Manually tested both the plugin and the upgrade path. Works perfectly! 🤩🚀

@nod_ is out, so I'll address the feedback.

I made one trivial change, plus some comment changes. So I can definitely still RTBC.

Also uploading as a patch to test against 10.0.x and 9.4.x. Cannot yet be cherry-picked to 9.3.x because it is still on version 32 of CKEditor 5. That's waiting for #3269651: Update Drupal 9.3.x to CKEditor 5 v34.0.0 along with other un-backported issues.