This patch moves the call to theme('closure') into process_page so that it runs later in the theming flow. This is necessary because other functions might add JavaScript to the footer scope which is not output correctly when closure is called that early.

CommentFileSizeAuthor
closure.patch751 byteskkaefer
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tic2000’s picture

tic2000 CreditAttribution: tic2000 commented

To make things a little bit clearer. In HEAD's current state you can't add javascript to $closure (footer scope) using #attached_js.
Like this for example (change in toolbar.module): 

  $build = array(
    '#theme' => 'toolbar',
    '#attached_js' => array(
      array('data' => $module_path . '/toolbar.js', 'scope' => 'footer', 'weight' => JS_LIBRARY + 2),
      array('data' => 'misc/jquery.cookie.js', 'scope' => 'footer', 'weight' => JS_LIBRARY + 2),
    ),
    '#attached_css' => array(
      $module_path . '/toolbar.css',
    ),
  );

This patch fixes the problem as advertised.
I feel bold so I will RTBC after bot gives it a run.

yched’s picture

yched CreditAttribution: yched commented

Mind giving a look at #469242: Move <head> outside page.tpl.php ? Seems really related.

joshmiller’s picture

joshmiller
he/him
Primary language English
Location Indianapolis, Indiana, USA
CreditAttribution: joshmiller commented
Status: Needs review » Needs work

I have checked the code syntax and all looks good. The bot is happy.

Does anyone think this needs a test to ensure that closure is called at the right time?

tic2000’s picture

tic2000 CreditAttribution: tic2000 commented
Status: Needs work » Needs review

It may touch the problem in the process, but I can't test if it will fix this problem because it needs a re-roll first.

tic2000’s picture

tic2000 CreditAttribution: tic2000 commented
Status: Needs review » Needs work

Crossposted.
I don't have an answer for the question.

tic2000’s picture

tic2000 CreditAttribution: tic2000 commented
Status: Needs work » Needs review

#469242: Move <head> outside page.tpl.php hit a dead end it seams so this should go in.
drupal_render has no test for #attached_css or #attached_js so if a test is needed it should be a new issue and not hold this one.

tic2000’s picture

tic2000 CreditAttribution: tic2000 commented

I opened #522782: Test #attached for tests.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented

No more dead end for #469242: Move <head> outside page.tpl.php. I suggest waiting on this a bit.

tic2000’s picture

tic2000 CreditAttribution: tic2000 commented
Status: Needs review » Closed (fixed)

#519782: Change $closure to become a hidden region like page_top got committed. This is no longer an issue.