Problem/Motivation
After #3339905: Add page_top and page_bottom to #attached to get rid of NodeThemeHooks::pageTop(), node preview adds the preview form through:
$build['#attached']['page_top']['node_preview']On a Drupal 11.4.1 site, node preview can fail with:
Error: Recursion detected in array_merge_recursive() (line 165 of core/lib/Drupal/Core/Render/BubbleableMetadata.php).The failure happens in:
Drupal\Core\Render\BubbleableMetadata::mergeAttachments()Debugging showed that both attachment arrays being merged contained the same keyed page region attachment:
#attached['page_top']['node_preview']
Because mergeAttachments() currently falls back to array_merge_recursive() for page_top and page_bottom, duplicate render arrays in those keys can trigger PHP recursion detection.
This does not appear to be content recursion. The recursion is in the attachment merge, not repeated entity rendering.
Steps to reproduce
Observed on Drupal core 11.4.1.
1. Have a node type with preview enabled.
2. Use a full view display with nested/rendered content, for example paragraphs/entity reference revisions and Display Suite.
3. Edit a node of that type.
4. Click Preview.
5. The preview route, e.g. /node/preview/{uuid}/full, returns a 500 error.
Observed error:
Error: Recursion detected in array_merge_recursive() (line 165 of core/lib/Drupal/Core/Render/BubbleableMetadata.php). Drupal\Core\Render\BubbleableMetadata::mergeAttachments() Drupal\Core\Render\BubbleableMetadata->merge() Drupal\Core\Render\RenderContext->update() Drupal\Core\Render\Renderer->doRender() Drupal\Core\Render\MainContent\HtmlRenderer->prepare()
In the failing request, the duplicate attachment was:
#attached['page_top']['node_preview']Proposed resolution
Handle page_top and page_bottom in BubbleableMetadata::mergeAttachments() before the generic array_merge_recursive(), similar to the existing special handling for placeholders.
These attachment keys are keyed page-region render arrays and should not be recursively merged when the same key exists on both sides.
Remaining tasks
- Add or confirm a failing test case for duplicate
#attached['page_top']attachments. - Review whether
page_bottomneeds identical handling. - Review whether keyed union is the correct merge behavior for duplicate page region children.
User interface changes
None.
Introduced terminology
None.
API changes
No public API changes intended.
Data model changes
None.
Release notes snippet
Fixes a fatal error on node preview where duplicate #attached['page_top'] attachments could cause recursion during render attachment merging.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 3611594-preview-page-region-recursion.patch | 1.51 KB | brandonlira |
| #2 | 3611594-preview-page-region-recursion.patch | 1.25 KB | brandonlira |
Issue fork drupal-3611594
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3611594-recursion-detected-when
changes, plain diff MR !16347
Comments
Comment #2
brandonlira commentedAttached patch prevents page_top/page_bottom attachments from being recursively merged by array_merge_recursive().
This fixed the node preview fatal locally on Drupal 11.4.1 where both attachment arrays contained #attached['page_top']['node_preview'].
Cheers!
Comment #4
brandonlira commentedComment #5
needs-review-queue-bot commentedThe 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.
Comment #6
brandonlira commentedComment #7
brandonlira commentedUploaded an updated patch.
Comment #8
smustgrave commentedThanks for reporting but as tagged this will need test coverage for sure.
Comment #9
brandonlira commentedHi @smustgrave,
I added test coverage for this case in BubbleableMetadataTest. The new test covers recursive render arrays in duplicate page_top/page_bottom attachments and is passing in the PHPUnit Unit (Core) job.
Could you please take another look when you have a chance?
Let me know if you think anything else should be adjusted.
Comment #10
smustgrave commentedRemoving tests tag based on https://git.drupalcode.org/issue/drupal-3611594/-/jobs/11131240 and manually reviewing the test.
admittedly I'm not great with this section of code but it appears to be well documented, change itself isn't massive, and in theory makes sense.
Going to mark and see what committers think.
Comment #11
quietone commentedComment #12
godotislate1 comment on the MR.
Comment #13
brandonlira commentedThanks @godotislate, that makes sense. I applied the suggested simplification.
Comment #14
smustgrave commentedThanks @godoislate, feedback appears to be addressed.
Comment #18
godotislateTest only fails as expected: https://git.drupalcode.org/project/drupal/-/jobs/11407530
Consulted with @catch to confirm this is a regression and should be backported to 11.4.x.
Committed and pushed t902684e to main, 5e98489 to 11.x, and 5439478 to 11.4.x. Thanks!
Comment #19
godotislate