I'm sure this needs a lot of work, but here is a first try.

CommentFileSizeAuthor
#6 519782-6-page_bottom.patch5.06 KBtic2000
#6 519782-6-page_bottom_test.txt1 KBtic2000
make-hook-footer-use-rendarable-arrays.patch3.52 KBtic2000
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Having a hook specific to altering a tiny bit of the page output (hook_footer_alter()) sounds ridiculous. I'd rather see modules add the footer via page_alter() to page['closure'] and then require the themes to output closure from there.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
Status: Needs review » Needs work

i think modules can just populate a footer region during hook_page_alter() .... agreed that there is no need for any special alter function here, nor for drupal_render_footer(). let drupal_render_page() do its thing.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

http://drupal.org/node/511284#comment-1813776 would also support this idea with standard tools like hook_page_alter() and standard time rendering, with all the other block regions.

tic2000’s picture

tic2000 CreditAttribution: tic2000 commented

If it works for something else than regions then is great. I'll modify the patch to change $closure to $html_bottom and remove hook_footer for good.

tic2000’s picture

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

I changed my mind, I'll let the toolbar patch to take care of this, or if it doesn't I'll do it in the html.tpl.php patch.

tic2000’s picture

tic2000 CreditAttribution: tic2000 commented
Title: Change $closure to become a hidden region like page_top » Make hook_footer use rendarable arrays
FileSize
519782-6-page_bottom_test.txt1 KB
519782-6-page_bottom.patch5.06 KB

I see that #469242: Move <head> outside page.tpl.php it's in "lets discuss this" phase and I would not make $closure a region there anyway so I reopen this with a new patch.
This patch does the following:
1. Change $closure to $page_bottom.
2. Makes page_bottom a system hidden region.
3. Removes hook_footer because we will now use hook_page_alter to add/change content.
4. Because of 3 also removes theme_closure function which is no longer used.
5. Now drupal_get_js('footer') is called inside process_page and not preprocess_page which solves the bug reported in #510108: Call theme closure function in process_page, not preprocess_page and will make the tests I provide in #522782: Test #attached pass.

The patch in txt format is just for testing purpose. It moves the toolbar in page_bottom and adds the javascript in 'footer' scope.

tic2000’s picture

tic2000 CreditAttribution: tic2000 commented
Title: Make hook_footer use rendarable arrays » Change $closure to become a hidden region like page_top
Status: Closed (fixed) » Needs review
moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented
Title: Make hook_footer use rendarable arrays » Change $closure to become a hidden region like page_top

looks like a solid cleanup to me. lets await some more feedback then i will rtbc.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

Looks great to me too. Do we need to add a note about this to the docs for hook_page_alter() or just let the upgrade guide cover this particular case?

tic2000’s picture

tic2000 CreditAttribution: tic2000 commented

I think the documentation should describe what the function does, not what it replaces. So I would say the upgrade guide should cover it.

catch’s picture

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

Sounds sensible to me. RTBC.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Nice clean-up. Thanks!

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Fixed » Needs work
Issue tags: +Needs documentation

Needs update docs.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Make sure to mention that themes are now required to have a page_bottom region to avoid notices like screenshot at http://drupal.org/node/484860#comment-1861274

tic2000’s picture

tic2000 CreditAttribution: tic2000 commented

I would happily do this if you want them in Romanian. If I do it in English you would need then an English to English translation. :)

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Added this doc at http://drupal.org/update/modules/6/7#hook_footer:

(issue) Drupal 6 used hook_footer() to add content to a special variable named $closure which was mandatory to put in themes to the bottom of the HTML body output. Drupal 7 generalizes this under hook_page_alter() and hidden regions.

Drupal 7 now comes with two hidden default regions internally named page_bottom and page_top. They are hidden in that they will not show up in the blocks administration interface, but they can receive programmatic data. The corresponding template variables are $page_bottom and $page_top, and $page_bottom became the successor to $closure. Since you can use the hook_page_alter() function in Drupal 7 to add content to regions, hook_footer() and consequently theme_footer() were removed.

Drupal 6:

/**
 * Implementation of hook_footer().
 */
function example_footer($main = 0) {
  if (variable_get('dev_query', 0)) {
    return '<div style="clear:both;">'. devel_query_table() .'</div>';
  }
}

Drupal 7:

/**
 * Implement hook_page_alter().
 */
function example_page_alter($main = 0) {
  if (variable_get('dev_query', 0)) {
    return array(
      'devel' => array(
        '#type' => 'markup',
        '#markup' => '<div style="clear:both;">' . devel_query_table() . '</div>',
      ),
    );
  }
}

Note that you need to return a renderable array construct and not simply plain HTML. Ideally, you'd not pre-render the output of your code and return a fully renderable structure instead.

Although hook_footer() did not have a counterpart to add content to the top of the markup output, Drupal 7 adds the special $page_top region, which serves this purpose and can be used just like $page_bottom.

Still needs docs in the theme area, doing that too.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Uhm, a bit too much copy-paste there. Fixed D7 code to:

/**
 * Implement hook_page_alter().
 */
function example_page_alter(&$page) {
  if (variable_get('dev_query', 0)) {
    $page['page_bottom']['devel']= array(
      '#type' => 'markup',
      '#markup' => '<div style="clear:both;">' . devel_query_table() . '</div>',
    );
  }
}
Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Issue tags: -Needs documentation

Added this to the theme update docs at http://drupal.org/update/theme/6/7#closure

$closure becomes $page_bottom, new $page_top and hidden regions

(issue 1), (issue 2) Drupal 6 provides a special variable called $closure which should be put at the bottom of the HTML body output and can be themed via theme_footer() (which calls out to implementations of hook_footer() in modules). To generalize on one way to put output to the different page areas, Drupal 7 standardizes on regions and introduced the page_bottom region in place of the $closure special variable. Also, page_top is added as a pair of page_bottom. In Drupal 7 you need to output $page_top at the top of the HTML body output and $page_bottom at the bottom.

Drupal 6:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
  "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
...
<body class="<?php print $body_classes; ?>">
...
    <?php print $closure; ?>
</body>
</html>

Drupal 7:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML+RDFa 1.0//EN"
  "http://www.w3.org/MarkUp/DTD/xhtml-rdfa-1.dtd">
...
<body class="<?php print $classes; ?>">
  <?php print $page_top; ?>
...
  <?php print $page_bottom; ?>
</body>
</html>

If you define custom regions, it is important to remember that you need to include the page_top and page_bottom regions in your set of regions.

theme .info file extract:

regions[content] = Content
regions[help] = Help
regions[page_top] = Page top
regions[page_bottom] = Page bottom

The page_top and page_bottom regions are hidden, which means they will not show up on the blocks administration interface. When doing site-specific themes, it might also be useful to add more hidden regions (to provide ways for modules to add output to more places in the theme without defining blocks showing up on the blocks interface), you can do that via the regions_hidden[] .info file array which is new to Drupal 7:

theme .info file extract:

regions[content] = Content
regions[help] = Help
regions[page_top] = Page top
regions[page_bottom] = Page bottom
regions[indicators] = Indicators
regions_hidden[] = indicators

Still needs work because I found some issues in the default page.tpl.php which I'm going to submit a follow up patch.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs work » Fixed

Actually, opened #534414: Fix missing docs and consistency issues with $page_top and $page_bottom as a follow up, since $page_bottom was never documented (but $closure is still documented) in page.tpl.php and its markup is not consistent with $page_top.

Status: Fixed » Closed (fixed)

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

hass’s picture

hass CreditAttribution: hass commented

Removed, I should not open so many windows...