In #2004286: Defer calls to drupal_get_* functions until printed inside a template by adding a RenderWrapper class the below code was added to defer region rendering:

  $variables['page_bottom'] = array();
  if (isset($variables['page']['page_bottom'])) {
    $variables['page_bottom'] = drupal_render($variables['page']['page_bottom']);
  }

  // Add footer scripts as '#markup' so they can be rendered with other
  // elements in page_bottom.
  $footer_scripts = new RenderWrapper('drupal_get_js', array('footer'));
  $variables['page_bottom'][] = array('#markup' => $footer_scripts);

If $variables['page']['page_bottom'] is set by a module, drupal_render() renders it into a string. This means that the second part with $variables['page_bottom'][] fails with "Fatal error: [] operator not supported for strings".

CommentFileSizeAuthor
#5 2056837-render-page-bottom-5-test-only.patch1.41 KBlongwave
#5 2056837-render-page-bottom-5.patch1.98 KBlongwave
#1 2056837-render-page-bottom.patch579 byteslongwave

Comments

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review
StatusFileSize
new 2056837-render-page-bottom.patch579 bytes

Attached patch seems to fix the issue but not sure it is the right approach; should I be using RenderWrapper here? This also needs a test.

fabianx’s picture

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

Nice patch, works for me.

This obviously is right.

=> RTBC

This is a good candidate for "defer rendering" and not drupal_render directly (#2046881: [meta] Avoid "early rendered" strings where beneficial to do so, build structured data to pass to drupal_render() once instead).

However lets get this regression / bug fixed first and then work on that.

fabianx’s picture

fabianx commented
Issue tags: +Quick fix, +Twig, +theme system cleanup

Tagging

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

We should have test module test this functionality. Perhaps we can use the existing theme_test module.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
StatusFileSize
new 2056837-render-page-bottom-5.patch1.98 KB
new 2056837-render-page-bottom-5-test-only.patch1.41 KB
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review
jessebeach’s picture

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

I just ran into this issue and solved it in a near similar way.

// Add footer scripts as '#markup' so they can be rendered with other
// elements in page_bottom.
$footer_scripts = new RenderWrapper('drupal_get_js', array('footer'));
$variables['page']['page_bottom'][] = array('#markup' => $footer_scripts);

$variables['page_bottom'] = drupal_render($variables['page']['page_bottom']);

I'm fine with the approach taken in this issue.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Nice test @longwave. Thanks.

Committed 0d9a413 and pushed to 8.x. Thanks!

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