In template_preprocess() we say:

Adds a default set of helper variables for variable processors and templates. This comes in before any other preprocess function which makes it possible to be used in default theme implementations (non-overridden theme functions).

In theme() we say:

template_preprocess(&$variables, $hook): Creates a default set of variables for all theme hooks.

Wrong. template_preprocess() does NOT run for all theme hooks. It does not run for theme functions at all, even though they are “theme hooks,” and even if the theme function has implemented it’s own preprocess function. It only runs for theme hooks implemented using templates, as indicated further down in the documentation:

If the implementation is a function, only the theme-hook-specific preprocess and process functions (the ones ending in _HOOK) are called from the list above. This is because theme hooks with function implementations need to be fast, and calling the non-theme-hook-specific preprocess and process functions for them would incur a noticeable performance penalty.

A little further down in theme() we say:

There are two special variables that these preprocess and process functions can set: ‘theme_hook_suggestion’ and ‘theme_hook_suggestions’. These will be merged together to form a list of ‘suggested’ alternate theme hooks to use, in reverse order of priority. theme_hook_suggestion will always be a higher priority than items in theme_hook_suggestions. theme() will use the highest priority implementation that exists. If none exists, theme() will use the implementation for the theme hook it was called with. These suggestions are similar to and are used for similar reasons as calling theme() with an array as the $hook parameter (see below). The difference is whether the suggestions are determined by the code that calls theme() or by a preprocess or process function.

Wrong. You can implement theme hook suggestions all you want, but they will only work under certain circumstances and are actually processed 3 different ways. See #956520-16: Available theme hook suggestions are only exposed when there are preprocess functions defined.

CommentFileSizeAuthor
#13 drupal-1333122-44.patch10.61 KBtim.plunkett
#4 1333122-4.patch10.35 KBeffulgentsia
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

Thanks for reporting this. Looks like a definite doc bug. Would you consider writing at least a first-pass patch for it, since you may be one of only a few people who fully understand what it should say? :)

effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia commented
Assigned: Unassigned » effulgentsia
Issue tags: -Novice

Working on a first draft now. Removing Novice tag, since this touches on some WTF internals of the theme system.

effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia commented

By the way, just added a comment to #736326-19: hook_process() and associated hooks are undocumented linking to this issue.

effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia commented
Assigned: effulgentsia » Unassigned
Status: Active » Needs review
FileSize
1333122-4.patch10.35 KB

Here's a draft.

You can implement theme hook suggestions all you want, but they will only work under certain circumstances

I don't think that's true. I think that if your preprocess function *sets* a theme_hook_suggestion(s), then that function/template will be used regardless of how theme() is invoked, and I think that's all that's implied in the current documentation.

#956520: Available theme hook suggestions are only exposed when there are preprocess functions defined is a bug related to preprocess functions not currently being able to know what theme_hook_suggestion(s) have been implicitly set by the caller of theme(), and I hope we can fix that bug, but I don't know of documentation that this affects.

#939462: Specific preprocess functions for theme hook suggestions are not invoked is also a WTF related to preprocess_HOOK() and process_HOOK() only being called for the base hook, not the suggestions. Maybe the documentation of these hooks can be made more explicit regarding this, though I wonder if attention would be better spent figuring out the underlying issue instead. Looks like there's some progress on that issue: sorry for not attending to it yet; I'll try to soon.

Jacine’s picture

Jacine
she/her
Location New York
CreditAttribution: Jacine commented

This looks really good. Much, much, much better. Thank you so much @effulgentsia!

I'm going read closer and ask a few other themers to read these changes and make sure it all makes sense to them when I get back.

+++ b/core/modules/system/theme.api.phpundefined
@@ -199,7 +203,11 @@ function hook_process(&$variables, $hook) {
-  $variables['classes'] .= ' my_added_class';
+  // @todo There are no use-cases in Drupal core for this hook. Find one from a
+  //   contributed module, or come up with a good example. Coming up with a good
+  //   example might be tough, since the intent is for nearly everything to be
+  //   achievable via preprocess functions, and for process functions to only be

Contextual links uses it, and it's actually a great use case example: http://api.drupal.org/api/function/contextual_preprocess/7

effulgentsia’s picture

effulgentsia CreditAttribution: effulgentsia commented

Contextual links uses it

contextual uses hook_preprocess(). It's hook_process() that needs an example.

Jacine’s picture

Jacine
she/her
Location New York
CreditAttribution: Jacine commented

Oops! Sorry about that! There is a use case for this too though. RDF module uses it to inject attributes it finds in random variables, along with theme_rdf_template_variable_wrapper(). It's actually a pretty crazy (magic) implementation IMO. It caught me off guard once, cuz I made a custom date variable and it wrapped it in a <span> with RDF attributes.

rdf_process()
theme_rdf_template_variable_wrapper()

But maybe you saw that and thought it wasn't a good example...

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Needs work

This is really nice, clear documentation -- thanks!

A couple of minor things I would suggest changing:

a)

+ * Adds helper variables derived from variables defined in template_preprocess() but potentially altered in other preprocess functions.

This line is too long. See http://drupal.org/node/1354#functions

b) Not sure about this one, but I was a bit thrown by the use of the word "implemented" in a few places in the theme() documentation. For instance:

+ * - template_process_HOOK(&$variables): Should be implemented by the module
+ *   that registers the theme hook, if it needs to perform additional variable
+ *   processing after all preprocess functions have finished.

I would have used the word 'defined' here instead of implemented, maybe? But maybe not, I'm open to leaving it this way too.

moshe weitzman’s picture

moshe weitzman CreditAttribution: moshe weitzman commented

I think a module implements a hook so the proposed text already looks good to me. So, we just have a line length to fix and then RTBC.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

OK. :)

Jacine’s picture

Jacine
she/her
Location New York
CreditAttribution: Jacine commented
Status: Needs work » Needs review
Issue tags: -Needs backport to D7

#4: 1333122-4.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Needs backport to D7

The last submitted patch, 1333122-4.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs work » Needs review
FileSize
drupal-1333122-44.patch10.61 KB

Rerolled. I'm still confused why this behavior itself not a bug, but I'll leave that for #1625158: template_preprocess() doesn't run for theme_field().

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented

The writing looks good here to me. I'll leave it for someone else to review for accuracy.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
CreditAttribution: jhodgdon commented
Status: Needs review » Fixed

I gave this a careful read-over again, and decided that everything in here was just clarifying the documentation we already had (making sure people realize that certain functions are only used when a template is in play), and it also meshes with what Jacine and effulgentcia said above.

So, I went ahead and committed it to 8.x. It also applied well enough to 7.x, so I committed it there too. Thanks all for contributing to this update!

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