The media library should perform access checks against the revision of the entity being edited

This seems to make sense to me. Can we add a test for this?

1. +++ b/core/modules/media_library/tests/src/Kernel/MediaLibraryWidgetTest.php
   @@ -0,0 +1,156 @@
   +class MediaLibraryWidgetTest extends KernelTestBase {
   

   We already have a MediaLibraryAccessTest. Might make sense to put this in there?

2. +++ b/core/modules/media_library/tests/src/Kernel/MediaLibraryWidgetTest.php
   @@ -0,0 +1,156 @@
   +  protected function buildWidgetForm($entity) {
   

   Not really sure if we need to build the widget, in MediaLibraryAccessTest we simply create a state ourselves and check access. Which should be sufficient.

       $state = MediaLibraryState::create(
         $state->getOpenerId(),
         $state->getAllowedTypeIds(),
         $state->getSelectedTypeId(),
         $state->getAvailableSlots(),
         $parameters
       );
   

3. +++ b/core/modules/media_library/tests/src/Kernel/MediaLibraryWidgetTest.php
   @@ -0,0 +1,156 @@
   +    $this->createUser();
   

   I know we do this a lot, but a comment that this is to make sure our uid is not 1 would be nice I think.

I have test on d9 patch apply and test executed successfully but when I try to reproduce the issue I got below js issue please see below issue

EntityToolbarView.js?v=9.1.0-dev:145 Uncaught TypeError: Cannot read property 'get' of null
at r.position (EntityToolbarView.js?v=9.1.0-dev:145)
at render (EntityToolbarView.js?v=9.1.0-dev:51)
at p (backbone.js:337)
at v (backbone.js:322)
at u (backbone.js:110)
at r.a.trigger (backbone.js:312)
at r.set (backbone.js:525)
at r.set (BaseModel.js?v=9.1.0-dev:29)
at r.success (EntityModel.js?v=9.1.0-dev:175)
at Drupal.AjaxCommands.entitySaverAjax.commands.quickeditEntitySaved (EntityModel.js?v=9.1.0-dev:234)

The reason I was a bit triggered by the class name MediaLibraryWidgetTest is because we had #3087227: [META] Split up and refactor MediaLibraryTest where we split up all kinds of widget related tests that were in 1 class (which became unmaintainable). So that is why I'm trying to see if we can have smaller, more specifically named classes.

Besides that, since this is already specifically named for widget related tests, it is good enough for me.

Committed and pushed d901cf487b to 9.1.x and 49f78bc356 to 9.0.x. Thanks!

diff --git a/core/modules/media_library/src/MediaLibraryFieldWidgetOpener.php b/core/modules/media_library/src/MediaLibraryFieldWidgetOpener.php
index 49a741b13e..348f22f104 100644
--- a/core/modules/media_library/src/MediaLibraryFieldWidgetOpener.php
+++ b/core/modules/media_library/src/MediaLibraryFieldWidgetOpener.php
@@ -73,7 +73,7 @@ public function checkAccess(MediaLibraryState $state, AccountInterface $account)
       $entity = $storage->loadRevision($parameters['revision_id']);
       $entity_access = $access_handler->access($entity, 'update', $account, TRUE);
     }
-    else if ($parameters['entity_id']) {
+    elseif ($parameters['entity_id']) {
       $entity = $storage->load($parameters['entity_id']);
       $entity_access = $access_handler->access($entity, 'update', $account, TRUE);
     }
diff --git a/core/modules/media_library/tests/src/Kernel/MediaLibraryWidgetTest.php b/core/modules/media_library/tests/src/Kernel/MediaLibraryWidgetTest.php
index 41df47c047..450bbda0dc 100644
--- a/core/modules/media_library/tests/src/Kernel/MediaLibraryWidgetTest.php
+++ b/core/modules/media_library/tests/src/Kernel/MediaLibraryWidgetTest.php
@@ -80,7 +80,7 @@ protected function setUp(): void {
 
     $this->adminUser = $this->createUser([
       'administer entity_test content',
-      'view media'
+      'view media',
     ]);
   }
 
@@ -101,7 +101,7 @@ public function testWidgetAccess() {
    */
   public function testRevisionableWidgetAccess() {
     $allowed_revision = EntityTestRev::create([
-      'name' => 'allowed_access'
+      'name' => 'allowed_access',
     ]);
     $allowed_revision->save();
 

Fixed coding standards on commit.

Leaving open for backport to 8.9.x as this is bug there. The test set up method will need fixing to not typehint.

Rando fail

Woops, there was actually some phpcs issues. Fixed those.

There was a random failure. Restoring RTBC.

Committed d89941f and pushed to 8.9.x. Thanks!

Adding the error that I got when hitting this, for searchability:

Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException: Non-reusable blocks must set an access dependency for access control. in Drupal\Core\Routing\AccessAwareRouter->checkAccess() (line 120 of /app/site/web/core/lib/Drupal/Core/Routing/AccessAwareRouter.php)

I received the same error as @pameeela recently on 8.9.13 when editing a Layout Builder block that contains a Media field with Media Library enabled. The only solution I could find was to manually delete the block from Layout Builder and recreate/readd it. Based on comment #33, shouldn't this have been resolved long before 8.9.13? How could we still be experiencing this error?

Hmm you're right, I hadn't checked versions yet, but my site also has the fix from this issue already. We just deleted the blocks and recreated them as you suggested in the other ticket.

But I don't have steps to reproduce this, it only occurred on one node on a staging site, have not had any other reports.

This issue exists in Drupal 9.5 with entity clone

Steps to reproduce:

• Add a Block which have a media reference in a Layout Builder enabled node
• Clone the node
• Go to layout of the new cloned node and edit the block.
• Remove the arrached media reference and try to add a new Media reference (Either by selecting existing media item or upload new)
• Throws an AJAX error Non-reusable blocks must set an access dependency for access control.

I have similar case with @imalabya

I get error `Non-reusable blocks must set an access dependency for access control.` after uploading the image for the second time

Steps to reproduce:

• Add a Block which have a media reference in a Layout Builder enabled node
• Clone the node
• Go to the layout of the new cloned node and edit the block.
• Remove the attached media reference and try to add upload new image. Save block. Save layout.
• Go to the layout of the cloned node again, and edit the block.
• Remove the attached media reference and again, try to add upload new image. After selecting the image there is an error on the browser console

I test this on simplytest.me using Drupal 10.3.6 and 10.3.9 with Entity clone module version 2.1.x.