Impossible to enable <ol type> or <ul type> with GHS: switch to List's successor, DocumentList

Failing tests.

4 expected failures, all somewhat like this:

Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'<ol type="A"><li>foo</li><li>bar</li></ol>'
+'<ol><li>foo</li><li>bar</li></ol>'

IOW: the test coverage works as expected 👍

Tested locally after #3274767: Update to CKEditor 5 v34.0.0 landed, still failing.

I was informed by the CKEditor 5 team that the List plugin in the ckeditor5-list package has never been updated to integrate with GHS, so this is expected.

The solution is to use DocumentList, which is new in v34.0.0. Unfortunately, it's not available in the DLL build of ckeditor5-list.

The CKEditor 5 team will tag a v34.0.1 release of CKEditor 5 which will add DocumentList to ckeditor5-list's DLL build. This will increase the size of the JS to be downloaded. Fortunately, the List plugin will be sunset (and removed) in the near future, which means that the JS size will automatically decrease in a future release.

Confirmed that <ol type> works fine out of the box in CKEditor 4 in Drupal 9's Basic HTML text format. Also reported upstream: https://github.com/ckeditor/ckeditor5/issues/11615#issuecomment-1099321116.

We'll get DocumentList in #3277405: Update @ckeditor/ckeditor5-list to v34.0.1 👍

1. Testing the <ol type="A"> test case in #2 with HEAD:
   

   1) Drupal\Tests\ckeditor5\FunctionalJavascript\SourceEditingTest::testAllowingExtraAttributes with data set "<ol type="A">" ('<ol type="A"><li>foo</li><li>...></ol>', '<ol type="A"><li>foo</li><li>...></ol>', '<ol type="A">')
   Failed asserting that two strings are identical.
   --- Expected
   +++ Actual
   @@ @@
   -'<ol type="A"><li>foo</li><li>bar</li></ol>'
   +'<ol><li>foo</li><li>bar</li></ol>'
   

2. Testing that same test case with #3277405: Update @ckeditor/ckeditor5-list to v34.0.1 applied and this tiny diff:
   

   diff --git a/core/modules/ckeditor5/ckeditor5.ckeditor5.yml b/core/modules/ckeditor5/ckeditor5.ckeditor5.yml
   index e91e43604c..aa0b6059d2 100644
   --- a/core/modules/ckeditor5/ckeditor5.ckeditor5.yml
   +++ b/core/modules/ckeditor5/ckeditor5.ckeditor5.yml
   @@ -304,7 +304,7 @@ ckeditor5_linkMedia:
    
    ckeditor5_list:
      ckeditor5:
   -    plugins: [list.List]
   +    plugins: [list.DocumentList]
      drupal:
        label: List
        library: core/ckeditor5.list
   

   results in:

   Testing /Users/wim.leers/Work/d8/core/modules/ckeditor5/tests/src/FunctionalJavascript
   .                                                                   1 / 1 (100%)
   
   Time: 29.14 seconds, Memory: 8.00 MB
   
   OK (1 test, 15 assertions)
   

So … that means we have another stable blocker almost fixed! 🥳

• For branches that already have #3261599: Use CKEditor 5's native <ol start> support (and also support <ol reversed>) merged: 3274651-11--branches-with-3261599.patch (currently only 10.0.x).
• For other branches: 3274651-11.patch (currently 9.4.x and 9.3.x).

Silly oversight for the --branches-with-3261599 patch … 🙈

I just confirmed over at #3274178-10: [upstream] [GHS] Allowing `<ul type>` or `<ol type>` in SourceEditing crashes CKEditor 5 when editing `<ul></ul>` or `<ol></ol>` without any <li> elements as children that this also fixes that bug! 🥳

Transfering tags + credit.

Same patch, but now running the entire core test suite.

I've proven in #11 that this will also work in 9.4.x and 9.3.x. That only requires a different patch because #3261599: Use CKEditor 5's native <ol start> support (and also support <ol reversed>) didn't land yet in those two branches.

So, this patch should be tested and committed against all 3. But for now, only testing against 10.0.x.

#3261599: Use CKEditor 5's native <ol start> support (and also support <ol reversed>) was just committed to 9.4.x and 9.3.x too — queuing tests of #17.

+++ b/core/modules/ckeditor5/ckeditor5.ckeditor5.yml
@@ -353,8 +353,8 @@ ckeditor5_linkMedia:
-     - list.List
-     - list.ListProperties
+     - list.DocumentList
+     - list.DocumentListProperties

Per https://github.com/ckeditor/ckeditor5/pull/11668#discussion_r859487538, this means we should also update \Drupal\ckeditor5\HTMLRestrictions::getTextContainerElementList().

Random unrelated fail.

Wow — interesting! 🤯

That is a pre-existing bug in Claro then, because it means other list style types already do not work in Claro. The scope of this issue is to make CKEditor 5 support this (because it is currently a data loss issue).

Claro making it look like it does not work correctly still is better than today: today it is a data loss issue.

Apparently that line was added in #3079738: Add Claro administration theme to core — i.e. the original commit of Claro in 2019. I cannot find a pre-existing bug report for this: https://www.drupal.org/project/issues/drupal?text=ordered+list+ol&status.... Created one: #3282598: Claro should not hardcode decimal list style type for <ol>.

<li>s will not always need a <div> or <p> inside — but if you apply an alignment, then yes, it will generate a <p>: CKEditor 5 will generate <li><p class="text-align-…">...</p></li>, not <li class="text-align-…">...</li>.

See https://github.com/ckeditor/ckeditor5/pull/11668#discussion_r859487538 (previously linked from ##2).