Problem/Motivation

While rendering something in a post render callback, it is possible that this results in new post render callbacks that need to be processed.

Right now, they are added to the stack and there's a no-op operation that merges them together but nothing happens to them.

Specific example:

A poll rendered through entity_embed, both use post render callbacks for their output.

Proposed resolution

A do/while (yay!) loop around the relevant lines of code in Renderer, so that it we keep processing them until there are none left.

Remaining tasks

User interface changes

API changes

None, drupal_render() inside a post render callback works as expected again.

CommentFileSizeAuthor
#8 interdiff.txt2.45 KBwim leers
#8 post-render-callback-loop-2382667-8.patch11.04 KBwim leers
#5 interdiff.txt8.86 KBwim leers
#5 post-render-callback-loop-2382667-5.patch10.38 KBwim leers
#1 post-render-callback-loop-2382667-1.patch1.72 KBberdir

Comments

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Active » Needs review
StatusFileSize
new post-render-callback-loop-2382667-1.patch1.72 KB

First patch. This worked for my specific use case, did not check if it affected something else.

Will also need a comment to explain this.

Status: Needs review » Needs work

The last submitted patch, 1: post-render-callback-loop-2382667-1.patch, failed testing.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#2367479: Comment forms are not displaying anymore
fabianx’s picture

fabianx commented
Priority: Normal » Major
Issue tags: +Contributed project blocker

Nice! Exactly what we need, it is major though.

I would RTBC, but there are some pesky test failures and we need more test coverage ...

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new post-render-callback-loop-2382667-5.patch10.38 KB
new interdiff.txt8.86 KB

A do/while (yay!) loop

:)

The test failures were due to the fact that in HEAD, the executed #post_render_cache metadata is left in place, and the existing tests very meticulously testing that metadata. With the proposed patch, that capability is explicitly removed. Hence the test failures.
Adjusted the tests, now they pass.

fabianx’s picture

fabianx commented
Status: Needs review » Reviewed & tested by the community

And RTBC :).

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/system/src/Tests/Common/RenderTest.php
@@ -861,7 +865,7 @@ function testDrupalRenderRenderCachePlaceholder() {
+    $this->assertIdentical($cached_element, $expected_element); //, 'The correct data is cached: the stored #markup and #attached properties are not affected by #post_render_cache callbacks.');

Do you want the assertion message or not? :)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new post-render-callback-loop-2382667-8.patch11.04 KB
new interdiff.txt2.45 KB

D'oh! That's a test debugging technique I use; I didn't mean to remove the assertion message :)

Fun fact: I found this in two more places in RenderTest, even though it wasn't being introduced by this patch… rectifying those too.

Also: this needed a reroll anyway since #2382503: Not possible to render self-contained render array while a render stack is active landed.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

  • catch committed f6e69bc on 8.0.x 
    Issue #2382667 by Wim Leers, Berdir: #post_render_callback's that result...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#2470715: cacheGet-case: #post_render_callback's that result from other #post_render_calback are not processed

A related issue was just opened: #2470715: cacheGet-case: #post_render_callback's that result from other #post_render_calback are not processed .