Drupal 10.5/11.2 Compatibility (tooltip broken, cannot edit embedded entities)

Fixed - pretty sure I've got the right link icon. It was removed from the way this plugin was loading it (see https://github.com/ckeditor/ckeditor5/issues/17304) and I saw there was a `IconLink` in the main package now so using that.

Core version req - If we don't bump this then people on 10.4 could update before they go to 10.5 or 11.2 and the plugin it would break for them.

Quick test of the syntax I used:

drush php:eval "var_dump(\Composer\Semver\Semver::satisfies('10.4.7', '>=10.5 <11.0 || ^11.2'))"
bool(false)
drush php:eval "var_dump(\Composer\Semver\Semver::satisfies('10.5.0', '>=10.5 <11.0 || ^11.2'))"
bool(true)
drush php:eval "var_dump(\Composer\Semver\Semver::satisfies('10.6.0', '>=10.5 <11.0 || ^11.2'))" 
bool(true)
drush php:eval "var_dump(\Composer\Semver\Semver::satisfies('11.0.0', '>=10.5 <11.0 || ^11.2'))"
bool(false)
drush php:eval "var_dump(\Composer\Semver\Semver::satisfies('11.1.0', '>=10.5 <11.0 || ^11.2'))" 
bool(false)
drush php:eval "var_dump(\Composer\Semver\Semver::satisfies('11.2.0', '>=10.5 <11.0 || ^11.2'))"
bool(true)

Tests failing - current major still set to 11.1?

If somebody can review / test so that it can be moved to RTBC, I'm happy to reach out to maintainers to ask them to get it committed - but I think its fair to wait till the status is RTBC before doing this.

Re #16 - yes I didn't realise this wasn't already committed. It wasn't present but also isn't it the gitignore so not clear here. It's pretty common to commit the yarn.lock file as it can ensure dependency versions aren't changed until it's intentional

Tests are still failing and it's not clear it it's related to this change or not, would be good to get a green pipeline.

Phpunit on previous minor is expected to fail as it's not compatible, but the current job is now testing on 11.2 and there is 2 failures there.

Thanks @smustgrave!

Confirming your change in d130db37 looks correct as per this change - https://github.com/ckeditor/ckeditor5/commit/4a595753940ab2087db7901922b...

Still have some unexplained test failures here - not really sure I'll have time to dig further into these

Ok - did end up trying to investigate these.

The last CI run shows the following tests failing:

58.307s Drupal\Tests\entity_embed\FunctionalJavascript\EntityEmbedDialogTest      0 passed, 1 failed, 1 log(s)
209.891s Drupal\Tests\entity_embed\FunctionalJavascript\ButtonAdminTest            3 passed, 1 failed, 1 log(s)
177.661s …upal\Tests\entity_embed\FunctionalJavascript\CKEditor5IntegrationTest    0 passed, 2 failed, 16 errored, 1 log(s)

I ran these locally on 11.2.2 with same phpunit version. The first 2 pass (with deprecations):

$ phpunit -c app/core/phpunit.xml app/modules/contrib/entity_embed/tests/src/FunctionalJavascript/EntityEmbedDialogTest.php 
PHPUnit 11.5.27 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.23
Configuration: /data/app/core/phpunit.xml

.                                                                   1 / 1 (100%)

Time: 00:38.220, Memory: 12.00 MB

OK, but there were issues!
Tests: 1, Assertions: 6, PHPUnit Deprecations: 5.


$ phpunit -c app/core/phpunit.xml app/modules/contrib/entity_embed/tests/src/FunctionalJavascript/ButtonAdminTest.php 
PHPUnit 11.5.27 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.23
Configuration: /data/app/core/phpunit.xml

....                                                                4 / 4 (100%)

Time: 03:09.833, Memory: 12.00 MB

OK, but there were issues!
Tests: 4, Assertions: 32, PHPUnit Deprecations: 16.

The last one CKEditor5IntegrationTest.php does indeed fail multiple times.

RuntimeException: The autoloader expected class "Drupal\embed\Plugin\CKEditor5Plugin\EmbedCKEditor5PluginBase" to be defined in file "/data/app/modules/contrib/embed/src/Plugin/CKEditor5Plugin/EmbedCKEditor5PluginBase.php". The file was found but the class was not in it, the class name or namespace probably has a typo.

Looks like in the embed module that class is using the wrong namespace - it has Drupal\entity_embed\Plugin\CKEditor5Plugin

But this module doesn't appear to be extended that - what even is that class

Adding #3517882: The namespace of EmbedCKEditor5PluginBase does not respect PSR4 as a related issue - that will need to land first for the tests to go green.

Re #37 - tests are also failing on 2.0.x branch for the previous minor which is not affected by the CKeditor version change - I'm not sure how much effort is due here vs a separate issue. Edit, my mistake, I see prev minor actually passes on 2.0.x.

Previous minor tests on this branch - well we kind of expect them to fail now, the JS is now dependent on a CKEditor version only available in 10.5 and 11.2 - given 10.6 and 11.3 will be out at some point, it makes sense to keep the tests running on prev minors? Maybe one for the maintainers to give input here.

Other tests - I'm not convinced anything is broken. I already investigated in #27. I added a new branch with the patch committed in #3517882 which was responsible for some of the failures. We can see here with that in place that the tests are passing https://git.drupalcode.org/issue/entity_embed-3531672/-/jobs/6389586 (except for CKEditor5Integration)

A quick win to reduce confusion here would be to please tag a new version of embed so that we can stop being distracted by this unrelated failure.

Then its just the CKEditor5Integration failure to address.

The CKEditor5Integration fails with:

"Test was run in child process and ended unexpectedly"

This test passes for me locally - if somebody wants to do battle with gitlab to figure out what / why the pipeline is crashing go for it - but I think I still have confidence the test failure is not related to this change and that the work everybody has done here is good.