In #241570: Theme preprocess functions do not get retained when using patterns the 'original hook' index was added to the hook_theme() array so that preprocess functions would carry over to any pattern-based variations of themes. But there's no documentation of this new feature in hook_theme(), which means it's not used in places where it should be.

CommentFileSizeAuthor
#10 909954-document-base-hook.patch852 bytesbarraponto
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

voxpelli’s picture

I agree on this - subscribing.

jhodgdon’s picture

Version: 6.x-dev » 7.x-dev

This was only I think added to Drupal 7?

voxpelli’s picture

Version: 7.x-dev » 6.x-dev

It's very much in Drupal 6 - but not documentated

jhodgdon’s picture

Version: 6.x-dev » 7.x-dev

Then it needs to be documented in Drupal 7 first if it is in Drupal 7, and then the doc fix can be backported to Drupal 6.

jhodgdon’s picture

Status: Active » Postponed (maintainer needs more info)

I went and read through that referenced issue and the patches that were committed in it.

It looks to me as though the 'original hook' component is something used internally by the theme system, not something you are supposed to use yourself in hook_theme(). Also, in D7 it is apparently called 'base hook' and not 'original hook'.

So I'm not sure whether we really need to document this?

voxpelli’s picture

Status: Postponed (maintainer needs more info) » Active

It's needed for other modules to be able to theme a theme suggestion from another module without losing the original preprocess functions.

That's why Views is suggesting people to use it: http://views-help.doc.logrus.com/help/views/api-default-views

That's why bangpound suggested me to use it in: #890490: Formatter themes don't see preprocessor functions and don't see provider-specific themes.

At least this is true for Drupal 6 - seems like there has been some extra changes made to Drupal 7. Still needed to document this in D7 prior to D6?

voxpelli’s picture

#34 in #241570: Theme preprocess functions do not get retained when using patterns had the comment for the D7 patch: "It leaves open the ability for a hook_theme_registry_alter() to unset the 'original hook' key"

Sounds like to be able to do that you need to know about the existence of such a property?

In addition merlinofchaos who wrote the original patch flagged the change of 'original hook' to 'base hook' in D7 to be an API-change - something it only was if other modules is supposed to know about the hook.

I can't find anything in the original issue that either explicitly says that 'original hook' shouldn't be documented or implies that it shouldn't.

jhodgdon’s picture

OK.

Yes, we need to document D7 before we port changes to D6. We're generally trying to avoid having things fixed in D6 that haven't yet been fixed in D7, and that also applies to documentation.

Anonymous’s picture

Title: Document 'original hook' in hook_theme() » Document 'base hook' in hook_theme()
Version: 7.x-dev » 8.x-dev

Funny, I actually used 'base hook' just last week after discovering it in code, and was wondering why I'd never heard about it before.

At this point, it will be a docs change to D8, then D7, then D6.

barraponto’s picture

Status: Active » Needs review
FileSize
852 bytes

I could really use this documentation since 2009. I've been asking for directions on irc/stackoverflow since I always forgot how to do this. Let me scratch my own itch.

jhodgdon’s picture

Thanks for the patch! It seems to me to be well-written and clear documentation.

But I can't vouch for its accuracy -- if someone who knows more about this than I do could review it for accuracy, that would be good.

JohnAlbin’s picture

Status: Needs review » Reviewed & tested by the community

The Fences module may use this as well. I don't specifically recall.

I can confirm that the "base hook" in hook_theme is indeed what Barraponto documents. So the docs in the patch are correct, though I haven't applied the patch. We should let the testbot have a go at that, but its RTBC from me.

jhodgdon’s picture

Assigned: Unassigned » jhodgdon

Testbot says go. JohnAlbin says go. I'll get it committed next time I'm doing commits. Thanks!

jhodgdon’s picture

Version: 8.x-dev » 6.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: +Needs backport to D6, +Needs backport to D7

Patch committed to 8.x and 7.x. Needs backport to 6.x (with change: component is called 'original hook' in D6 apparently).

jhodgdon’s picture

Assigned: jhodgdon » Unassigned
Cameron Tod’s picture

Status: Patch (to be ported) » Needs review

If I'm not mistaken, the hook documentation for Drupal 6 is in the documentation project. I've created an issue over there and posted a patch.

#1817140: Backport: document 'original hook' in hook_theme() in Drupal 6

jhodgdon’s picture

Version: 6.x-dev » 7.x-dev
Status: Needs review » Fixed

Correct, thanks! So let's leave this at 7.x, fixed. I'll take care of the other issue.

Status: Fixed » Closed (fixed)

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

jay.dansand’s picture

Version: 7.x-dev » 8.x-dev
Status: Closed (fixed) » Active

The current documentation could be improved. On the API page: https://api.drupal.org/api/drupal/core%21modules%21system%21system.api.p... the documentation says only:

base hook: A string declaring the base theme hook if this theme implementation is actually implementing a suggestion for another theme hook.

I was attempting to implement exactly that - a hook suggestion for theme_image() - and the single sentence above left many questions unanswered; for instance, did I need to fully copy the base hook's 'variables' => array(...) in my hook's info? The answer in this case is "yes." But the documentation is entirely unclear.

As it turns out, when theme($hook, $variables) (in D7; the steps are basically the same for D8) is called base hook can have either no effect or greatly change the outcome of theme(). For the following, assume base hook = 'image' and hook = 'image__example'.

  1. If the base hook ('image' in our example) has any $info['includes'] files, they are included.
  2. If the base hook does not have any (pre)process functions, then it does nothing, except potentially including some files (see #1 above). Or, if the base hook ('image') does have preprocess or process functions, then:
    1. theme_hook_suggestion is set to 'image__example' (the original $hook parameter passed to theme('image__example', $variables) in our example), and $hook is changed to 'image' (the base hook).
    2. Any 'image' preprocess and process functions are run. Those (pre)process functions have the option to change theme_hook_suggestion (where 'image__example' is stored), preventing the original hook from ever firing.
    3. If the original hook 'image__example' remains as theme_hook_suggestion after the previous step, and if it has $info defined (which it is guaranteed to have, otherwise $info['base hook'] wouldn't have existed to start this whole process), then $hook is changed back from 'image' to 'image__example'.

I think the documentation should instead say something like:

base hook: A string declaring the base theme hook if this theme implementation is actually implementing a suggestion for another theme hook. All of the base hook's files will be included, and any of its variable processors will be called before this theme implementation's processors. The base hook's processors may change the invoked theme and prevent this implementation from being invoked.

The proposed documentation would be correct for D8 and D7 (although obviously in D7 both preprocess and process functions are checked on the base hook), and D6 with the caveat that "base hook" was called "original hook".

jhodgdon’s picture

Adding to this documentation seems like a good idea. Please make a patch. I think maybe more than you suggested should be included -- you seem to have discovered quite a bit about how this works, and much of this would be useful to others to know.

jhodgdon’s picture

Version: 8.x-dev » 7.x-dev
Status: Active » Closed (fixed)

Although it would be much better actually to open a new issue instead of reopening this issue -- it is quite old and the original intent was satisfied. Can you do that please?

jay.dansand’s picture

Absolutely! I just created issue #2106635: Improve documentation of "base hook" in hook_theme(). and attached a patch with more verbose documentation.