This patch adds a hook_page_add() that is pretty much the same as hook_page_alter(), but runs before hook_page_alter().

This seperation between adding stuff to the $page array and altering these elements makes hook_page_alter() much easier to use. Currently, to alter e.g. the toolbar, your module has to have a higher weight as toolbar.module. This is limiting, as hook_page_alter() can force you to set weights that might be conflicting with other stuff your module does, and it can also easily create conflicts when lots of contrib modules start using hook_page_alter().

By adding hook_page_add(), these problems are solved. If your module just adds stuff to $page, it should use hook_page_add() so that these elements can easily altered by hook_page_alter() without having to have a higher module weight. If a module alters elements or adds elements that depend on the elements added by other modules, it uses hook_page_alter().

This is also similar to how we have hook_node_view and hook_node_build_alter: These two also do the same but run one after the other to be not dependent on module weight.

All core implementations of hook_page_alter() are changed to hook_page_add(), as none of them was actually altering things but only adding elements.

Would be great to get this in before the freeze, as it will make it much much safer for contrib to add and alter stuff at the page level.

CommentFileSizeAuthor
#4 hook_page_build.patch5.53 KBFrando
hook_page_add.patch5.51 KBFrando
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

Yeah, this will make life a bit easier.

webchick’s picture

I'm not sure about 'add'. This is not standard with any of our other hooks. For example, hook_node_view() is the name of the hook where you add stuff while the node is being viewed.

Maybe hook_page_build()?

webchick’s picture

Status: Reviewed & tested by the community » Needs review
Frando’s picture

Title: Add a hook_page_add() that runs before hook_page_alter() » Add a hook_page_build() that runs before hook_page_alter()
Status: Needs review » Reviewed & tested by the community
FileSize
5.53 KB

I like hook_page_build(). Renamed patch attached. Ready to commit IMO once the testbot gives green light.

moshe weitzman’s picture

build is non standard too. and less clear than add in this case. i agree we want consistency but not sure we can achieve that in this issue.

Frando’s picture

I don't really care whether it's _add or _build. For _add use patch in the OP, for _build patch in #4, both are RTBC. My vote is for _build.

pwolanin’s picture

Issue tags: +API change, +API addition

tagging

dropcube’s picture

+1, makes a lot of sense hook_page_build() to add building 'blocks' to the page being built, hook_page_alter() to alter the structure built by modules.

Note, that the approach proposed here http://drupal.org/node/516150#comment-1987010 for #516150: Add fallback for main content block rendering, is totally compatible with this. In other words, if any of the modules implementing hook_page_build() or hook_page_alter() ensures that the main content of the page is already added, then the module must set $page['#has_main_content'] = TRUE;, otherwise system.module provides it's fallback.

catch’s picture

I can live with hook_page_build() as a name. hook_page_add() sounds a bit like the hook that runs when adding a page if I didn't know the context.

moshe weitzman’s picture

OK, I am happy with build as well.

Status: Reviewed & tested by the community » Needs work

The last submitted patch failed testing.

catch’s picture

Status: Needs work » Reviewed & tested by the community
webchick’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs documentation

I agree, this makes a lot of sense.

Committed to HEAD! Please document in the module upgrade guide!

sun’s picture

Issue tags: -API change, -API addition

.

sun’s picture

Issue tags: +API change, +API addition

Still undocumented API addition/change.

jhodgdon’s picture

changing tag to new tagging scheme

sheilaj’s picture

Status: Needs work » Needs review

I added a section titled "Two new functions added: hook_page_build() and hook_page_alter()" to the update modules page (Converting 6.x modules to 7.x).

In the "Comment rendering overhaul" section, I added hook_page_build() where hook_page_alter() was mentioned.

I think the section "hook_footer() was removed, $closure became $page_bottom, $page_top added" also needs updating to include hook_page_build(), and maybe the code sample changed to use hook_page_build() instead of hook_page_alter().

jhodgdon’s picture

Status: Needs review » Needs work

I reviewed the added section and the comment rendering changes, and they look fine to me.

I agree that the hook_footer section needs an update.

One other spot that might need a mention of hook_page_build:
Menu "page callbacks" and blocks should return an array and hook_page_alter() (Render arrays discussion)

jhodgdon’s picture

Status: Needs work » Fixed

As suggested in #17, I added hook_page_build() to this section:
http://drupal.org/update/modules/6/7#hook_footer
and I changed the example to use this hook instead of page_alter.

I decided the page callbacks section mentioned in #18 did not need a mention of hook_page_build after all.

Status: Fixed » Closed (fixed)
Issue tags: -API change, -API addition, -Needs documentation updates

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