Layout builder fails to assign inline block access dependencies for the overrides section storage on entities with pending revisions

Hello @Sam152

Thanks for the patch! I am also running through the same problem but here the actual case is with Translations.

Applied this patch to make layout builder inline blocks translatable https://www.drupal.org/project/drupal/issues/2946333#comment-13186348

So there is no problem if translation is not added for the embedded paragraph inside the inline block of a layout enabled node.

Moment you add the translation for the paragraphs, the paragraph gets locked and data disappear from frontend.
I have checked the things in deep and its because the access return was FORBIDDEN.

As you mentioned it might be related to the wrongly provided access dependency for the inline blocks.

Do you have any quick idea if this patch can be amended in a way that it handles translation as well?

Seeing the same issues with paragraphs, however patch #12 didn't fix it for me, #3 did though!

This seems similar to a problem I'm having on drupal core 8.7.8 with layout builder, custom block content, paragraphs and node revisions with moderation. Everything is fine up until someone makes a draft revision off of a published node, and then makes edits to one of the custom paragraph blocks in the layout builder. The block vanishes from the published "current revision" and then becomes inaccessible within the layout builder in the "latest revision" where the contents do not render, and any attempt to edit the block leaves the user with an alert message that they do not have permissions to edit or delete the block. Obviously the code is pretty different in 8.7.8 so these patches wouldn't install. I'm relatively new to the code behind layout builder but hope to figure out a work around.

Patches #7 and #12 worked to fix the permission issue that locked a content editor out of making changes to the paragraph in the layout builder, thanks so much for the patch. But those edited paragraphs on the "latest revision" still don't seem to be rendering on the published revision. Would that be a separate issue from the one in this thread? I'm currently wading through objects in xdebug looking for the cause.

it turns out I was experiencing two different bugs:
The access issue fixed in the patch here on this thread

and

An additional access issue stemming from the paragraph module, where the module does not track the correct revision ID of it's parent, so the permission checks for rendering fail on account of the parent's most recent revision ID always being passed (by default) to the permission check. This is a problem if you're using paragraphs wrapped in a custom block so that you can use them within layout builder.

So this patch solved the problem where we were unable to edit the blocks if we have a current and latest revision juggled at the same time.

the paragraph issue seemed to prevent paragraphs from rendering on the current and latest revisions if you made edits to them in the latest revision. In case anyone else has run into that, it seems related to this issue on paragraphs:
https://www.drupal.org/project/paragraphs/issues/2949412

I've managed to fix my issue where the edited paragraphs were not being rendered (due to access check failing) on the published revision. But noticed the content still wasn't appearing in the layout builder with "preview" mode checked.

I don't know how to deal with the scenario that a user has the view latest version permission and may be using the 'view' operation on either the active or canonical revision, but for now it might be okay to assume the 'active' version maps to 'update' and 'canonical' maps to view.

Would it be safe to assume whenever the layout builder is being viewed, that by default, we don't need the canonical version of the node, but always the active version? Since the layout builder is always used for editing purposes and we need the editable version (regardless of whether we're in preview mode or not)?

In my edge case, with the code as is, the paragraph does not render in the layout builder preview when "Show content preview" is checked because getInlineBlockDependency is loading the canonical entity for any "view" operation, which means the wrong revision IDs are going to be passed to the access check. This is the scenario I'm running into with the layout builder preview- the content of my edited paragraphs in custom blocks don't render in the layout builder preview unless I change getInlineBlockDependency to use getcanonical getActive for the "view" operation.

Hmm, this patch breaks my implementation I have in https://www.drupal.org/project/layout_builder_at/ where I override the SetInlineBlockDependency class so I can call get getTranslationFromContext on the entity which solved an issue with paragraphs where fields were not editable anymore. Of course, the class is internal, so following to see what the outcome is of this, and more specifically if it fixes the original problem I had as well.

Coming from #3090261: SetInlineBlockDependency override might no longer be needed. since this patch breaks the implementation. Will definitely test more and report back.

Attaching patch which addresses @Poindexterous's point on needing to use isActive for successful rendering of paragraph content when content preview is checked on the layout builder override edit page.

This at least allows the paragraph content to render. In combination with the patch from https://www.drupal.org/project/paragraphs/issues/3090200 this is 99% working for me. However, I have run into another situation where I revert to an old revision, edit the layout, choose to edit a block who's paragraph content has changed and am told "You are not allowed to edit or remove this Paragraph." for each paragraph that changed. One step forward, one step back...

I wanted to update this thread since I'm trying to paint a complete picture. I can confirm that with the patch from #26 here and the patch in https://www.drupal.org/project/paragraphs/issues/3090200#comment-13340694, I'm able to have the experience one would expect when working with Layout Builder + Content Moderation + Paragraphs on Block Content.

• I'm always seeing rendered paragraphs.
• I don't get any access errors when editing inline block components with paragraphs.
• I can revert to a previous revision that contains differences in paragraph content, see the correct revision when viewing the layout builder UI, and edit the inline block component that hosts the altered paragraphs without access errors.

This is a cross-post of my comment here: https://www.drupal.org/project/paragraphs/issues/3090200#comment-13364406

I'm writing to confirm that @lpeabody is correct: using both patches resolves the issue described in #3090200.

Confirming #28, needed both patches for everything to work nicely.

Unfortunately I spoke too soon, something in this combination of patches is causing weird behaviour. When I edit a draft and add a new block (reusable or otherwise) it seems to hide the other block in the layout that was already in the draft before even hitting save. I'm going to debug into things and try and see what's happening.

EDIT: Debugging into this, it's because when adding or removing a block, the AJAX response is no longer considered a layout_builder route by this logic

+++ b/core/modules/layout_builder/src/EventSubscriber/SetInlineBlockDependency.php
@@ -126,6 +135,12 @@
+    $is_layout_builder = $current_route->getOption('_layout_builder');
+    if ('view' == $operation && $is_layout_builder) {
+      $active_operations[] = 'view';
+    }

Which means the canonical layout entity is checked which results in isBlockRevisionUsedInEntity returning FALSE.

I've gone back to #12 to see if I can track down what my initial issues were with that patch and these are my findings:

WITH PATCH 12 APPLIED
- After creating a Draft of a previously published node, my inline block no longer displays the paragraph field on the published version of the node. It does however show on the Draft version but NOT when editing the layout (i.e the paragraph only shows on /node/123/latest, no where else).
- Debugging into the ParagraphAccessControlHandler I saw the reason access was denied was recorded as Non-reusable blocks must set an access dependency for access control.
- Debugging into SetInlineBlockDependency::getInlineBlockDependency I found isBlockRevisionUsedInEntity was returning FALSE
- Debugging into isBlockRevisionUsedInEntity I found that getInlineBlockRevisionIdsInSections was returning a single valued array, of which the value was the block's original revision id (8 in this case)

BUT the block that is being checked against has a revision id of 9, because it had been updated when I created that draft. The block_content table is showing that revision_id too, so does that mean that the changes made in the Draft version have incorrectly been updated on the latest version of that block?

EDIT: The answer to that I think is...maybe? But the issue here is that Paragraphs only tracks the parent id, not the revision id. So in it's access handler it's calling $paragraph->getParentEntity() and loading the current revision of the block rather than the old revision that the published version is showing.

Further to #12, applying the patch from https://www.drupal.org/project/paragraphs/issues/3090200#comment-13364406 fixes the issue for the Published version, but the layout page is still busted. I'm assuming that's what the changes in #26 fix however that combination then leads to the weird behaviour I mentioned in #31.

Attached patch bypasses the `isBlockRevisionUsedInEntity()` which verifies the comments in #32. Testing locally confirmed the bypass allowed editing of paragraphs that were previously uneditable.

@ctrlADel can you please provide an interdiff?

+++ b/core/modules/layout_builder/src/EventSubscriber/SetInlineBlockDependency.php
@@ -122,16 +134,31 @@ public function onGetDependency(BlockContentGetDependencyEvent $event) {
     if ($this->isLayoutCompatibleEntity($layout_entity)) {
+      return $layout_entity;
+      // Check below bypassed as the parent entity is not correctly set so revisioning always returns false.

We definitely don't want to do this.

Continuing the monologue - Patch #3 still seems to be the only one that solves the issue with no weird edge case bugs for me. With that, I don't need the paragraphs patch over in #3090200: Paragraph view access check using incorrect revision of its parent, leading to issues viewing paragraphs when reverted host entities or content moderation is used either.

EDIT: The above is no longer true, I'm unable to get a working set of patches for LB + CM + Paragraphs

Adding a patch for 8.7.14 drupal core version to allow users to configure all custom blocks editing as inline in layout builder.

Echoing @acbramley that Patch #3 is the only patch which solves the restriction to blocks using paragraphs which are edited under content moderation. I am not using private files, so I cannot confirm the points made by @Sam152 in regards to #3 in the follow-up comment #4.

The other patch combinations, such as #26 + #11 from issue #3090200, resolve the "You are not allowed to edit or remove this Paragraph." error message, but the "You are not allowed to view this Paragraph." error message persists.

Tagging for BugSmash!

I created a related issue: #3169409: Paragraph permission issue on Layout builder

and created a patch based on #26

When update inline block that has paragraph reference, Block Content will missing AccessDependency, and then trigger Block Content access forbidden 'Non-reusable blocks must set an access dependency for access control.'

Update block will trigger ajax form submission, then in SetInlineBlockDependency, the event cannot get inline block dependency, because the current route is ajax, no layout builder route.

Rerolled the patch #44.

Resolving coding standard issues, deprecations and failing test cases

This error is effecting a lot of blocks in Layout Builder, essentially making them uneditable. Is there any hope of a patch that applies to 8.9.x? I tried to reroll the patch in #47 to apply to 8.9.13 but I end up causing this error instead when editing a Media field with Media Library enabled within a block added to Layout Builder:

"TypeError: Argument 1 passed to Drupal\layout_builder\EventSubscriber\SetInlineBlockDependency::__construct() must implement interface Drupal\Core\Entity\EntityRepositoryInterface, instance of Drupal\Core\Entity\EntityTypeManager given, called in /app/web/core/lib/Drupal/Component/DependencyInjection/Container.php on line 259 in Drupal\layout_builder\EventSubscriber\SetInlineBlockDependency->__construct() (line 83 of /app/web/core/modules/layout_builder/src/EventSubscriber/SetInlineBlockDependency.php)
#0 /app/web/core/lib/Drupal/Component/DependencyInjection/Container.php(259): Drupal\layout_builder\EventSubscriber\SetInlineBlockDependency->__construct(Object(Drupal\Core\Entity\EntityTypeManager), Object(Drupal\Core\Database\Driver\mysql\Connection), Object(Drupal\layout_builder\InlineBlockUsage), Object(Drupal\layout_builder\SectionStorage\SectionStorageManager))
#1 /app/web/core/lib/Drupal/Component/DependencyInjection/Container.php(173): Drupal\Component\DependencyInjection\Container->createService(Array, 'layout_builder....')
#2 /app/web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php(105): Drupal\Component\DependencyInjection\Container->get('layout_builder....')
#3 /app/web/core/modules/block_content/src/BlockContentAccessControlHandler.php(76): Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('block_content.g...', Object(Drupal\block_content\Event\BlockContentGetDependencyEvent))
#4 /app/web/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php(105): Drupal\block_content\BlockContentAccessControlHandler->checkAccess(Object(Drupal\block_content\Entity\BlockContent), 'update', Object(Drupal\Core\Session\AccountProxy))
#5 /app/web/core/modules/media_library/src/MediaLibraryFieldWidgetOpener.php(74): Drupal\Core\Entity\EntityAccessControlHandler->access(Object(Drupal\block_content\Entity\BlockContent), 'update', Object(Drupal\Core\Session\AccountProxy), true)
#6 /app/web/core/modules/media_library/src/MediaLibraryUiBuilder.php(211): Drupal\media_library\MediaLibraryFieldWidgetOpener->checkAccess(Object(Drupal\media_library\MediaLibraryState), Object(Drupal\Core\Session\AccountProxy))
#7 [internal function]: Drupal\media_library\MediaLibraryUiBuilder->checkAccess(Object(Drupal\Core\Session\AccountProxy), Object(Drupal\media_library\MediaLibraryState))
#8 /app/web/core/lib/Drupal/Core/Access/CustomAccessCheck.php(75): call_user_func_array(Array, Array)
#9 [internal function]: Drupal\Core\Access\CustomAccessCheck->access(Object(Symfony\Component\Routing\Route), Object(Drupal\Core\Routing\RouteMatch), Object(Drupal\Core\Session\AccountProxy))
#10 /app/web/core/lib/Drupal/Core/Access/AccessManager.php(159): call_user_func_array(Array, Array)
#11 /app/web/core/lib/Drupal/Core/Access/AccessManager.php(135): Drupal\Core\Access\AccessManager->performCheck('access_check.cu...', Object(Drupal\Component\Utility\ArgumentsResolver))
#12 /app/web/core/lib/Drupal/Core/Access/AccessManager.php(112): Drupal\Core\Access\AccessManager->check(Object(Drupal\Core\Routing\RouteMatch), Object(Drupal\Core\Session\AccountProxy), Object(Symfony\Component\HttpFoundation\Request), true)
#13 /app/web/core/lib/Drupal/Core/Routing/AccessAwareRouter.php(109): Drupal\Core\Access\AccessManager->checkRequest(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Session\AccountProxy), true)
#14 /app/web/core/lib/Drupal/Core/Routing/AccessAwareRouter.php(94): Drupal\Core\Routing\AccessAwareRouter->checkAccess(Object(Symfony\Component\HttpFoundation\Request))
#15 /app/vendor/symfony/http-kernel/EventListener/RouterListener.php(113): Drupal\Core\Routing\AccessAwareRouter->matchRequest(Object(Symfony\Component\HttpFoundation\Request))
#16 [internal function]: Symfony\Component\HttpKernel\EventListener\RouterListener->onKernelRequest(Object(Symfony\Component\HttpKernel\Event\GetResponseEvent), 'kernel.request', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher))
#17 /app/web/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php(111): call_user_func(Array, Object(Symfony\Component\HttpKernel\Event\GetResponseEvent), 'kernel.request', Object(Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher))
#18 /app/vendor/symfony/http-kernel/HttpKernel.php(127): Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.request', Object(Symfony\Component\HttpKernel\Event\GetResponseEvent))
#19 /app/vendor/symfony/http-kernel/HttpKernel.php(68): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1)
#20 /app/web/core/lib/Drupal/Core/StackMiddleware/Session.php(57): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#21 /app/web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#22 /app/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#23 /app/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#24 /app/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#25 /app/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(52): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#26 /app/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#27 /app/web/core/lib/Drupal/Core/DrupalKernel.php(708): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#28 /app/web/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#29 {main}"

Ok, here's a version that applies to 8.9.13. I would not apply it because it causes the error I mentioned previously, but maybe we're a step closer now.

The error in #49 is only possible if you apply the patch but do not rebuild the container (clear caches).

Also, I understand that you are still on D8 and haven't updated to D9 yet. But posting 8.9 patches without 9.2 patches just confuses the testbot and makes the issue take longer to resolve.

Thanks Tim! After clearing caches I no longer get the TypeError. Sorry about the lack of D9 patch.

Unfortunately the TypeError was masking the fact that this patch doesn't solve the original error I was getting which caused me to apply the patch in the first place. Maybe my issue may be more related to Media Library than Layout Builder? I ended up here because this issue is one of the only ones to mention the "Non-reusable blocks must set an access dependency for access control." error.

My original error is below. Sorry if this is actually completely unrelated to this issue. I don't mean to derail the conversation, but any thoughts would be appreciated.

"Path: /media-library?destination=/node/11549/layout&_wrapper_format=drupal_ajax&ajax_form=1&media_library_opener_id=media_library.opener.field_widget&media_library_allowed_types%5Bimage%5D=image&media_library_selected_type=image&media_library_remaining=1&media_library_opener_parameters%5Bfield_widget_id%5D=field_bg_image%3A-settings-block_form&media_library_opener_parameters%5Bentity_type_id%5D=block_content&media_library_opener_parameters%5Bbundle%5D=hero&media_library_opener_parameters%5Bfield_name%5D=field_bg_image&media_library_opener_parameters%5Bentity_id%5D=302&media_library_opener_parameters%5Brevision_id%5D=790&hash=I7mUlnkXpcBUh5GqHy3zGuxBQwb_UcDn9Mb9ABZ9zd4&views_display_id=widget&_wrapper_format=drupal_ajax. 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/web/core/lib/Drupal/Core/Routing/AccessAwareRouter.php)."

#47 along with https://www.drupal.org/project/paragraphs/issues/3090200#comment-13738216 seems to be working pretty well for Blocks inside LB with paragraph fields (haven't yet tested nested paragraphs).

Have tested reverting drafts, re-publishing etc and all is working as expected. I did not need the patch from #3169409: Paragraph permission issue on Layout builder

FWIW I've found for my particular issue that manually recreating blocks affected by this "Non-reusable blocks must set an access dependency for access control." error fixes them (at least for now). By manually recreating I mean manually delete the block via Layout Builder and recreate it. This is not ideal, but it's worth a shot for anyone who this patch doesn't work for. In fact, recreating the block fixes the issue with or without the patch applied, so I don't know what to make of that. Still I'm going to trust that this patch is doing something and deploy it anyways.

+++ b/core/modules/layout_builder/src/EventSubscriber/SetInlineBlockDependency.php
@@ -122,15 +147,28 @@ public function onGetDependency(BlockContentGetDependencyEvent $event) {
+    $is_layout_builder = $current_route->getOption('_layout_builder');

This blows up when not in a route context e.g when calling \Drupal::entityTypeManager()->getAccessControlHandler('block_content')->access($block, 'update'); directly when $block is an inline block.

I think since this is all being tested in Kernel tests it's getting a /<none> route so it isn't being exposed.

I've run out of time to figure out tests for this but here's an updated patch.

This applies to 9.2.x as well. Setting to NR for existing tests.

Fixes PHPCS

#56 Patch Failed to Apply. So reroll the patch for 9.2.x, kindly review.

No changes to the 9.2.x patch, but reattaching it to this comment as per #51. Also attaching 8.9.x, 9.0.x, and 9.1.x backport patches so we can get tests going for those.

This comment changes the 8.9.x and 9.0.x backport patches to install the schema for key_value_expire.

The same change is not desired for 9.1.x and 9.2.x because of this change record.

@maskedjellybean I think you are looking for #3124302: The media library should perform access checks against the revision of the entity being edited, I had the same error and found this issue first but looks like that's the actual culprit where media library is involved.

Thanks @pameela! It sounds like this was supposedly resolved 6 months ago, so I wonder how we're experiencing this error now. I'll comment on that thread to add that this happened recently. Maybe the issue is not actually resolved?

This is still 'Needs tests' per #55

@larowlan: apologies, I skipped right over that... In any event, the existing test code looks good to me.

@acbramley: is it just the AJAX control flow that's left to be tested in SetInlineBlockDependency::onGetDependency()? Everything in SetInlineBlockDependency::getInlineBlockDependency() seems to be covered as best as I can tell.

I'm more than happy to try to develop the outstanding tests; just want to be sure I've got a complete list before I dive in.

@clayfreeman we just need a case that goes through getInlineBlockDependency when getRouteObject returns NULL

Attaching updated interdiff + backport patches first to avoid confusing testbot. Will attach the 9.2.x patch in my next comment.

This interdiff addresses #66.

+++ b/core/modules/layout_builder/tests/src/Kernel/SetInlineBlockDependencyTest.php
@@ -71,6 +72,35 @@ protected function setUp(): void {
+    $block->access('view', $account);

Are we missing an assert here?

@acbramley not necessarily; we're asserting the value of the access check elsewhere, so it doesn't make sense to duplicate that logic in this new test case. If all we're wanting to test is that an access violation doesn't occur due to the route object being NULL, then that type of failure should fall through and be reported by PHPUnit simply by calling any of the access checks.

I will note, however, that our test case could be bolstered by checking whether the route object is actually being retrieved throughout the course of the access check; this gives us some assurance that the test case could properly find such a bug if one were to exist. I've attached this change to this reply.

@clayfreeman yup that's a good approach!

I've been testing this and everything seems to be working well with one exception.

Might contrib modules like slick_paragraphs and/or slick_entityreference need to do something special to compensate as well?

https://www.drupal.org/project/slick_paragraphs/issues/3199638
https://www.drupal.org/project/slick_entityreference/issues/3205012

I haven't dug in deeply, but wondered if those other two issues might be directly or indirectly related to this.
Hoped that someone closer to the issues might be able to chime-in/comment.

Rebase on 9.3.x, convert to MR for easier iteration & review.

Tested #71 as we are still on 9.2 but it seems to be working for us! Thanks a lot!

Left some observations

Added some comments to the MR as follow-up to #80. These probably need answers before any additional revisions to the code can be made.

Patch format for use in composer. Tested on 9.2.8

Edit: Ignore the fact that the filename seems to be for another issue. It was a copy paste error from me on the filename only, not the patch :D

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Can the MR be updated for 10.1 please.
Left some comments on the MR 1106 also.

If we do end up changing the parameters for the eventSubscriber we will need a change record also.

Hiding the files to avoid confusion as the solution is being worked on in the MR.

The merge request does not appear to work in the scenario where a media item is being added to a field in an inline block.

The problem is that this line does not work with future revisions:

      $layout_entity = $this->entityRepository->getActive($layout_entity_info->layout_entity_type, $layout_entity_info->layout_entity_id);

We either need a different method to get the newest revision instead of the active revision, or pass the appropriate context definition to getActive() so it gets the correct revision.

I've run into a similar problem where the block_content entity has translations (because of core patch which adds symmetric translations to layouts). The media library widget was unable to update the media file once a translation was created. It would only work for the translation, but not the original language.

Checking block usage with latest revision ID of the block_content entity won't work, because the section storage stores the latest translation affected revision ID in the layout. I'm not sure if this is a problem which should be taken into account here or solved elsewhere. But considering that translation support will be added to core at some point, it might be worth fixing it here.

I've attached a patch which uses getLatestTranslationAffectedRevisionId on the entity storage to work around this (requires changes from the MR to be applied first). I can push it to the MR if this is useful, but since it needs entityTypeManager I wanted to check first if there isn't a better solution to the problem.

EDIT: In our case, translations are added via tmgmt using another patch #3097660: Add layout builder (inline) block support. It's possible that the revision IDs aren't properly updated. I haven't tried to reproduce this after manually translating content, yet.

The merge request does not appear to work in the scenario where a media item is being added to a field in an inline block

Could you elaborate or provide steps to reproduce?

Unrelated, but after spending a lot of time reviewing this problem area, isn't the best solution here for layout builder to track what entity revision ID an inline block is attached to, instead of just its ID? If we had that in place, then there wouldn't be a need for us to have all this logic in place to check if we should be trying to load the canonical revision vs the active revision. We'd know exactly which revision to load. Maybe that's not compatible with translations or something?

I guess that's a bit more complicated to accomplish and requires more refactoring though.

The problem is that this line does not work with future revisions:

$layout_entity = $this->entityRepository->getActive($layout_entity_info->layout_entity_type, $layout_entity_info->layout_entity_id);

We either need a different method to get the newest revision instead of the active revision, or pass the appropriate context definition to getActive() so it gets the correct revision.

That code should be getting the latest revision. If you have a future revision, isn't that considered the latest revision? In what scenario is it not?

@bkosborne getActive will get the latest draft/translation - the naming isn't great, but the docs on the interface are good.

Rebased off 11.x

As the view operation only assertTrue if the route has option "_layout_builder". So, the expected is to assertFalse for view and True for update and delete.
@larowlan correct me if I'm wrong.
Please review.

Not sure the change to false should be done. Seems like an odd test if you can update/delete but can't view.

I believe this should be re-triaged as major, if not critical.

Once content enters this state, it becomes impossible for content managers to update certain parts of it unless:

1. The content is saved as a default revision (this is not a valid work-around for many situations)
2. The troublesome blocks are removed and replaced with new blocks end-user confirmed, but still data loss?

https://www.drupal.org/project/drupal/issues/3053881 -- which I was also involved in -- was promoted to critical due to the same sort of potential for a "data-loss-like" condition.

As a follow-up to my triage recommendation, this flaw can be reproduced via only core modules in common configurations.

1. install layout builder, node, media, media library, and content moderation
2. Add a block type with a media reference field, assigning the media library widget in the form display settings
3. enable layout builder on the article content type and enroll it in content moderation
4. Create a new node and edit the layout, adding an inline block that references a media item
5. Save as draft
6. Edit the inline block and attempt to swap out the media, notice that it fails with an AJAX error (non-reusable blocks must set an access dependency for control).

Wonder if the media issue is related to #3106315: Block content permissions required to select or upload new Media with media library when using Layout Builder? What is the AJAX error?

The AJAX error contains...

non-reusable blocks must set an access dependency for control

when going to insert the new media.

Note - I updated the previous comment to keep all the details neatly packaged, thanks for pointing out the missing info! :-)

Yow. I can reproduce #98 as user 1 with full admin privileges.

JS console error:
ResponseText: {"message":"Non-reusable blocks must set an access dependency for access control."}

Watchdog:

Path: /media-library?destination=/node/4/layout&_wrapper_format=drupal_ajax&ajax_form=1&media_library_opener_id=media_library.opener.field_widget&media_library_allowed_types%5Bimage%5D=image&media_library_selected_type=image&media_library_remaining=1&media_library_opener_parameters%5Bfield_widget_id%5D=field_image%3A-settings-block_form&media_library_opener_parameters%5Bentity_type_id%5D=block_content&media_library_opener_parameters%5Bbundle%5D=image&media_library_opener_parameters%5Bfield_name%5D=field_image&media_library_opener_parameters%5Bentity_id%5D=2&media_library_opener_parameters%5Brevision_id%5D=2&hash=RZpw_Ue-CORgfs34fAruBGXBLZUhx_qIoGwlq0UPRZU&views_display_id=widget&_wrapper_format=drupal_ajax.

Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException: Non-reusable blocks must set an access dependency for access control. in Drupal\Core\Routing\AccessAwareRouter->checkAccess() (line 118 of /var/www/html/public/core/lib/Drupal/Core/Routing/AccessAwareRouter.php).

Current MR applies to 10.1.2 and fixes it. Not able to test in 11.x at present sorry.

Addressed all remarks on the MR.

Some small change requests. Also for the deprecation will need a change record and that link pasted into the deprecation messages.

Thanks for picking this one back up!

Note that there are at least two contrib modules (just in our current project) that will need changes to avoid "Declaration must be compatible with Drupal\layout_builder\EventSubscriber\SetInlineBlockDependency::getInlineBlockDependency(Drupal\block_content\BlockContentInterface $block_content, string $operation): ?Drupal\Core\Access\AccessibleInterface":

• Layout Builder Asymmetric Translation (patch at https://www.drupal.org/project/layout_builder_at/issues/3090261)
• Workspaces Extra (no patch yet)

I have tested the latest version of the MR and it works well in Drupal 10.1.7

Uploading the latest version of the MR in a patch.

Changed the version by mistake.

As mentioned in #106, I currently have an issue with:

"Node access should be granted, but the node is assigned to NULL, since the associated revision IDs are checked against the default entity, not the active entity."

I was able to reproduce the bug and the patch solved my issue on a vanilla install.

However, on different projects where we are using the module Layout Builder Asymmetric Translation, we can't use this current patch.

Hello there.
This might be related: https://www.drupal.org/project/drupal/issues/3442910
I hope it helps.

This bug does actually affect Media library fields in Blocks as well, #3124302: The media library should perform access checks against the revision of the entity being edited did not fix it.

While the Block Content revision is now loaded in MediaLibraryFieldWidgetOpener, and ->access() is called on the revision, SetInlineBlockDependency::getInlineBlockDependency still just loads the default revision of the parent of the block (e.g the Node) and checks if that block revision exists in the Node, which it doesn't when you're editing a forward revision of the Node (because that'll have different block revision ids). In fact, #3124302: The media library should perform access checks against the revision of the entity being edited probably caused this regression with media library because it used to check the default revision of the block which would have been in the default revision of the node!

Rebased, fixed tests and linting, added CR. This is ready for review.

@acbramley - The latest updates done are making this MR diff (applying it as a patch) not work now with Drupal 10 environments. When trying to apply the diff as a patch on Drupal 10.4.1, Composer throws an error saying that the patch cannot be applied.

For anyone needing to apply this as a fix for Drupal 10, I've been able to go and create a patch file that can be used.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Hiding patches so they don't trip the bot.

In an attempt to refactor the complicated getInlineBlockDependency function, I noticed there was no test coverage added for the code that was inside the case when $layout_entity_info was empty.

This code was added in the following 2 commits

https://git.drupalcode.org/project/drupal/-/merge_requests/1106/diffs?co...
https://git.drupalcode.org/project/drupal/-/merge_requests/1106/diffs?co...

I tried to debug various ways of adding/updating blocks with media library in them and hit a breakpoint inside that code but I was unable to.

The first commit mentions fixing #98 and #101 but, again, I was unable to hit that code when following those steps. That is, without that code I am able to successfully update a media reference via media library on an existing block in a draft revision of a node.

Without test coverage, it's hard to figure out what the code was actually doing it so I have reverted it.

If anyone is able to provide steps to reproduce the issues that this was fixing, and those steps still fail on the latest version of this branch, please let me know!

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Quick reroll of #1106 so it applies to 11.2 for a stable composer patch.

Merged and fixed conflicts. Hiding patches so it doesn't trigger the nr bot in the future.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Fixed up the MR for missing attribute and rebased as it got hit with the cspell bug.

Believe all feedback for this one has been addressed.

Excited to see this ready to commit! I verified this resolves the following scenario:

1. Content moderation enabled for Page content type
2. Layout builder & layout overrides enabled for Page content type
3. Content block with a paragraph field on it
4. User creates a new Page node, edits layout, and adds the block with paragraph field on it. Saves as published.
5. User edits layout and makes changes to the block & paragraph fields and saves a new draft (forward revision).
6. User goes to edit layout again, they are not able to edit the paragraph fields at all. RESOLVED with this MR

Note there's another issue here where the paragraph is no longer viewable on the active/default revision of the node once the forward revision is created, but that is a separate issue specifically related to Paragraphs: #3090200: Paragraph view access check using incorrect revision of its parent, leading to issues viewing paragraphs when reverted host entities or content moderation is used

There is also this issue https://www.drupal.org/project/drupal/issues/3442910 that isn't solved by this MR because we still go out from the fact that the block is revisionable which is not always the case, so I think we need to add an extra condition to actually check that that block is revisionable.

Forget my last comment. Even if the block is not revisionable, there is a revision id, so it works with new blocks. Somehow there is an issue in my database after an update.

Patch resolves the issue we were getting re "Non-reusable blocks must set an access dependency for access control" error when adding media to blocks in Layout Builder if a draft revision is the one being edited. What's left to do to merge this bug fix into core?

Noticed the deprecation message and started a separate issue for that at #3564173.

What's left to do to merge this bug fix into core?

Once it's at RTBC, it's up to the committers. Right now, the RTBC queue is over 100 issues, so basically this issue will have to wait its turn.

For what its worth, I'm on 11.2.8 and the patch from #129 works great for me. Thank you!

An additional scenario this fixes that does not require contrib modules:

1. Have a layout builder overrides enabled content type using content moderation.
2. A custom inline block type with a media reference field.
3. Create a published instance of the content type.
4. Add an instance of the inline block type with the referenced media in a draft revision on the content and save.
5. As a user without admin content/publishing permissions who does have editing permissions, attempt to replace the referenced media in the inline block that only exists in the draft revision.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Part of new Bugsmash targets we are triaging RTBC issues.

I rebased this one, upped the deprecation version for 11.4, and tweaked the CR. Don't see anything to warrant sending back to NW.

Reviewed the MR and posted a comment.

We've been using this patch in the MR for the past 6 months as it was causing a lot of problems - I know the MR needs work but so far it's been solid.