Drupal 10 compatibility

Queued for 9.3 and 9.4

1. +++ b/tests/src/Functional/EntityEmbedTwigTest.php
   @@ -34,19 +34,34 @@ class EntityEmbedTwigTest extends EntityEmbedTestBase {
   +    // TODO: Drupal Rector Notice: Please delete the following comment after you've made any necessary changes.
   +    // Verify the assertion: pageTextContains() for HTML responses, responseContains() for non-HTML responses.
   +    // The passed text should be HTML decoded, exactly as a human sees it in the browser.
   ...
   +    // TODO: Drupal Rector Notice: Please delete the following comment after you've made any necessary changes.
   +    // Verify the assertion: pageTextNotContains() for HTML responses, responseNotContains() for non-HTML responses.
   +    // The passed text should be HTML decoded, exactly as a human sees it in the browser.
   ...
   +    // TODO: Drupal Rector Notice: Please delete the following comment after you've made any necessary changes.
   +    // Verify the assertion: pageTextContains() for HTML responses, responseContains() for non-HTML responses.
   +    // The passed text should be HTML decoded, exactly as a human sees it in the browser.
   ...
   +    // TODO: Drupal Rector Notice: Please delete the following comment after you've made any necessary changes.
   +    // Verify the assertion: pageTextNotContains() for HTML responses, responseNotContains() for non-HTML responses.
   +    // The passed text should be HTML decoded, exactly as a human sees it in the browser.
   

   Needs manual review (remove and make sure assertions transformed correctly.)

2. +++ b/tests/src/FunctionalJavascript/MediaImageTest.php
   @@ -596,6 +596,7 @@ class MediaImageTest extends EntityEmbedTestBase {
   +   * @doesNotPerformAssertions
   

   Is this needed? maybe not.

This needs to also bump the drupal/core requirement from 9.2 to 9.4.

Still needs a bit of work since drupal/embed doesn't have a D10 release

Embed now has a D10 release

I tested, @nkoporec (#9) MR on Drupal 10 and am happy to pass the review!

Test Steps.

1. Installed 8.x-1.x-dev and patched.
2. Created an embed button for media, "/admin/config/content/embed"
3. Added embed button for full_html format. "/admin/config/content/formats/manage/full_html"
4. Tested "entity_embed" by embedding documents and images.
5. Tested content entity embeds via "entity_embed".

Some drupal tests failed.
https://www.drupal.org/pift-ci-job/2462015

I have removed the update hooks and its tests and fixtures since those update hooks are more that 3 years old, instead I implemented the hook_update_last_removed().

I also added the protected $failOnJavascriptConsoleErrors = FALSE; to ConfigurationUiTest, since D10 tests will fail if the console have any JS errors, I think that the JS error reported is not related to the module, but a core issue, see a similar issue https://www.drupal.org/project/drupal/issues/3091878

The ContentTranslationTest failure is most likely a legit failure and we need to fix that still.

Fixed the tests, partly overlaps with https://www.drupal.org/project/entity_embed/issues/3269454, leaving up to the maintainers to decide whether to include this in this patch or commit the separate issue first.

hook_field_widget_single_element_form_alter was added in 9.2.x, when hook_field_widget_form_alter was deprecated. See https://www.drupal.org/node/3180429

Let's make the fixes backward compatible for at least 9.2.x with the file_url_generator service changes.

Moving to Needs Review.

I think we should address CKEditor5 in its own issue. Despite the fact this module may not do anything without it 😬. But this is a good first step.

+1 to adding CKE5 support in a separate issue — that already exists: #3272732: Drupal 10 & CKEditor 5 readiness 😊

Posted a review — I think we can reduce the amount of change and simplify the code 😊

Retitling because there's little automatic about this left 😅

I have officially bumped the module to require ^9.3 in the current 8.x-1.x branch.

Rebased the MR, as the changes to composer.json and the info.yml file were creating merge conflicts

Closed #3264333: Drupal 9 - Deprecated class name Twig_Extension as a duplicate.

Done.

And now running all tests again against D10 — let's see what fails.

Let's find out how this does on Drupal 10.

DrupalCI + D10 fail unless composer.json is modified.

#29: I doubt it, because there's nothing about Drupal core in there:

{
    "name": "drupal/entity_embed",
    "description": "Allows any entity to be embedded within a text area using a WYSIWYG editor.",
    "type": "drupal-module",
    "license": "GPL-2.0-or-later",
    "require": {
        "drupal/embed": "^1.5"
    },
    "require-dev": {
        "drupal/entity_browser": "^2.2"
    }
}

Just like in #3294389: Fix fails in HEAD, there's no way to convince DrupalCI to run this on Drupal 10 — the only solution is to commit it. Because the only "Drupal 9.3" mention is in the *.info.yml file and that has been broadened to support Drupal 10.

Ran the entire test suite in D10:

$ php core/scripts/run-tests.sh --sqlite /tmp/test.sqlite --url http://core.test --module entity_embed

Drupal test run
---------------

Tests to be run:
  - Drupal\Tests\entity_embed\FunctionalJavascript\ButtonAdminTest
  - Drupal\Tests\entity_embed\FunctionalJavascript\CKEditorIntegrationTest
  - Drupal\Tests\entity_embed\FunctionalJavascript\ConfigurationUiTest
  - Drupal\Tests\entity_embed\FunctionalJavascript\ContentTranslationTest
  - Drupal\Tests\entity_embed\FunctionalJavascript\EntityEmbedDialogTest
  - Drupal\Tests\entity_embed\FunctionalJavascript\ImageFieldFormatterTest
  - Drupal\Tests\entity_embed\FunctionalJavascript\MediaImageTest
  - Drupal\Tests\entity_embed\Functional\EntityEmbedDialogTest
  - Drupal\Tests\entity_embed\Functional\EntityEmbedDisplayManagerTest
  - Drupal\Tests\entity_embed\Functional\EntityEmbedEntityBrowserTest
  - Drupal\Tests\entity_embed\Functional\EntityEmbedHooksTest
  - Drupal\Tests\entity_embed\Functional\EntityEmbedTwigTest
  - Drupal\Tests\entity_embed\Functional\EntityReferenceFieldFormatterTest
  - Drupal\Tests\entity_embed\Functional\FileFieldFormatterTest
  - Drupal\Tests\entity_embed\Functional\ImageFieldFormatterTest
  - Drupal\Tests\entity_embed\Functional\RecursionProtectionTest
  - Drupal\Tests\entity_embed\Functional\ViewModeFieldFormatterTest
  - Drupal\Tests\entity_embed\Kernel\EntityEmbedFilterDisabledIntegrationsTest
  - Drupal\Tests\entity_embed\Kernel\EntityEmbedFilterLegacyTest
  - Drupal\Tests\entity_embed\Kernel\EntityEmbedFilterOverridesTest
  - Drupal\Tests\entity_embed\Kernel\EntityEmbedFilterTest
  - Drupal\Tests\entity_embed\Kernel\EntityEmbedFilterTranslationTest

Test run started:
  Wednesday, October 26, 2022 - 15:43

Test summary
------------

Drupal\Tests\entity_embed\FunctionalJavascript\ButtonAdminTe   4 passes                                      
Drupal\Tests\entity_embed\FunctionalJavascript\CKEditorInteg   0 passes             1 exceptions             
FATAL Drupal\Tests\entity_embed\FunctionalJavascript\CKEditorIntegrationTest: test runner returned a non-zero error code (2).
Drupal\Tests\entity_embed\FunctionalJavascript\CKEditorInteg   0 passes   1 fails                            
Drupal\Tests\entity_embed\FunctionalJavascript\Configuration   0 passes             1 exceptions             
FATAL Drupal\Tests\entity_embed\FunctionalJavascript\ConfigurationUiTest: test runner returned a non-zero error code (2).
Drupal\Tests\entity_embed\FunctionalJavascript\Configuration   0 passes   1 fails                            
Drupal\Tests\entity_embed\FunctionalJavascript\ContentTransl   0 passes             1 exceptions             
FATAL Drupal\Tests\entity_embed\FunctionalJavascript\ContentTranslationTest: test runner returned a non-zero error code (2).
Drupal\Tests\entity_embed\FunctionalJavascript\ContentTransl   0 passes   1 fails                            
Drupal\Tests\entity_embed\FunctionalJavascript\EntityEmbedDi   0 passes             1 exceptions             
FATAL Drupal\Tests\entity_embed\FunctionalJavascript\EntityEmbedDialogTest: test runner returned a non-zero error code (2).
Drupal\Tests\entity_embed\FunctionalJavascript\EntityEmbedDi   0 passes   1 fails                            
Drupal\Tests\entity_embed\FunctionalJavascript\ImageFieldFor   0 passes             1 exceptions             
FATAL Drupal\Tests\entity_embed\FunctionalJavascript\ImageFieldFormatterTest: test runner returned a non-zero error code (2).
Drupal\Tests\entity_embed\FunctionalJavascript\ImageFieldFor   0 passes   1 fails                            
Drupal\Tests\entity_embed\FunctionalJavascript\MediaImageTes   0 passes             1 exceptions             
FATAL Drupal\Tests\entity_embed\FunctionalJavascript\MediaImageTest: test runner returned a non-zero error code (2).
Drupal\Tests\entity_embed\FunctionalJavascript\MediaImageTes   0 passes   1 fails                            
Drupal\Tests\entity_embed\Functional\EntityEmbedDialogTest     0 passes             1 exceptions             
FATAL Drupal\Tests\entity_embed\Functional\EntityEmbedDialogTest: test runner returned a non-zero error code (2).
Drupal\Tests\entity_embed\Functional\EntityEmbedDialogTest     0 passes   1 fails                            
Drupal\Tests\entity_embed\Functional\EntityEmbedDisplayManag   0 passes             1 exceptions             
FATAL Drupal\Tests\entity_embed\Functional\EntityEmbedDisplayManagerTest: test runner returned a non-zero error code (2).
Drupal\Tests\entity_embed\Functional\EntityEmbedDisplayManag   0 passes   1 fails                            
Drupal\Tests\entity_embed\Functional\EntityEmbedEntityBrowse   0 passes             1 exceptions             
FATAL Drupal\Tests\entity_embed\Functional\EntityEmbedEntityBrowserTest: test runner returned a non-zero error code (2).
Drupal\Tests\entity_embed\Functional\EntityEmbedEntityBrowse   0 passes   1 fails                            
Drupal\Tests\entity_embed\Functional\EntityEmbedHooksTest      0 passes             1 exceptions             
FATAL Drupal\Tests\entity_embed\Functional\EntityEmbedHooksTest: test runner returned a non-zero error code (2).
Drupal\Tests\entity_embed\Functional\EntityEmbedHooksTest      0 passes   1 fails                            
Drupal\Tests\entity_embed\Functional\EntityEmbedTwigTest       0 passes             1 exceptions             
FATAL Drupal\Tests\entity_embed\Functional\EntityEmbedTwigTest: test runner returned a non-zero error code (2).
Drupal\Tests\entity_embed\Functional\EntityEmbedTwigTest       0 passes   1 fails                            
Drupal\Tests\entity_embed\Functional\EntityReferenceFieldFor   0 passes             1 exceptions             
FATAL Drupal\Tests\entity_embed\Functional\EntityReferenceFieldFormatterTest: test runner returned a non-zero error code (2).
Drupal\Tests\entity_embed\Functional\EntityReferenceFieldFor   0 passes   1 fails                            
Drupal\Tests\entity_embed\Functional\FileFieldFormatterTest    0 passes             1 exceptions             
FATAL Drupal\Tests\entity_embed\Functional\FileFieldFormatterTest: test runner returned a non-zero error code (2).
Drupal\Tests\entity_embed\Functional\FileFieldFormatterTest    0 passes   1 fails                            
Drupal\Tests\entity_embed\Functional\ImageFieldFormatterTest   0 passes             1 exceptions             
FATAL Drupal\Tests\entity_embed\Functional\ImageFieldFormatterTest: test runner returned a non-zero error code (2).
Drupal\Tests\entity_embed\Functional\ImageFieldFormatterTest   0 passes   1 fails                            
Drupal\Tests\entity_embed\Functional\RecursionProtectionTest   0 passes             1 exceptions             
FATAL Drupal\Tests\entity_embed\Functional\RecursionProtectionTest: test runner returned a non-zero error code (2).
Drupal\Tests\entity_embed\Functional\RecursionProtectionTest   0 passes   1 fails                            
Drupal\Tests\entity_embed\Functional\ViewModeFieldFormatterT   0 passes             1 exceptions             
FATAL Drupal\Tests\entity_embed\Functional\ViewModeFieldFormatterTest: test runner returned a non-zero error code (2).
Drupal\Tests\entity_embed\Functional\ViewModeFieldFormatterT   0 passes   1 fails                            
Drupal\Tests\entity_embed\Kernel\EntityEmbedFilterDisabledIn   1 passes                                      
Drupal\Tests\entity_embed\Kernel\EntityEmbedFilterLegacyTest   4 passes                                      
Drupal\Tests\entity_embed\Kernel\EntityEmbedFilterOverridesT   2 passes                                      
Drupal\Tests\entity_embed\Kernel\EntityEmbedFilterTest        21 passes                                      
Drupal\Tests\entity_embed\Kernel\EntityEmbedFilterTranslatio  15 passes                                      

Test run duration: 10 min 33 sec

Hiding patches in favor of the merge request.

Transferring credit from #3317353: Replace "stable" them with "stable9" for testing purposes.

The 2 commits you added make total sense, and now it's green! 👍🚀

Thanks, everyone. Merged into 8.x-1.x!

Note that we probably should NOT tag a release with this change until we have #3309747: Add CKEditor 5 compatibility and keep supporting CKEditor 4 done, because without CKEditor 5 support, this module is essentially worthless with Drupal 10 core alone.

Thank you, for having time to maintain the Entity Embed important module.

Hoping for a soft tag release ( pre-release ), to ease testing with Drupal 10
Testing now with the 1.x-dev development.

Worked great for me with the patch from MR9. Looking forward to a release.

Agrees with Adam

Note that we probably should NOT tag a release with this change until we have #3309747: Add CKEditor 5 compatibility and keep supporting CKEditor 4 done, because without CKEditor 5 support, this module is essentially worthless with Drupal 10 core alone.

@Maintainers: If #3309747: Add CKEditor 5 compatibility and keep supporting CKEditor 4 won't land soon and nobody can do anything about it, would it perhaps make sense to document on the module page, why there's no Drupal 10 compatible release yet for this module?

Or can anyone push things forward at #3309747: Add CKEditor 5 compatibility and keep supporting CKEditor 4 or become Co-Maintainer of Embed? Or does someone know someone who knows the maintainers to ping them? ;)

Thanks!

@Maintaners: Do you have any idea if your team plans to be releasing a D10 compatible version of this module in the near future? Any help you can provide would be appreciated!

Thanks

You can still use ckeditor 4 w/ drupal 10. So a dev release that only supports ckeditor 4 isn't entirely worthless.

@phenaproxima, CKE4 is still relevant for D10 as updating CKE4 to CKE5 is not that straightforward. So, having a D10 compliant release with CKE4 support only is completely fine

I'm using Drupal 9.5.x and this is the only important module still showing not compatible on Upgrade Status. I tried following this thread but lost it. Is there a available patch that would cover the upgrade to 10?

@wxman I think you're looking for #3272732: Drupal 10 & CKEditor 5 readiness which has a patch that needs review.

@SamLerner I think it looks like the working patch is https://www.drupal.org/files/issues/2023-05-05/entity_embed-3272732-80.p... .
What I'm wondering, before I try it, if this is one I can apply to the module while still using D9.5.x. As far as I understand, the upgrade won't work unless all the installed modules show ready for D10.

@wxman you'd need a patched and working version of Embed to also get a patched version of Entity Embed working. Hopefully that gets resolved soon, lots of attention on it since it's DrupalCon. Once Embed is CKE5-compatible, this should quickly follow.