Migrate "codetag" contrib CKEditor 4 plugin to built-in equivalent in core's CKEditor 5

Resolved CSpell errors

Seems to be an unrelated feature, requeuing tests.

Thanks for this great contribution and wonderful news, @jcnventura! Thanks for investigating! 😊🙏 Because this is de facto now upgrading the core upgrade path, because it must be Drupal core providing this upgrade path (since the equivalent CKE5 plugin of the CKE4 plugin is part of Drupal core now), bumping this to a stable blocker.

Indeed unrelated failure. Re-queueing.

Note that this will need tests. I'll work on those.

Tests. :) Test-only patch is also the interdiff.

(And limiting test runs for both to only CKE5 tests.)

That worked :)

Testing Drupal\Tests\ckeditor5\Kernel\CKEditor4to5UpgradeCompletenessTest
E..........                                                       11 / 11 (100%)

Time: 00:07.343, Memory: 4.00 MB

There was 1 error:

1) Drupal\Tests\ckeditor5\Kernel\CKEditor4to5UpgradeCompletenessTest::testButtons
OutOfBoundsException: No upgrade path found for the "Code" button. 

Let's run all tests again, not just CKE5's.

1. +++ b/core/modules/ckeditor5/src/Plugin/CKEditor4To5Upgrade/Contrib.php
   @@ -0,0 +1,73 @@
   +      case NULL:
   +        // Do Nothing.
   +        break;
   

   I think we should delete this. phpcs dictates there must be at least one case. Well, that just means we can remove the switch statement.

2. +++ b/core/modules/ckeditor5/src/Plugin/CKEditor4To5Upgrade/Contrib.php
   @@ -0,0 +1,73 @@
   +      case NULL:
   +        // Do Nothing.
   +        break;
   

   Same thing here.

Manually tested patch #13

Steps I took:

1. On a updated 9.4.x Drupal environment I installed codetag via composer ten enabled codetag
2. Went to /admin/config/content/formats chose to configure one of the text formats (on my case I chose Full HTML)
3. Added the codetag button to the active toolbar (first image)
4. Saved the configuration
5. Entered on the configuration again
6. Attempted tho upgrade the editor to CKEditor 5 though the drop down
7. Got the message "The CKEditor 4 button Code does not have a known upgrade path....." and the button was moved back to Available Buttons (second image)
8. Applied patch and cleared the cache.
9. Entered on the configuration again
10. Attempted tho upgrade the editor to CKEditor 5 though the drop down
11. Didn't get the previous message and the button was on the right place (third image)

I also checked the patch for spelling errors and coding standards. Everything was correct, only the method names on Contrib.php were not in lowerCamel format, but I see it was on purpose because of this module's name so it's RTBC for me.

+1 RTBC

Edit.: Please ignore the fourth file. I sent it by mistake.

Given:

1. the manual testing in #14
2. the fact that it's @jcnventura who wrote the logic additions, not I, I only provided live reviews for him :)
3. the fact that the test coverage was basically 2 lines of code

I think it's reasonable to RTBC this, despite my having contributed those 2 lines of code for the test coverage.

This looks great!

I notice that even when the code plugin is enabled to support the switch from codetag, it's not included in the "The following plugins were enabled" message. It would be good to have that, or at the very least an explanation as to why it isn't included (much better to have though 🙂)

I notice that even when the code plugin is enabled to support the switch from codetag, it's not included in the "The following plugins were enabled" message. It would be good to have that, or at the very least an explanation as to why it isn't included (much better to have though 🙂)

That is intentional. We only generate such messages when there are additional plugins enabled compared to the CKEditor 4 configuration. In this case, the same exact functionality/button has an exact equivalent, so providing a message is not helpful.

If we'd change this then #3245967: Messages upon switching to CKEditor 5 are overwhelming would be even worse, and hence the expected_messages expectations in \Drupal\Tests\ckeditor5\Kernel\SmartDefaultSettingsTest::provider() would increase by a lot.

To prove this, I'm adding an explicit test case to SmartDefaultSettingsTest too, that is just like the others. Observe how in this case, the expected messages are indeed empty, but many test cases have the The following plugins were enabled to support … style message you were hoping for. Since this is again so trivial, I am comfortable self-RTBC'ing.

Committed and pushed 46903e6c3c to 10.0.x and 12e8b703f8 to 9.4.x and c94dc2dcc6 to 9.3.x. Thanks!